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

Search fewer projects when looking for linked documents #73939

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

CyrusNajmabadi
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi commented Jun 11, 2024

Followup to #73937.

Drops CPU cost while loading a project like Roslyn from:

image

to

image

The idea here is that instead of searching every other project to find linked documents to share document contents with, we instead only search the particular other "flavors" of a particular project. For example, when trying to share content for a file in Workspaces (netstandard2.0) we now only search in Workspaces (net7.0) and Workspaces (net8.0) instead of all projects.

--

Build: https://devdiv.visualstudio.com/DevDiv/_build/results?buildId=9815633&view=results

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Jun 11, 2024
@CyrusNajmabadi CyrusNajmabadi marked this pull request as ready for review June 11, 2024 17:24
@CyrusNajmabadi CyrusNajmabadi requested a review from a team as a code owner June 11, 2024 17:24
@CyrusNajmabadi
Copy link
Member Author

@jasonmalinowski @ToddGrun this is ready for review.

@jasonmalinowski
Copy link
Member

For example, when trying to share content for a file in Workspaces (netstandard2.0) we now only search in Workspaces (net7.0) and Workspaces (net8.0) instead of all projects.

This would work for direct multi-targeting but would mean we're no longer doing file sharing for things like shared projects or linked files. So this may have a performance impact in those cases if the file counts were high enough.

I am missing some historical context here, but this kinda feels like we're undoing the benefits of #72792 -- if the code is simpler now but we're having to add magic hashing and fancy locking and heuristics...was the old code just better? Or can we get the best of both worlds somehow?

@CyrusNajmabadi
Copy link
Member Author

if the code is simpler now but we're having to add magic hashing and fancy locking and heuristics...was the old code just better?

It was much more complex to try to have a single dictionary. It's nto really magic hashing. It's just what the runtime does on .net core for strings. We're on netfx, so i'm just copying their impl since it's faster :)

It is a heuristic. But i don't see a concern there. Having things perform great for the common case is the ideal for me :)

This would work for direct multi-targeting but would mean we're no longer doing file sharing for things like shared projects or linked files. So this may have a performance impact in those cases if the file counts were high enough.

Sure. It's a tradeoff. With the old system tehre was a lot of complexity (and bugginess) keepign the full map correct. In hte new system, there are smaller maps that are trivial to keep up to date. But we need to bit smarter as we have 1k projects to search through. This feels like a good middle ground.

@CyrusNajmabadi
Copy link
Member Author

Ok. Talking to @jasonmalinowski @ToddGrun, this is a prime case for an control tower flag. We can make the change, and then have an option to disable this if we run into any issues in the wild. @jasonmalinowski does that work for you?

@CyrusNajmabadi
Copy link
Member Author

@jasonmalinowski @ToddGrun ptal. Note: i have a feature flag here so we can control this through control tower if we need to.

@CyrusNajmabadi CyrusNajmabadi changed the title Search less projects when looking for linked documents Search fewer projects when looking for linked documents Jul 1, 2024
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 VSCode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants