From 5aeb42b891093c9442b6f9f12454447326a4f8b3 Mon Sep 17 00:00:00 2001 From: Todd Grunke Date: Tue, 25 Jun 2024 18:39:35 -0700 Subject: [PATCH] Remove semantic model keep-alive code in completion (#73844) * Remove semantic model keep-alive code in completion There are quite a few providers that don't request/use the semantic model during their computations, and in the case where they are being used to commit / GetDescription, the model would be obtained/calculated unnecessarilly. I don't believe the keep alive mechanism here is necessary anymore, as Solution.OnSemanticModelObtained caches the model when it's obtained for the active document in the solution. Additionally, I switched a couple places in CommitManager to use JTF.Runn instead of WaitAndGetResult --- .../IntelliSense/AsyncCompletion/CommitManager.cs | 13 ++++++++++--- .../Core/Portable/Completion/CompletionService.cs | 10 ++++------ .../Completion/CompletionService_GetCompletions.cs | 14 ++++++-------- 3 files changed, 20 insertions(+), 17 deletions(-) diff --git a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CommitManager.cs b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CommitManager.cs index 7d4b4c67a47bc..7c4c8d68f732e 100644 --- a/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CommitManager.cs +++ b/src/EditorFeatures/Core/IntelliSense/AsyncCompletion/CommitManager.cs @@ -212,7 +212,7 @@ public IEnumerable PotentialCommitCharacters // This might be an item promoted by us, make sure we restore it to the original state first. roslynItem = Helpers.DemoteItem(roslynItem); - CompletionChange change; + CompletionChange change = null!; // We met an issue when external code threw an OperationCanceledException and the cancellationToken is not canceled. // Catching this scenario for further investigations. @@ -229,7 +229,10 @@ public IEnumerable PotentialCommitCharacters if (_textView is IDebuggerTextView) roslynItem = ImportCompletionItem.MarkItemToAlwaysFullyQualify(roslynItem); - change = completionService.GetChangeAsync(document, roslynItem, commitCharacter, cancellationToken).WaitAndGetResult(cancellationToken); + _threadingContext.JoinableTaskFactory.Run(async () => + { + change = await completionService.GetChangeAsync(document, roslynItem, commitCharacter, cancellationToken).ConfigureAwait(true); + }); } catch (OperationCanceledException e) when (e.CancellationToken != cancellationToken && FatalError.ReportAndCatch(e)) { @@ -319,7 +322,11 @@ public IEnumerable PotentialCommitCharacters var spanToFormat = triggerSnapshotSpan.TranslateTo(subjectBuffer.CurrentSnapshot, SpanTrackingMode.EdgeInclusive); // Note: C# always completes synchronously, TypeScript is async - var changes = formattingService.GetFormattingChangesAsync(currentDocument, subjectBuffer, spanToFormat.Span.ToTextSpan(), cancellationToken).WaitAndGetResult(cancellationToken); + var changes = ImmutableArray.Empty; + _threadingContext.JoinableTaskFactory.Run(async () => + { + changes = await formattingService.GetFormattingChangesAsync(currentDocument, subjectBuffer, spanToFormat.Span.ToTextSpan(), cancellationToken).ConfigureAwait(true); + }); subjectBuffer.ApplyChanges(changes); } } diff --git a/src/Features/Core/Portable/Completion/CompletionService.cs b/src/Features/Core/Portable/Completion/CompletionService.cs index 6ecbfa300daf4..5eae73ebce122 100644 --- a/src/Features/Core/Portable/Completion/CompletionService.cs +++ b/src/Features/Core/Portable/Completion/CompletionService.cs @@ -214,14 +214,14 @@ public virtual TextSpan GetDefaultCompletionListSpan(SourceText text, int caretP var extensionManager = document.Project.Solution.Workspace.Services.GetRequiredService(); - // We don't need SemanticModel here, just want to make sure it won't get GC'd before CompletionProviders are able to get it. - (document, var semanticModel) = await GetDocumentWithFrozenPartialSemanticsAsync(document, cancellationToken).ConfigureAwait(false); + document = await GetDocumentWithFrozenPartialSemanticsAsync(document, cancellationToken).ConfigureAwait(false); + var description = await extensionManager.PerformFunctionAsync( provider, cancellationToken => provider.GetDescriptionAsync(document, item, options, displayOptions, cancellationToken), defaultValue: null, cancellationToken).ConfigureAwait(false); - GC.KeepAlive(semanticModel); + return description; } @@ -245,8 +245,7 @@ public virtual TextSpan GetDefaultCompletionListSpan(SourceText text, int caretP { var extensionManager = document.Project.Solution.Workspace.Services.GetRequiredService(); - // We don't need SemanticModel here, just want to make sure it won't get GC'd before CompletionProviders are able to get it. - (document, var semanticModel) = await GetDocumentWithFrozenPartialSemanticsAsync(document, cancellationToken).ConfigureAwait(false); + document = await GetDocumentWithFrozenPartialSemanticsAsync(document, cancellationToken).ConfigureAwait(false); var change = await extensionManager.PerformFunctionAsync( provider, @@ -256,7 +255,6 @@ public virtual TextSpan GetDefaultCompletionListSpan(SourceText text, int caretP if (change == null) return CompletionChange.Create(new TextChange(new TextSpan(), "")); - GC.KeepAlive(semanticModel); Debug.Assert(item.Span == change.TextChange.Span || item.IsComplexTextEdit); return change; } diff --git a/src/Features/Core/Portable/Completion/CompletionService_GetCompletions.cs b/src/Features/Core/Portable/Completion/CompletionService_GetCompletions.cs index 178d6758b87e3..604fbf2a78cd4 100644 --- a/src/Features/Core/Portable/Completion/CompletionService_GetCompletions.cs +++ b/src/Features/Core/Portable/Completion/CompletionService_GetCompletions.cs @@ -68,8 +68,7 @@ public abstract partial class CompletionService ImmutableHashSet? roles = null, CancellationToken cancellationToken = default) { - // We don't need SemanticModel here, just want to make sure it won't get GC'd before CompletionProviders are able to get it. - (document, var semanticModel) = await GetDocumentWithFrozenPartialSemanticsAsync(document, cancellationToken).ConfigureAwait(false); + document = await GetDocumentWithFrozenPartialSemanticsAsync(document, cancellationToken).ConfigureAwait(false); var text = await document.GetValueTextAsync(cancellationToken).ConfigureAwait(false); var completionListSpan = GetDefaultCompletionListSpan(text, caretPosition); @@ -115,8 +114,6 @@ public abstract partial class CompletionService var augmentingContexts = await ComputeNonEmptyCompletionContextsAsync( document, caretPosition, trigger, options, completionListSpan, augmentingProviders, sharedContext, cancellationToken).ConfigureAwait(false); - GC.KeepAlive(semanticModel); - // Providers are ordered, but we processed them in our own order. Ensure that the // groups are properly ordered based on the original providers. var completionProviderToIndex = GetCompletionProviderToIndex(providers); @@ -172,19 +169,20 @@ public abstract partial class CompletionService } /// - /// Returns a document with frozen partial semantic unless we already have a complete compilation available. + /// Returns a document with frozen partial semantic model unless caller is test code require full semantics. /// Getting full semantic could be costly in certain scenarios and would cause significant delay in completion. /// In most cases we'd still end up with complete document, but we'd consider it an acceptable trade-off even when /// we get into this transient state. /// - private async Task<(Document document, SemanticModel? semanticModel)> GetDocumentWithFrozenPartialSemanticsAsync(Document document, CancellationToken cancellationToken) + private async Task GetDocumentWithFrozenPartialSemanticsAsync(Document document, CancellationToken cancellationToken) { if (_suppressPartialSemantics) { - return (document, await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false)); + var _ = await document.GetRequiredSemanticModelAsync(cancellationToken).ConfigureAwait(false); + return document; } - return await document.GetFullOrPartialSemanticModelAsync(cancellationToken).ConfigureAwait(false); + return document.WithFrozenPartialSemantics(cancellationToken); } private static bool ValidatePossibleTriggerCharacterSet(CompletionTriggerKind completionTriggerKind, IEnumerable triggeredProviders,