-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Very slow IntelliSense on relatively small code base #67926
Comments
Trace shared with @CyrusNajmabadi and @sharwell. |
I'm suspecting this is related to: roslyn/src/Compilers/CSharp/Portable/Binder/Semantics/OverloadResolution/OverloadResolution.cs Lines 250 to 257 in ef625f3
Tagging @AlekseyTs as well since this seems a compiler issue. |
On the IDE side, it was a simple call to LookupSymbols roslyn/src/Workspaces/Core/Portable/Recommendations/AbstractRecommendationServiceRunner.cs Line 385 in ef625f3
Random thought, are use site diagnostics (or diagnostics in general) relevant in the IDE scenario above? Could they somehow be eliminated? |
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? |
@sharwell @AlekseyTs How can we move forward with this? |
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 that's a fairly sprawling bit of code. Can you help narrow down what file is having the perf issue? |
@jaredpar I added more info the repo readme. |
@Youssef1313, I didn't see a notification for the repo. Please resend if possible, thanks. |
@cston Re-sent. |
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. |
@cston Thanks for the update! When you're mentioning hidden overloads, are you referring to the browsable attribute, or hidden by accessibility? |
@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
|
@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? |
@Youssef1313, I think we could keep the change simple by targeting extension methods only which are not part of a class hierarchy. |
@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. |
@Youssef1313 do you have an updated trace? |
@CyrusNajmabadi Yup sure. Already uploading, but... |
Uplaoded the trace here: https://developercommunity.visualstudio.com/t/Performance-trace-for-Roslyn-team/10415897 |
FWIW, it looks like there are a number of
If those two |
@sharwell can you look into sharing the semantic model with KeywordCompletionProvider and SymbolCompletionProvider |
@cston How did you determine that separate semantic models were being used for those two providers? |
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. |
The lambda expressions were bound for each provider, and the
Are there multiple completion requests when typing a single |
It looks like typing |
@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. |
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. |
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:
@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. |
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). |
@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.
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. |
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 byNameSyntaxClassifier.AddClassification
devenv
:GetSymbolInfo
is called byCSharpSyntaxContext.CreateContext
andCSharpTypeInferenceService+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.
The text was updated successfully, but these errors were encountered: