Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move usings on paste off the UI thread #61092

Merged
merged 3 commits into from
May 3, 2022
Merged

Move usings on paste off the UI thread #61092

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2bfb83c
Move usings on paste to the background and utilize IBackgroundWorkInd…
ryzngard May 2, 2022
d89d48a
Do correct async pattern
ryzngard May 2, 2022
932aab7
Fix integration tests
ryzngard May 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 39 additions & 23 deletions src/EditorFeatures/Core/AddImports/AbstractAddImportsPasteCommandHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@
// See the LICENSE file in the project root for more information.

using System;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis.AddMissingImports;
using Microsoft.CodeAnalysis.CodeCleanup;
using Microsoft.CodeAnalysis.Completion;
using Microsoft.CodeAnalysis.Editor.BackgroundWorkIndicator;
using Microsoft.CodeAnalysis.Editor.Shared.Extensions;
using Microsoft.CodeAnalysis.Editor.Shared.Options;
using Microsoft.CodeAnalysis.Editor.Shared.Utilities;
Expand Down Expand Up @@ -115,32 +117,46 @@ private void ExecuteCommandWorker(
return;
}

using var _ = executionContext.OperationContext.AddScope(allowCancellation: true, DialogText);
var cancellationToken = executionContext.OperationContext.UserCancellationToken;
var indicatorFactory = document.Project.Solution.Workspace.Services.GetRequiredService<IBackgroundWorkIndicatorFactory>();
var backgroundWorkContext = indicatorFactory.Create(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not put this in a using block?

see the code in GoToDefinitionCommandHandler for a suitable pattern on how to do the sync->async jump that does all this without the need for a TAsk.Run

args.TextView,
snapshotSpan,
DialogText,
cancelOnEdit: true,
cancelOnFocusLost: true);

// We're going to log the same thing on success or failure since this blocks the UI thread. This measurement is
// intended to tell us how long we're blocking the user from typing with this action.
using var blockLogger = Logger.LogBlock(FunctionId.CommandHandler_Paste_ImportsOnPaste, KeyValueLogMessage.Create(LogType.UserAction), cancellationToken);
var cancellationToken = backgroundWorkContext.UserCancellationToken;

var addMissingImportsService = document.GetRequiredLanguageService<IAddMissingImportsFeatureService>();
#pragma warning disable VSTHRD102 // Implement internal logic asynchronously
var updatedDocument = _threadingContext.JoinableTaskFactory.Run(async () =>
Task.Run(async () =>
{
var cleanupOptions = await document.GetCodeCleanupOptionsAsync(_globalOptions, cancellationToken).ConfigureAwait(false);

var options = new AddMissingImportsOptions(
CleanupOptions: cleanupOptions,
HideAdvancedMembers: _globalOptions.GetOption(CompletionOptionsStorage.HideAdvancedMembers, document.Project.Language));

return await addMissingImportsService.AddMissingImportsAsync(document, textSpan, options, cancellationToken).ConfigureAwait(false);
});
#pragma warning restore VSTHRD102 // Implement internal logic asynchronously
if (updatedDocument is null)
{
return;
}

workspace.TryApplyChanges(updatedDocument.Project.Solution);
try
{
// We're going to log the same thing on success or failure since this blocks the UI thread. This measurement is
// intended to tell us how long we're blocking the user from typing with this action.
using var blockLogger = Logger.LogBlock(FunctionId.CommandHandler_Paste_ImportsOnPaste, KeyValueLogMessage.Create(LogType.UserAction), cancellationToken);

var addMissingImportsService = document.GetRequiredLanguageService<IAddMissingImportsFeatureService>();

var cleanupOptions = await document.GetCodeCleanupOptionsAsync(_globalOptions, cancellationToken).ConfigureAwait(false);

var options = new AddMissingImportsOptions(
CleanupOptions: cleanupOptions,
HideAdvancedMembers: _globalOptions.GetOption(CompletionOptionsStorage.HideAdvancedMembers, document.Project.Language));

var updatedDocument = await addMissingImportsService.AddMissingImportsAsync(document, textSpan, options, cancellationToken).ConfigureAwait(false);

if (updatedDocument is null)
{
return;
}

workspace.TryApplyChanges(updatedDocument.Project.Solution);
}
finally
{
backgroundWorkContext.Dispose();
}
}, cancellationToken);
}
}
}