Remove semantic model keep-alive code in completion #73844
Conversation
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
dotnet-issue-labeler
bot
added
Area-IDE
untriaged
|
Marking as draft as there are test failures |
|
@genlu -- ptal. It's not a huge perf improvement, but it does help a bit. |
| @@ -6923,7 +6923,6 @@ namespace NS2 | |||
| Dim document = state.Workspace.CurrentSolution.GetDocument(state.Workspace.Documents.Single(Function(d) d.Name = "C.cs").Id) | |||
|
|
|||
| Dim completionService = document.GetLanguageService(Of CompletionService)() | |||
| completionService.GetTestAccessor().SuppressPartialSemantics() | |||
There was a problem hiding this comment.
This test involves 2 documents, will using frozen partial cause flakiness? @jasonmalinowski
There was a problem hiding this comment.
@jasonmalinowski -- any thoughts here?
There was a problem hiding this comment.
ping @jasonmalinowski
There was a problem hiding this comment.
def seemsl ike it could. what's the thinking on why this is ok to remove here?
There was a problem hiding this comment.
Don't remember exactly why I went down that path, but it doesn't seem necessary for this PR, and probably not a good thing as you both mentioned. Added back.
| change = completionService.GetChangeAsync(document, roslynItem, commitCharacter, cancellationToken).WaitAndGetResult(cancellationToken); | ||
| _threadingContext.JoinableTaskFactory.Run(async () => | ||
| { | ||
| change = await completionService.GetChangeAsync(document, roslynItem, commitCharacter, cancellationToken).ConfigureAwait(false); |
There was a problem hiding this comment.
same with remainder of code.
…changes Revert "Remove semantic model keep-alive code in completion (#73844)"
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