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.

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

Remove TaskQueue and SafeContinueWith extensions #76459

Merged
merged 19 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
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
103 changes: 0 additions & 103 deletions src/Workspaces/Core/Portable/Utilities/TaskQueue.cs

This file was deleted.

68 changes: 55 additions & 13 deletions src/Workspaces/Core/Portable/Workspace/Workspace.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.CodeAnalysis;
using Microsoft.CodeAnalysis.Collections;
using Microsoft.CodeAnalysis.Diagnostics;
using Microsoft.CodeAnalysis.ErrorReporting;
using Microsoft.CodeAnalysis.Host;
Expand All @@ -19,7 +20,6 @@
using Microsoft.CodeAnalysis.PooledObjects;
using Microsoft.CodeAnalysis.Shared.Extensions;
using Microsoft.CodeAnalysis.Shared.TestHooks;
using Microsoft.CodeAnalysis.Shared.Utilities;
using Microsoft.CodeAnalysis.Text;
using Roslyn.Utilities;

Expand All @@ -36,6 +36,12 @@ public abstract partial class Workspace : IDisposable
{
private readonly ILegacyGlobalOptionService _legacyOptions;

private readonly IAsynchronousOperationListener _asyncOperationListener;

private readonly AsyncBatchingWorkQueue<Action> _workQueue;
private readonly CancellationTokenSource _workQueueTokenSource = new();
private readonly ITaskSchedulerProvider _taskSchedulerProvider;

// forces serialization of mutation calls from host (OnXXX methods). Must take this lock before taking stateLock.
private readonly SemaphoreSlim _serializationLock = new(initialCount: 1);

Expand All @@ -47,8 +53,6 @@ public abstract partial class Workspace : IDisposable
/// </summary>
private Solution _latestSolution;

private readonly TaskQueue _taskQueue;

// test hooks.
internal static bool TestHookStandaloneProjectsDoNotHoldReferences = false;

Expand All @@ -74,16 +78,22 @@ protected Workspace(HostServices host, string? workspaceKind)
_legacyOptions.RegisterWorkspace(this);

// queue used for sending events
var schedulerProvider = Services.GetRequiredService<ITaskSchedulerProvider>();
_taskSchedulerProvider = Services.GetRequiredService<ITaskSchedulerProvider>();

var listenerProvider = Services.GetRequiredService<IWorkspaceAsynchronousOperationListenerProvider>();
_taskQueue = new TaskQueue(listenerProvider.GetListener(), schedulerProvider.CurrentContextScheduler);
_asyncOperationListener = listenerProvider.GetListener();
_workQueue = new(
TimeSpan.Zero,
ProcessWorkQueueAsync,
_asyncOperationListener,
_workQueueTokenSource.Token);

// initialize with empty solution
var info = SolutionInfo.Create(SolutionId.CreateNewId(), VersionStamp.Create());

var emptyOptions = new SolutionOptionSet(_legacyOptions);

_latestSolution = CreateSolution(info, emptyOptions, analyzerReferences: [], fallbackAnalyzerOptions: ImmutableDictionary<string, StructuredAnalyzerConfigOptions>.Empty);
_latestSolution = CreateSolution(
SolutionInfo.Create(SolutionId.CreateNewId(), VersionStamp.Create()),
new SolutionOptionSet(_legacyOptions),
analyzerReferences: [],
fallbackAnalyzerOptions: ImmutableDictionary<string, StructuredAnalyzerConfigOptions>.Empty);

_updateSourceGeneratorsQueue = new AsyncBatchingWorkQueue<(ProjectId? projectId, bool forceRegeneration)>(
// Idle processing speed
Expand Down Expand Up @@ -581,15 +591,25 @@ internal void UpdateCurrentSolutionOnOptionsChanged()
/// Executes an action as a background task, as part of a sequential queue of tasks.
/// </summary>
[SuppressMessage("Style", "VSTHRD200:Use \"Async\" suffix for async methods", Justification = "This is a Task wrapper, not an asynchronous method.")]
#pragma warning disable IDE0060 // Remove unused parameter
protected internal Task ScheduleTask(Action action, string? taskName = "Workspace.Task")
=> _taskQueue.ScheduleTask(taskName ?? "Workspace.Task", action, CancellationToken.None);
{
_workQueue.AddWork(action);
return _workQueue.WaitUntilCurrentBatchCompletesAsync();
}

/// <summary>
/// Execute a function as a background task, as part of a sequential queue of tasks.
/// </summary>
[SuppressMessage("Style", "VSTHRD200:Use \"Async\" suffix for async methods", Justification = "This is a Task wrapper, not an asynchronous method.")]
protected internal Task<T> ScheduleTask<T>(Func<T> func, string? taskName = "Workspace.Task")
=> _taskQueue.ScheduleTask(taskName ?? "Workspace.Task", func, CancellationToken.None);
protected internal async Task<T> ScheduleTask<T>(Func<T> func, string? taskName = "Workspace.Task")
{
T? result = default;
_workQueue.AddWork(() => result = func());
await _workQueue.WaitUntilCurrentBatchCompletesAsync().ConfigureAwait(false);
return result!;
}
#pragma warning restore IDE0060 // Remove unused parameter

/// <summary>
/// Override this method to act immediately when the text of a document has changed, as opposed
Expand Down Expand Up @@ -695,6 +715,28 @@ protected virtual void Dispose(bool finalize)

// We're disposing this workspace. Stop any work to update SG docs in the background.
_updateSourceGeneratorsQueueTokenSource.Cancel();
_workQueueTokenSource.Cancel();
}

private async ValueTask ProcessWorkQueueAsync(ImmutableSegmentedList<Action> list, CancellationToken cancellationToken)
{
// Hop over to the right scheduler to execute all this work.
await Task.Factory.StartNew(() =>
{
foreach (var item in list)
{
cancellationToken.ThrowIfCancellationRequested();
Copy link
Member

Choose a reason for hiding this comment

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

This cancellation could change behavior here -- before Workspace events would be queued and would continue, at least until the termination of the process. If that cancellation triggers at some earlier point now it might mean people won't get events. Practically, this would only matter during shutdown, and since we were queuing to the UI thread there was already a pretty good chance those might never fire -- there's really no difference between "we cancelled them and they never ran during shutdown" versus "the UI thread was never available during shutdown to run them anyways".

This is probably a small win for shutdown perf; but if we see any strange bugs appear during shutdown this might be a place to investigate. Probably good to see if it's fine, at least!

Copy link
Member Author

Choose a reason for hiding this comment

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

yup. i'm very ok with this behavior. you dispose of the workspace, and outstanding work may not happen.


try
{
item();
}
catch (Exception e) when (FatalError.ReportAndCatchUnlessCanceled(e))
Copy link
Member

Choose a reason for hiding this comment

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

Not sure why the "UnlessCancelled" here -- I wouldn't expect cancellation from the action since we never gave it a token.

{
// Ensure we continue onto further items, even if one particular item fails.
}
}
}, cancellationToken, TaskCreationOptions.None, _taskSchedulerProvider.CurrentContextScheduler).ConfigureAwait(false);
}

#region Host API
Expand Down
8 changes: 4 additions & 4 deletions src/Workspaces/Core/Portable/Workspace/Workspace_Editor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -455,9 +455,9 @@ internal void OnSourceGeneratedDocumentOpened(

// Fire and forget that the workspace is changing.
// We raise 2 events for source document opened.
var token = _taskQueue.Listener.BeginAsyncOperation(nameof(OnSourceGeneratedDocumentOpened));
var token = _asyncOperationListener.BeginAsyncOperation(nameof(OnSourceGeneratedDocumentOpened));
_ = RaiseDocumentOpenedEventAsync(document).CompletesAsyncOperation(token);
token = _taskQueue.Listener.BeginAsyncOperation(TextDocumentOpenedEventName);
token = _asyncOperationListener.BeginAsyncOperation(TextDocumentOpenedEventName);
_ = RaiseTextDocumentOpenedEventAsync(document).CompletesAsyncOperation(token);
}

Expand All @@ -475,9 +475,9 @@ internal void OnSourceGeneratedDocumentClosed(SourceGeneratedDocument document)

// Fire and forget that the workspace is changing.
// We raise 2 events for source document closed.
var token = _taskQueue.Listener.BeginAsyncOperation(nameof(OnSourceGeneratedDocumentClosed));
var token = _asyncOperationListener.BeginAsyncOperation(nameof(OnSourceGeneratedDocumentClosed));
_ = RaiseDocumentClosedEventAsync(document).CompletesAsyncOperation(token);
token = _taskQueue.Listener.BeginAsyncOperation(TextDocumentClosedEventName);
token = _asyncOperationListener.BeginAsyncOperation(TextDocumentClosedEventName);
_ = RaiseTextDocumentClosedEventAsync(document).CompletesAsyncOperation(token);
}
}
Expand Down
Loading
Loading