-
Notifications
You must be signed in to change notification settings - Fork 418
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
Optimize Intellisense performance #1761
Optimize Intellisense performance #1761
Conversation
@@ -60,9 +60,13 @@ public static async Task<IEnumerable<ISymbol>> GetCompletionSymbolsAsync(this Co | |||
} | |||
|
|||
// if the completion provider encoded symbols into Properties, we can return them | |||
if (properties.ContainsKey(SymbolName) && properties.ContainsKey(SymbolKind)) | |||
if (properties.TryGetValue(SymbolName, out string symbolNameValue) | |||
&& properties.TryGetValue(SymbolKind, out string symbolKindValue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calls to the ImmutableDictionary.GetValue<> were the main CPU-hog, now they are cached outside of the loop.
@@ -51,7 +51,7 @@ public async Task<IEnumerable<AutoCompleteResponse>> Handle(AutoCompleteRequest | |||
|
|||
// get recommended symbols to match them up later with SymbolCompletionProvider | |||
var semanticModel = await document.GetSemanticModelAsync(); | |||
var recommendedSymbols = await Recommender.GetRecommendedSymbolsAtPositionAsync(semanticModel, position, _workspace); | |||
var recommendedSymbols = (await Recommender.GetRecommendedSymbolsAtPositionAsync(semanticModel, position, _workspace)).ToArray(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Collecting recommendedSymbols into an array, instead of passing Rolsyn's opaque collection type, pulled a lot of weight here. Note that this gets that expensive .Where() call inside the BigN loop (number of elements in the auto complete dropdown).
Actually tried to test with this change only, and it does help a lot, but only does half of the job.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very nice, thanks!
dotnet/vscode-csharp#3421
Auto complete popup always seemed to have a spotty performance and random delays, so I drilled the problem in VS profiler. Luckily, seems that a lot of it is very low hanging fruit.
Main culprit seems to be
CompletionItemExtensions
, which did a complex search onrecommendedSymbols
times everycompletionItem
in the dropdown. RecommendedSymbols seems like something that should be a very short list, but this still throttles the CPU.Lesser issue was a collection of string extensions methods used for sorting the resulting competition list. VS (or Rolsynator?) immediately started yelling in red squiggles about using StringComparison everywhere, so I did. Luckily
StringExtensions
class was only used by Intellisense service, so there are no worries about breaking something else.Before vs After
"Before" gif doesn't capture the "cpu melting noise" part of the experience