Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep the solution pinned (on host and OOP) when making multiple calls between systems. #70765

Merged
merged 10 commits into from
Nov 11, 2023

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented Nov 10, 2023

Fixes https://devdiv.visualstudio.com/DevDiv/_workitems/edit/1914915

Most times when a feature calls into OOP, it does so with a single message it wants to compute, against a particular solution snapshot. The OOP communication system ensures both the client and server are in sync with that solution snapshot, and it then runs the requested command on that snapshot. When the command returns, the OOP side is free to then drop that snapshot if nothing else is looking at it.

This can be a potential perf issue when a feature makes many calls to OOP against a particular snapshot. in the worst case, between each call, the OOP side may choose to drop the solution, forcing the differences between it, and the main solution to be resync'ed on each individual call, and causing things like compilations to drop away.

This PR changes a couple of places where we do this to use a simple utility class we have to keep a particular solution pinned for the scope where we are making several calls from the host to OOP. This ensure that the solution is not ever dropped on the OOP side for the duration of that scope, and that cache values (like compilations) can be reused from call to call.

Generally speaking, any features that make multiple calls to OOP with a single snapshot, should use this system.

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Nov 10, 2023
@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review November 10, 2023 20:36
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner November 10, 2023 20:36
Copy link
Contributor

@ToddGrun ToddGrun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

global.json Outdated Show resolved Hide resolved
@CyrusNajmabadi CyrusNajmabadi merged commit eeb8518 into dotnet:main Nov 11, 2023
23 of 24 checks passed
@ghost ghost added this to the Next milestone Nov 11, 2023
@CyrusNajmabadi CyrusNajmabadi deleted the keepAlive branch November 12, 2023 02:56
@RikkiGibson RikkiGibson modified the milestones: Next, 17.9 P2 Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants