From 070ec96c62f587afa25d37c7a3690aad76b7ada7 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 16 Dec 2024 14:39:16 -0800 Subject: [PATCH 1/3] remove usage of TaskQueue from the interactive window --- .../Interactive/InteractiveEvaluator.cs | 4 - .../Core/Interactive/InteractiveSession.cs | 75 +++++++++++++------ .../CSharpVsInteractiveWindowProvider.cs | 5 -- 3 files changed, 53 insertions(+), 31 deletions(-) diff --git a/src/EditorFeatures/Core.Wpf/Interactive/InteractiveEvaluator.cs b/src/EditorFeatures/Core.Wpf/Interactive/InteractiveEvaluator.cs index 452e3a2849679..bf3b243d0be11 100644 --- a/src/EditorFeatures/Core.Wpf/Interactive/InteractiveEvaluator.cs +++ b/src/EditorFeatures/Core.Wpf/Interactive/InteractiveEvaluator.cs @@ -14,7 +14,6 @@ using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Host; -using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.VisualStudio.InteractiveWindow; using Microsoft.VisualStudio.InteractiveWindow.Commands; @@ -70,7 +69,6 @@ internal CSharpInteractiveEvaluator( IInteractiveWindowCommandsFactory commandsFactory, ImmutableArray commands, ITextDocumentFactoryService textDocumentFactoryService, - EditorOptionsService editorOptionsService, InteractiveEvaluatorLanguageInfoProvider languageInfo, string initialWorkingDirectory) { @@ -87,10 +85,8 @@ internal CSharpInteractiveEvaluator( _session = new InteractiveSession( _workspace, - threadingContext, listener, textDocumentFactoryService, - editorOptionsService, languageInfo, initialWorkingDirectory); diff --git a/src/EditorFeatures/Core/Interactive/InteractiveSession.cs b/src/EditorFeatures/Core/Interactive/InteractiveSession.cs index cede11519dbfa..661376af18a4a 100644 --- a/src/EditorFeatures/Core/Interactive/InteractiveSession.cs +++ b/src/EditorFeatures/Core/Interactive/InteractiveSession.cs @@ -13,10 +13,8 @@ using System.Linq; using System.Threading; using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.CodeAnalysis.ErrorReporting; using Microsoft.CodeAnalysis.Host; -using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Scripting.Hosting; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.TestHooks; @@ -27,17 +25,16 @@ namespace Microsoft.CodeAnalysis.Interactive; using InteractiveHost::Microsoft.CodeAnalysis.Interactive; +using Microsoft.CodeAnalysis.Collections; using RelativePathResolver = Scripting::Microsoft.CodeAnalysis.RelativePathResolver; internal sealed class InteractiveSession : IDisposable { public InteractiveHost Host { get; } - private readonly IThreadingContext _threadingContext; private readonly InteractiveEvaluatorLanguageInfoProvider _languageInfo; private readonly InteractiveWorkspace _workspace; private readonly ITextDocumentFactoryService _textDocumentFactoryService; - private readonly EditorOptionsService _editorOptionsService; private readonly CancellationTokenSource _shutdownCancellationSource; @@ -54,7 +51,7 @@ internal sealed class InteractiveSession : IDisposable // At the same time a code submission might be in progress. // If we left these operations run in parallel we might get into a state // inconsistent with the state of the host. - private readonly TaskQueue _taskQueue; + private readonly AsyncBatchingWorkQueue> _workQueue; private ProjectId? _lastSuccessfulSubmissionProjectId; private ProjectId? _currentSubmissionProjectId; @@ -76,21 +73,21 @@ internal sealed class InteractiveSession : IDisposable public InteractiveSession( InteractiveWorkspace workspace, - IThreadingContext threadingContext, IAsynchronousOperationListener listener, ITextDocumentFactoryService documentFactory, - EditorOptionsService editorOptionsService, InteractiveEvaluatorLanguageInfoProvider languageInfo, string initialWorkingDirectory) { _workspace = workspace; - _threadingContext = threadingContext; _languageInfo = languageInfo; _textDocumentFactoryService = documentFactory; - _editorOptionsService = editorOptionsService; - _taskQueue = new TaskQueue(listener, TaskScheduler.Default); _shutdownCancellationSource = new CancellationTokenSource(); + _workQueue = new( + TimeSpan.Zero, + ProcessWorkQueueAsync, + listener, + _shutdownCancellationSource.Token); // The following settings will apply when the REPL starts without .rsp file. // They are discarded once the REPL is reset. @@ -112,6 +109,31 @@ public void Dispose() Host.Dispose(); } + private async ValueTask ProcessWorkQueueAsync(ImmutableSegmentedList> list, CancellationToken cancellationToken) + { + foreach (var taskCreator in list) + { + cancellationToken.ThrowIfCancellationRequested(); + + Task task; + try + { + // Even producing the task may fail (due to synchronous execution of the delegate). Ensure we NFW in + // that case and move on. + task = taskCreator(); + } + catch (Exception ex) when (FatalError.ReportAndCatchUnlessCanceled(ex)) + { + continue; + } + + // Now that we have the task to process, ensure we always process the next piece of work, even if the + // current work fails. We still report this as an nfw here though: + _ = task.ReportNonFatalErrorAsync(); + await task.NoThrowAwaitableInternal(captureContext: false); + } + } + /// /// Invoked by when a new process initialization completes. /// @@ -119,7 +141,7 @@ private void ProcessInitialized(InteractiveHostPlatformInfo platformInfo, Intera { Contract.ThrowIfFalse(result.InitializationResult != null); - _ = _taskQueue.ScheduleTask(nameof(ProcessInitialized), () => + _workQueue.AddWork(() => { _workspace.ResetSolution(); @@ -140,7 +162,8 @@ private void ProcessInitialized(InteractiveHostPlatformInfo platformInfo, Intera } _pendingBuffers.Clear(); - }, _shutdownCancellationSource.Token); + return Task.CompletedTask; + }); } /// @@ -148,10 +171,12 @@ private void ProcessInitialized(InteractiveHostPlatformInfo platformInfo, Intera /// internal void AddSubmissionProject(ITextBuffer submissionBuffer) { - _taskQueue.ScheduleTask( - nameof(AddSubmissionProject), - () => AddSubmissionProjectNoLock(submissionBuffer, _languageInfo.LanguageName), - _shutdownCancellationSource.Token); + _workQueue.AddWork( + () => + { + AddSubmissionProjectNoLock(submissionBuffer, _languageInfo.LanguageName); + return Task.CompletedTask; + }); } private void AddSubmissionProjectNoLock(ITextBuffer submissionBuffer, string languageName) @@ -316,9 +341,10 @@ private Project CreateSubmissionProjectNoLock(Solution solution, ProjectId newSu /// Called once a code snippet is submitted. /// Followed by creation of a new language buffer and call to . /// - internal Task ExecuteCodeAsync(string text) + internal async Task ExecuteCodeAsync(string text) { - return _taskQueue.ScheduleTask(nameof(ExecuteCodeAsync), async () => + var returnValue = false; + _workQueue.AddWork(async () => { var result = await Host.ExecuteAsync(text).ConfigureAwait(false); if (result.Success) @@ -329,8 +355,11 @@ internal Task ExecuteCodeAsync(string text) UpdatePathsNoLock(result); } - return result.Success; - }, _shutdownCancellationSource.Token); + returnValue = result.Success; + }); + + await _workQueue.WaitUntilCurrentBatchCompletesAsync().ConfigureAwait(false); + return returnValue; } internal async Task ResetAsync(InteractiveHostOptions options) @@ -377,11 +406,13 @@ private static SourceReferenceResolver CreateSourceReferenceResolver(ImmutableAr public Task SetPathsAsync(ImmutableArray referenceSearchPaths, ImmutableArray sourceSearchPaths, string workingDirectory) { - return _taskQueue.ScheduleTask(nameof(ExecuteCodeAsync), async () => + _workQueue.AddWork(async () => { var result = await Host.SetPathsAsync(referenceSearchPaths, sourceSearchPaths, workingDirectory).ConfigureAwait(false); UpdatePathsNoLock(result); - }, _shutdownCancellationSource.Token); + }); + + return _workQueue.WaitUntilCurrentBatchCompletesAsync(); } private void UpdatePathsNoLock(RemoteExecutionResult result) diff --git a/src/VisualStudio/CSharp/Impl/Interactive/CSharpVsInteractiveWindowProvider.cs b/src/VisualStudio/CSharp/Impl/Interactive/CSharpVsInteractiveWindowProvider.cs index 53b16a0239d57..c0f0e8f5271c9 100644 --- a/src/VisualStudio/CSharp/Impl/Interactive/CSharpVsInteractiveWindowProvider.cs +++ b/src/VisualStudio/CSharp/Impl/Interactive/CSharpVsInteractiveWindowProvider.cs @@ -10,7 +10,6 @@ using Microsoft.CodeAnalysis.Host.Mef; using Microsoft.CodeAnalysis.Interactive; using Microsoft.CodeAnalysis.Internal.Log; -using Microsoft.CodeAnalysis.Options; using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.VisualStudio.InteractiveWindow.Commands; using Microsoft.VisualStudio.InteractiveWindow.Shell; @@ -29,7 +28,6 @@ internal sealed class CSharpVsInteractiveWindowProvider : VsInteractiveWindowPro private readonly IThreadingContext _threadingContext; private readonly IAsynchronousOperationListener _listener; private readonly ITextDocumentFactoryService _textDocumentFactoryService; - private readonly EditorOptionsService _editorOptionsService; [ImportingConstructor] [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)] @@ -43,14 +41,12 @@ public CSharpVsInteractiveWindowProvider( IInteractiveWindowCommandsFactory commandsFactory, [ImportMany] IInteractiveWindowCommand[] commands, ITextDocumentFactoryService textDocumentFactoryService, - EditorOptionsService editorOptionsService, VisualStudioWorkspace workspace) : base(serviceProvider, interactiveWindowFactory, classifierAggregator, contentTypeRegistry, commandsFactory, commands, workspace) { _threadingContext = threadingContext; _listener = listenerProvider.GetListener(FeatureAttribute.InteractiveEvaluator); _textDocumentFactoryService = textDocumentFactoryService; - _editorOptionsService = editorOptionsService; } protected override Guid LanguageServiceGuid => LanguageServiceGuids.CSharpLanguageServiceId; @@ -76,7 +72,6 @@ protected override CSharpInteractiveEvaluator CreateInteractiveEvaluator( CommandsFactory, Commands, _textDocumentFactoryService, - _editorOptionsService, CSharpInteractiveEvaluatorLanguageInfoProvider.Instance, Environment.GetFolderPath(Environment.SpecialFolder.UserProfile)); } From 48fd9e19d2a4a397d9314c4ff7819207c87c2797 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 16 Dec 2024 14:43:18 -0800 Subject: [PATCH 2/3] Simplify --- .../Core/Interactive/InteractiveSession.cs | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/src/EditorFeatures/Core/Interactive/InteractiveSession.cs b/src/EditorFeatures/Core/Interactive/InteractiveSession.cs index 661376af18a4a..8b4ee5b63197d 100644 --- a/src/EditorFeatures/Core/Interactive/InteractiveSession.cs +++ b/src/EditorFeatures/Core/Interactive/InteractiveSession.cs @@ -115,20 +115,10 @@ private async ValueTask ProcessWorkQueueAsync(ImmutableSegmentedList> { cancellationToken.ThrowIfCancellationRequested(); - Task task; - try - { - // Even producing the task may fail (due to synchronous execution of the delegate). Ensure we NFW in - // that case and move on. - task = taskCreator(); - } - catch (Exception ex) when (FatalError.ReportAndCatchUnlessCanceled(ex)) - { - continue; - } - - // Now that we have the task to process, ensure we always process the next piece of work, even if the - // current work fails. We still report this as an nfw here though: + // Kick off the task to run. This also ensures that if the taskCreator func throws synchronously, that we + // appropriately handle reporting it in ReportNonFatalErrorAsync. Also, ensure we always process the next + // piece of work, even if the current work fails. + var task = Task.Run(taskCreator, cancellationToken); _ = task.ReportNonFatalErrorAsync(); await task.NoThrowAwaitableInternal(captureContext: false); } From 2bc143c57e5286b395a516aa8ecc0bf25a34aa9b Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Mon, 16 Dec 2024 14:44:45 -0800 Subject: [PATCH 3/3] Delete unused code --- .../Core/Portable/Utilities/TaskQueue.cs | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/src/Workspaces/Core/Portable/Utilities/TaskQueue.cs b/src/Workspaces/Core/Portable/Utilities/TaskQueue.cs index a8b5380f6bae6..9354e302866c4 100644 --- a/src/Workspaces/Core/Portable/Utilities/TaskQueue.cs +++ b/src/Workspaces/Core/Portable/Utilities/TaskQueue.cs @@ -59,10 +59,6 @@ public Task ScheduleTask(string taskName, Func operation, CancellationT public Task ScheduleTask(string taskName, Func operation, CancellationToken cancellationToken) => EndOperation(BeginOperation(taskName), ScheduleTaskInProgress(operation, cancellationToken)); - /// - public Task ScheduleTask(string taskName, Func> operation, CancellationToken cancellationToken) - => EndOperation(BeginOperation(taskName), ScheduleTaskInProgress(operation, cancellationToken)); - /// /// Enqueue specified . /// Assumes has already been notified of its start and will be notified when it completes. @@ -103,17 +99,5 @@ private Task ScheduleTaskInProgress(Func operation, CancellationToken canc } } - /// - [PerformanceSensitive("https://developercommunity.visualstudio.com/content/problem/854696/changing-target-framework-takes-10-minutes-with-10.html", AllowCaptures = false)] - private Task ScheduleTaskInProgress(Func> operation, CancellationToken cancellationToken) - { - lock (_gate) - { - var task = LastScheduledTask.SafeContinueWithFromAsync(_ => operation(), cancellationToken, TaskContinuationOptions.None, Scheduler); - LastScheduledTask = task; - return task; - } - } - #pragma warning restore }