Very slow IntelliSense on relatively small code base

[Youssef1313]

I'm not sure which process should be looked into for this case, devenv or ServiceHub.RoslynCodeAnalysisService.

GetSymbolInfo is commonly showing up as expensive in both:

• ServiceHub.RoslynCodeAnalysisService: GetSymbolInfo is called by NameSyntaxClassifier.AddClassification
• devenv: GetSymbolInfo is called by CSharpSyntaxContext.CreateContext and CSharpTypeInferenceService+TypeInferrer.InferTypesWorker_DoNotCallDirectly.

---

If I clear GroupPats and FoldPats (which I read somewhere, but I don't understand what they actually do), I see GetUseSiteInfo starts showing up.

[Youssef1313]

Trace shared with @CyrusNajmabadi and @sharwell.

[Youssef1313]

I'm suspecting this is related to:

roslyn/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs

Lines 250 to 257 in ef625f3

	// NOTE: We use a quadratic algorithm to determine which members override/hide
	// each other (i.e. we compare them pairwise). We could move to a linear
	// algorithm that builds the closure set of overridden/hidden members and then
	// uses that set to filter the candidate, but that would still involve realizing
	// a lot of PE symbols. Instead, we partition the candidates by containing type.
	// With this information, we can efficiently skip checks where the (potentially)
	// overriding or hiding member is not in a subtype of the type containing the
	// (potentially) overridden or hidden member.

Tagging @AlekseyTs as well since this seems a compiler issue.

[Youssef1313]

On the IDE side, it was a simple call to LookupSymbols

roslyn/src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationServiceRunner.cs

Line 385 in ef625f3

protected ImmutableArray<ISymbol> LookupSymbolsInContainer(

Random thought, are use site diagnostics (or diagnostics in general) relevant in the IDE scenario above? Could they somehow be eliminated?

[AlekseyTs]

How many requests IDE performs for this scenario?

[AlekseyTs]

Also, how much in terms of elanced time we are talking about

[Youssef1313]

How many requests IDE performs for this scenario?
Also, how much in terms of elanced time we are talking about

@sharwell Do you know the answer for these?

[Youssef1313]

@sharwell @AlekseyTs How can we move forward with this?

[jaredpar]

@Youssef1313

Do you have a code sample that illustrates this problem? Can you share out the ETL traces here? It's hard to tell what is going on from the screen shots alone.

@cston PTAL once the information is available.

[Youssef1313]

@jaredpar @cston I have invited you to a private repo with a repro.

[jaredpar]

@Youssef1313 that's a fairly sprawling bit of code. Can you help narrow down what file is having the perf issue?

[Youssef1313]

@jaredpar I added more info the repo readme.

[cston]

@Youssef1313, I didn't see a notification for the repo. Please resend if possible, thanks.

[Youssef1313]

@cston Re-sent.

[cston]

Thanks @Youssef1313 for the initial investigation. It looks like the perf issue may be determining which overloads are hidden, as you indicated above. I'll investigate a possible fix.

[jeromelaban]

@cston Thanks for the update! When you're mentioning hidden overloads, are you referring to the browsable attribute, or hidden by accessibility?

[Youssef1313]

@jeromelaban I think it's about shadowing, even if the code doesn't have any methods that shadow others from base. It's an algorithm that runs as part of the overload resolution:

roslyn/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs

Lines 250 to 257 in ef625f3

	// NOTE: We use a quadratic algorithm to determine which members override/hide
	// each other (i.e. we compare them pairwise). We could move to a linear
	// algorithm that builds the closure set of overridden/hidden members and then
	// uses that set to filter the candidate, but that would still involve realizing
	// a lot of PE symbols. Instead, we partition the candidates by containing type.
	// With this information, we can efficiently skip checks where the (potentially)
	// overriding or hiding member is not in a subtype of the type containing the
	// (potentially) overridden or hidden member.

[Youssef1313]

@cston I see the comment says "We could move to a linear algorithm". I'm curious if this is the possible fix you're investigating?

[cston]

@Youssef1313, I think we could keep the change simple by targeting extension methods only which are not part of a class hierarchy.

[Youssef1313]

@cston The performance issue is still present in Preview 3. Can't tell for sure if it has improved a bit, but anyway it's still veeeery slow, really.

[CyrusNajmabadi]

@Youssef1313 do you have an updated trace?

[Youssef1313]

@CyrusNajmabadi Yup sure. Already uploading, but...

[Youssef1313]

Uplaoded the trace here: https://developercommunity.visualstudio.com/t/Performance-trace-for-Roslyn-team/10415897

[cston]

FWIW, it looks like there are a number of CompletionProvider instances invoked by CompletionService.GetCompletionsAsync when typing . in the method, and the CompletionProvider instances bind the modified method at least 4 times with distinct SemanticModel instances, so there is no sharing of BoundLambda instances between those cases. Two of the CompletionProvider instances are from Roslyn:

• Microsoft.CodeAnalysis.CSharp.Completion.Providers.KeywordCompletionProvider
• Microsoft.CodeAnalysis.CSharp.Completion.Providers.SymbolCompletionProvider

If those two CompletionProvider instances could share a SemanticModel, at least when not using a SpeculativeSemanticModel, that might increase Intellisense perf slightly.

cc @CyrusNajmabadi

[CyrusNajmabadi]

@sharwell can you look into sharing the semantic model with KeywordCompletionProvider and SymbolCompletionProvider

[sharwell]

@cston How did you determine that separate semantic models were being used for those two providers?

[sharwell]

Ah, found it. The two instances are using a shared cache (SharedSyntaxContextsWithSpeculativeModel.GetSyntaxContextAsync). Both show up as causing the creation of a semantic model, but not during the same completion request.

[cston]

How did you determine that separate semantic models were being used for those two providers?

The lambda expressions were bound for each provider, and the BoundLambda cache was not shared.

The two instances are using a shared cache (SharedSyntaxContextsWithSpeculativeModel.GetSyntaxContextAsync). Both show up as causing the creation of a semantic model, but not during the same completion request.

Are there multiple completion requests when typing a single . in the editor? And if so, are the completion requests executed serially?

[cston]

It looks like typing . in the Visual Studio editor results in binding the method body 10 times. It's not clear how many of those method body bindings are required for displaying the completion list though.

[cston]

It looks like typing . in the Visual Studio editor results in binding the method body 10 times.

@sharwell, @CyrusNajmabadi, do we expect to bind the containing method 10 times in this scenario? Again, it's not clear how many of those bindings block completion but perhaps the additional work in other threads may affect overall perf of completion.

[CyrusNajmabadi]

We have lots of features waking up and doing things. What matters to me is what the threads are doing that are computing the items for completion.

[cston]

For overload resolution cases where there are many overloads and potentially nested lambda expressions, it should be possible to reduce the number of times lambda bodies are bound by:

• using explicit parameter types and even return types for lambda expressions, or
• using explicit type arguments for generic method calls with lambda arguments, or
• reducing nesting of lambda expressions.

@jeromelaban, @Youssef1313, sorry for not covering that earlier. Would one or more of those changes work in this case? Are there overload scenarios where those changes are not appropriate? Thanks.

[Youssef1313]

The issue here is that it's not something we encounter only in our codebase where we can workaround the perf issue, but that's the API we ship for C# Markup and the performance affects our users' code. So, we really want our users to write the code in the way they want (keeping in mind that options 1 and 2 are very much verbose anyways).

[sharwell]

@Youssef1313 For that situation, you'll want to provide an API that discourages the use of patterns that cause these problems. My understanding is the compiler is interested in introducing a warning when type inference reaches a particular complexity threshold, but that work is not yet complete.

we really want our users to write the code in the way they want

No objection to this, as long as either "the way they want" doesn't involve deeply nested lambdas without types specified or the user is willing to accept severe performance problems.