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

DependentProjectsFinder.GetAssemblyReferenceType might perform unnecessary binding #41358

Open
sharwell opened this issue Feb 2, 2020 · 3 comments
Labels
Area-IDE Bug Concept-OOP Related to out-of-proc help wanted The issue is "up for grabs" - add a comment if you are interested in working on it Urgency-Soon
Milestone

Comments

@sharwell
Copy link
Member

sharwell commented Feb 2, 2020

The DependentProjectsFinder.GetAssemblyReferenceType method obtains an assembly or module symbol here:

if (compilation.GetAssemblyOrModuleSymbol(reference) is IAssemblySymbol symbol)

This symbol is passed to a predicate, where the only actual use of this predicate is the following:

a => a.Name == assemblyName ? true : (bool?)null,

The HasReferenceToAssembly method should be updated to avoid requiring symbol binding to check the name of an assembly.

🔗 Originally discovered while investigating internal issue DevDiv 1006914.

@sharwell sharwell added Area-IDE Bug Concept-OOP Related to out-of-proc help wanted The issue is "up for grabs" - add a comment if you are interested in working on it labels Feb 2, 2020
@jinujoseph jinujoseph added this to the 16.6 milestone Feb 4, 2020
sharwell added a commit to sharwell/roslyn that referenced this issue Feb 4, 2020
@jinujoseph jinujoseph added 4 - In Review A fix for the issue is submitted for review. and removed help wanted The issue is "up for grabs" - add a comment if you are interested in working on it labels Feb 5, 2020
@bleroy
Copy link

bleroy commented Feb 20, 2020

FWIW, still repros in 16.5 Preview 3.

@sharwell sharwell added help wanted The issue is "up for grabs" - add a comment if you are interested in working on it and removed 4 - In Review A fix for the issue is submitted for review. labels Mar 22, 2020
@sharwell sharwell removed their assignment Mar 22, 2020
@jinujoseph jinujoseph modified the milestones: 16.6, 16.7 Apr 17, 2020
@jinujoseph jinujoseph modified the milestones: 16.7, Backlog Jun 19, 2020
@sharwell
Copy link
Member Author

😨 This was responsible for 3.5GiB allocations in a 100 second performance trace taken with 16.9 Preview 2 (IntPreview 1a8a572).

@sharwell sharwell modified the milestones: Backlog, 16.9 Oct 24, 2020
@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Oct 24, 2020

Any suggestions on an alternate path to take? If this is a PEReference we could attempt to crack it open directly. But that seems non-ideal. Esp. as i would hope we could get an IAssemblySymbol cheaply to do this check. Should this maybe go to compiler to see if htey can mkae things faster or more lightweight here?

@jinujoseph jinujoseph modified the milestones: 16.9, Backlog Jan 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE Bug Concept-OOP Related to out-of-proc help wanted The issue is "up for grabs" - add a comment if you are interested in working on it Urgency-Soon
Projects
None yet
Development

No branches or pull requests

5 participants