From 15ab434546a0a25f7d052a169ebe831d661a712e Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 30 May 2023 14:44:08 -0700 Subject: [PATCH 01/10] Update our 'delay until visible' logic to not throw cancellation exceptions --- .../NavigationBarController_ModelComputation.cs | 5 +++++ ...nchronousTaggerProvider.TagSource_ProduceTags.cs | 5 +++++ .../Core/Workspaces/ITextBufferVisibilityTracker.cs | 13 ++++++++++--- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs b/src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs index ed03a513b68c4..bdbc00b0ce277 100644 --- a/src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs +++ b/src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs @@ -68,6 +68,11 @@ internal partial class NavigationBarController await _visibilityTracker.DelayWhileNonVisibleAsync( _threadingContext, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).ConfigureAwait(false); + // DelayWhileNonVisibleAsync returns-early (but doesn't throw) in the event of cancellation. + // Gracefully bail out here to avoid excess cancellation exceptions from being thrown. + if (cancellationToken.IsCancellationRequested) + return null; + using (Logger.LogBlock(FunctionId.NavigationBar_ComputeModelAsync, cancellationToken)) { var items = await itemService.GetItemsAsync( diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index 16da65e2c6390..8fab1b6ec13fd 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -206,6 +206,11 @@ private async Task RecomputeTagsAsync(bool highPriority, CancellationToken cance { await _visibilityTracker.DelayWhileNonVisibleAsync( _dataSource.ThreadingContext, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).ConfigureAwait(true); + + // DelayWhileNonVisibleAsync returns-early (but doesn't throw) in the event of cancellation. + // Gracefully bail out here to avoid excess cancellation exceptions from being thrown. + if (cancellationToken.IsCancellationRequested) + return; } await _dataSource.ThreadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); diff --git a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs index 54b5c13e2b14e..1b4f1caf779e5 100644 --- a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs +++ b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs @@ -8,6 +8,7 @@ using Microsoft.CodeAnalysis.Editor.Shared.Utilities; using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Text.Editor; +using Microsoft.VisualStudio.Threading; namespace Microsoft.CodeAnalysis.Workspaces { @@ -37,8 +38,11 @@ internal interface ITextBufferVisibilityTracker internal static class ITextBufferVisibilityTrackerExtensions { /// - /// Waits the specified amount of time while the specified is not visible. If any - /// document visibility changes happen, the delay will cancel. + /// Waits the specified amount of time while the specified is not visible. If + /// any document visibility changes happen, the delay will cancel. Note: in the event of cancellation, the + /// returned task will transition to the state not the state. This is to prevent excess exception throws in the common case of + /// cancellation. Callers should check if they are canceled after this returns and bail out gracefully. /// public static async Task DelayWhileNonVisibleAsync( this ITextBufferVisibilityTracker? service, @@ -51,6 +55,9 @@ public static async Task DelayWhileNonVisibleAsync( if (service is null) return; + if (cancellationToken.IsCancellationRequested) + return; + await threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); if (service.IsVisible(subjectBuffer)) return; @@ -66,7 +73,7 @@ public static async Task DelayWhileNonVisibleAsync( { // Listen to when the active document changed so that we startup work on a document once it becomes visible. var delayTask = Task.Delay(timeSpan, cancellationToken); - await Task.WhenAny(delayTask, visibilityChangedTaskSource.Task).ConfigureAwait(false); + await Task.WhenAny(delayTask, visibilityChangedTaskSource.Task).NoThrowAwaitable(captureContext: false); } finally { From ba4d2801939a2d83524b63eb750c3539f0c31b63 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 31 May 2023 09:48:16 -0700 Subject: [PATCH 02/10] Continue normal semantics --- ...avigationBarController_ModelComputation.cs | 7 +- ...ousTaggerProvider.TagSource_ProduceTags.cs | 6 +- .../ITextBufferVisibilityTracker.cs | 79 ++++++++++++------- 3 files changed, 58 insertions(+), 34 deletions(-) diff --git a/src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs b/src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs index bdbc00b0ce277..5e687bc36a1c4 100644 --- a/src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs +++ b/src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs @@ -65,11 +65,12 @@ internal partial class NavigationBarController // If these are navbars for a file that isn't even visible, then avoid doing any unnecessary computation // work until far in the future (or if visibility changes). This ensures our non-visible docs do settle // once enough time has passed, while greatly reducing their impact on the system. + // + // Use NoThrow as this is a high source of cancellation exceptions. This avoids the exception and instead + // bails gracefully by checking below. await _visibilityTracker.DelayWhileNonVisibleAsync( - _threadingContext, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).ConfigureAwait(false); + _threadingContext, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).NoThrowAwaitable(false); - // DelayWhileNonVisibleAsync returns-early (but doesn't throw) in the event of cancellation. - // Gracefully bail out here to avoid excess cancellation exceptions from being thrown. if (cancellationToken.IsCancellationRequested) return null; diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index 8fab1b6ec13fd..9bc417f18f109 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -204,11 +204,11 @@ private async Task RecomputeTagsAsync(bool highPriority, CancellationToken cance // possible. if (!highPriority) { + // Use NoThrow as this is a high source of cancellation exceptions. This avoids the exception and instead + // bails gracefully by checking below. await _visibilityTracker.DelayWhileNonVisibleAsync( - _dataSource.ThreadingContext, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).ConfigureAwait(true); + _dataSource.ThreadingContext, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).NoThrowAwaitable(captureContext: true); - // DelayWhileNonVisibleAsync returns-early (but doesn't throw) in the event of cancellation. - // Gracefully bail out here to avoid excess cancellation exceptions from being thrown. if (cancellationToken.IsCancellationRequested) return; } diff --git a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs index 1b4f1caf779e5..bcd7847db4aa6 100644 --- a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs +++ b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs @@ -39,46 +39,69 @@ internal static class ITextBufferVisibilityTrackerExtensions { /// /// Waits the specified amount of time while the specified is not visible. If - /// any document visibility changes happen, the delay will cancel. Note: in the event of cancellation, the - /// returned task will transition to the state not the state. This is to prevent excess exception throws in the common case of - /// cancellation. Callers should check if they are canceled after this returns and bail out gracefully. + /// any document visibility changes happen, the delay will cancel. /// - public static async Task DelayWhileNonVisibleAsync( + public static Task DelayWhileNonVisibleAsync( this ITextBufferVisibilityTracker? service, IThreadingContext threadingContext, ITextBuffer subjectBuffer, TimeSpan timeSpan, CancellationToken cancellationToken) { - // Only add a delay if we have access to a service that will tell us when the buffer become visible or not. - if (service is null) - return; + var completionSource = new TaskCompletionSource(); + var underlyingTask = DelayWhileNonVisibleWorkerAsync(); + underlyingTask.ContinueWith(task => + { + if (cancellationToken.IsCancellationRequested || task.IsCanceled) + { + completionSource.TrySetCanceled(cancellationToken); + } + else if (task.IsFaulted) + { + completionSource.TrySetException(task.Exception); + } + else + { + completionSource.TrySetResult(true); + } + }, cancellationToken, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); - if (cancellationToken.IsCancellationRequested) - return; + return completionSource.Task; - await threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); - if (service.IsVisible(subjectBuffer)) - return; + // Normal delay logic, except that this does not throw in the event of cancellation, but instead returns + // gracefully. The above task continuation logic then ensures we return a canceled task without needing + // exceptions. + async Task DelayWhileNonVisibleWorkerAsync() + { + // Only add a delay if we have access to a service that will tell us when the buffer become visible or not. + if (service is null) + return; - // ensure we listen for visibility changes before checking. That way we don't have a race where we check - // something see it is not visible, but then do not hear about its visibility change because we've hooked up - // our event after that happens. - var visibilityChangedTaskSource = new TaskCompletionSource(); - var callback = void () => visibilityChangedTaskSource.TrySetResult(true); - service.RegisterForVisibilityChanges(subjectBuffer, callback); + if (cancellationToken.IsCancellationRequested) + return; - try - { - // Listen to when the active document changed so that we startup work on a document once it becomes visible. - var delayTask = Task.Delay(timeSpan, cancellationToken); - await Task.WhenAny(delayTask, visibilityChangedTaskSource.Task).NoThrowAwaitable(captureContext: false); - } - finally - { await threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); - service.UnregisterForVisibilityChanges(subjectBuffer, callback); + if (service.IsVisible(subjectBuffer)) + return; + + // ensure we listen for visibility changes before checking. That way we don't have a race where we check + // something see it is not visible, but then do not hear about its visibility change because we've hooked up + // our event after that happens. + var visibilityChangedTaskSource = new TaskCompletionSource(); + var callback = void () => visibilityChangedTaskSource.TrySetResult(true); + service.RegisterForVisibilityChanges(subjectBuffer, callback); + + try + { + // Listen to when the active document changed so that we startup work on a document once it becomes visible. + var delayTask = Task.Delay(timeSpan, cancellationToken); + await Task.WhenAny(delayTask, visibilityChangedTaskSource.Task).NoThrowAwaitable(captureContext: false); + } + finally + { + await threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); + service.UnregisterForVisibilityChanges(subjectBuffer, callback); + } } } } From a7a1aa77f5d47a8ce82a09d6a79fe89363b9be6a Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 31 May 2023 09:56:10 -0700 Subject: [PATCH 03/10] Simplify --- .../Core/Workspaces/ITextBufferVisibilityTracker.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs index bcd7847db4aa6..b5c1b48e3de09 100644 --- a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs +++ b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs @@ -49,10 +49,9 @@ public static Task DelayWhileNonVisibleAsync( CancellationToken cancellationToken) { var completionSource = new TaskCompletionSource(); - var underlyingTask = DelayWhileNonVisibleWorkerAsync(); - underlyingTask.ContinueWith(task => + DelayWhileNonVisibleWorkerAsync().ContinueWith(task => { - if (cancellationToken.IsCancellationRequested || task.IsCanceled) + if (cancellationToken.IsCancellationRequested) { completionSource.TrySetCanceled(cancellationToken); } From 8b35aa9b51db9594488b7d1c6fe2090930a7356f Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 31 May 2023 09:59:09 -0700 Subject: [PATCH 04/10] Remove switch --- .../Core/Workspaces/ITextBufferVisibilityTracker.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs index b5c1b48e3de09..e6c22cfabea9f 100644 --- a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs +++ b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs @@ -94,11 +94,10 @@ async Task DelayWhileNonVisibleWorkerAsync() { // Listen to when the active document changed so that we startup work on a document once it becomes visible. var delayTask = Task.Delay(timeSpan, cancellationToken); - await Task.WhenAny(delayTask, visibilityChangedTaskSource.Task).NoThrowAwaitable(captureContext: false); + await Task.WhenAny(delayTask, visibilityChangedTaskSource.Task).NoThrowAwaitable(captureContext: true); } finally { - await threadingContext.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); service.UnregisterForVisibilityChanges(subjectBuffer, callback); } } From 81a4a204c0fa7cedeaa1835badcb44c2a732e5fa Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 7 Jun 2023 12:02:58 -0700 Subject: [PATCH 05/10] Docs --- .../Core/Workspaces/ITextBufferVisibilityTracker.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs index 2f9b0f8a157b3..95804ab339fa2 100644 --- a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs +++ b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs @@ -48,20 +48,24 @@ public static Task DelayWhileNonVisibleAsync( TimeSpan timeSpan, CancellationToken cancellationToken) { + // Because cancellation is both expensive, and a super common thing to occur while we're delaying the caller + // until visbility, we special case the implementation here and transition a TaskCompletionSource to the + // canceled state explicitly, rather than throwing a cancellation exception. + var completionSource = new TaskCompletionSource(); DelayWhileNonVisibleWorkerAsync().ContinueWith(task => { if (cancellationToken.IsCancellationRequested) { - completionSource.TrySetCanceled(cancellationToken); + completionSource.SetCanceled(cancellationToken); } else if (task.IsFaulted) { - completionSource.TrySetException(task.Exception!); + completionSource.SetException(task.Exception!); } else { - completionSource.TrySetResult(true); + completionSource.SetResult(true); } }, cancellationToken, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); From 48b8cd79bbeb1d993d9d911654e779183b65ded3 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Wed, 7 Jun 2023 12:24:43 -0700 Subject: [PATCH 06/10] Try --- .../Core/Workspaces/ITextBufferVisibilityTracker.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs index 95804ab339fa2..181bc4f9d473a 100644 --- a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs +++ b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs @@ -57,15 +57,15 @@ public static Task DelayWhileNonVisibleAsync( { if (cancellationToken.IsCancellationRequested) { - completionSource.SetCanceled(cancellationToken); + completionSource.TrySetCanceled(cancellationToken); } else if (task.IsFaulted) { - completionSource.SetException(task.Exception!); + completionSource.TrySetException(task.Exception!); } else { - completionSource.SetResult(true); + completionSource.TrySetResult(true); } }, cancellationToken, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); From cb04373b1cdb4787ddfeeeb98b5e5d5514bb9332 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 8 Jun 2023 13:51:17 -0700 Subject: [PATCH 07/10] Simplify --- .../ITextBufferVisibilityTracker.cs | 26 +++++-------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs index 45f3aa9f166f2..a41fbc0ef12d4 100644 --- a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs +++ b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs @@ -49,27 +49,13 @@ public static Task DelayWhileNonVisibleAsync( CancellationToken cancellationToken) { // Because cancellation is both expensive, and a super common thing to occur while we're delaying the caller - // until visibility, we special case the implementation here and transition a TaskCompletionSource to the - // canceled state explicitly, rather than throwing a cancellation exception. + // until visibility, we special case the implementation here and transition to the canceled state + // explicitly, rather than throwing a cancellation exception. - var completionSource = new TaskCompletionSource(); - DelayWhileNonVisibleWorkerAsync().ContinueWith(task => - { - if (cancellationToken.IsCancellationRequested) - { - completionSource.TrySetCanceled(cancellationToken); - } - else if (task.IsFaulted) - { - completionSource.TrySetException(task.Exception!); - } - else - { - completionSource.TrySetResult(true); - } - }, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); - - return completionSource.Task; + var taskOfTask = DelayWhileNonVisibleWorkerAsync().ContinueWith( + task => cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : task, + CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); + return taskOfTask.Unwrap(); // Normal delay logic, except that this does not throw in the event of cancellation, but instead returns // gracefully. The above task continuation logic then ensures we return a canceled task without needing From dcc66a6a8fbe39625e263e464ff1ebf93858b562 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Thu, 8 Jun 2023 13:56:05 -0700 Subject: [PATCH 08/10] Use expedited delay --- .../NavigationBarController_ModelComputation.cs | 2 +- ...ynchronousTaggerProvider.TagSource_ProduceTags.cs | 2 +- .../Core/Workspaces/ITextBufferVisibilityTracker.cs | 12 +++++++----- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs b/src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs index 5e687bc36a1c4..c24e57a8153bc 100644 --- a/src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs +++ b/src/EditorFeatures/Core/NavigationBar/NavigationBarController_ModelComputation.cs @@ -69,7 +69,7 @@ internal partial class NavigationBarController // Use NoThrow as this is a high source of cancellation exceptions. This avoids the exception and instead // bails gracefully by checking below. await _visibilityTracker.DelayWhileNonVisibleAsync( - _threadingContext, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).NoThrowAwaitable(false); + _threadingContext, _asyncListener, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).NoThrowAwaitable(false); if (cancellationToken.IsCancellationRequested) return null; diff --git a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs index 9bc417f18f109..e72a42d00dee8 100644 --- a/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs +++ b/src/EditorFeatures/Core/Tagging/AbstractAsynchronousTaggerProvider.TagSource_ProduceTags.cs @@ -207,7 +207,7 @@ private async Task RecomputeTagsAsync(bool highPriority, CancellationToken cance // Use NoThrow as this is a high source of cancellation exceptions. This avoids the exception and instead // bails gracefully by checking below. await _visibilityTracker.DelayWhileNonVisibleAsync( - _dataSource.ThreadingContext, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).NoThrowAwaitable(captureContext: true); + _dataSource.ThreadingContext, _dataSource.AsyncListener, _subjectBuffer, DelayTimeSpan.NonFocus, cancellationToken).NoThrowAwaitable(captureContext: true); if (cancellationToken.IsCancellationRequested) return; diff --git a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs index a41fbc0ef12d4..d410e721d4ac9 100644 --- a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs +++ b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs @@ -6,6 +6,7 @@ using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Editor.Shared.Utilities; +using Microsoft.CodeAnalysis.Shared.TestHooks; using Microsoft.VisualStudio.Text; using Microsoft.VisualStudio.Text.Editor; using Microsoft.VisualStudio.Threading; @@ -44,10 +45,15 @@ internal static class ITextBufferVisibilityTrackerExtensions public static Task DelayWhileNonVisibleAsync( this ITextBufferVisibilityTracker? service, IThreadingContext threadingContext, + IAsynchronousOperationListener listener, ITextBuffer subjectBuffer, TimeSpan timeSpan, CancellationToken cancellationToken) { + // Only add a delay if we have access to a service that will tell us when the buffer become visible or not. + if (service is null) + return Task.CompletedTask; + // Because cancellation is both expensive, and a super common thing to occur while we're delaying the caller // until visibility, we special case the implementation here and transition to the canceled state // explicitly, rather than throwing a cancellation exception. @@ -62,10 +68,6 @@ public static Task DelayWhileNonVisibleAsync( // exceptions. async Task DelayWhileNonVisibleWorkerAsync() { - // Only add a delay if we have access to a service that will tell us when the buffer become visible or not. - if (service is null) - return; - if (cancellationToken.IsCancellationRequested) return; @@ -83,7 +85,7 @@ async Task DelayWhileNonVisibleWorkerAsync() try { // Listen to when the active document changed so that we startup work on a document once it becomes visible. - var delayTask = Task.Delay(timeSpan, cancellationToken); + var delayTask = listener.Delay(timeSpan, cancellationToken); await Task.WhenAny(delayTask, visibilityChangedTaskSource.Task).NoThrowAwaitable(captureContext: true); } finally From 179c4c38098a41507d79843a25c702e14a10d592 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 9 Jun 2023 09:42:37 -0700 Subject: [PATCH 09/10] Add sync test --- .../Core/Workspaces/ITextBufferVisibilityTracker.cs | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs index d410e721d4ac9..8884f15fed97c 100644 --- a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs +++ b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs @@ -58,8 +58,17 @@ public static Task DelayWhileNonVisibleAsync( // until visibility, we special case the implementation here and transition to the canceled state // explicitly, rather than throwing a cancellation exception. - var taskOfTask = DelayWhileNonVisibleWorkerAsync().ContinueWith( - task => cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : task, + var delayTask = DelayWhileNonVisibleWorkerAsync(); + + // it's very reasonable for the delay-task to complete synchronously (we've already been canceled, or the + // buffer is already visible. So fast path that out. + if (delayTask.IsCompleted) + return delayTask; + + var taskOfTask = delayTask.ContinueWith( + // Convert a successfully completed task when we were canceled to a canceled task. Otherwise, return + // the faulted or non-canceled task as is. + task => !task.IsFaulted && cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : task, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); return taskOfTask.Unwrap(); From 3bc0a1654b51ec5b1e1ef21577c055258e3c1bc0 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Fri, 9 Jun 2023 09:44:04 -0700 Subject: [PATCH 10/10] Simplify --- .../Core/Workspaces/ITextBufferVisibilityTracker.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs index 8884f15fed97c..2b4098c123adb 100644 --- a/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs +++ b/src/EditorFeatures/Core/Workspaces/ITextBufferVisibilityTracker.cs @@ -68,7 +68,7 @@ public static Task DelayWhileNonVisibleAsync( var taskOfTask = delayTask.ContinueWith( // Convert a successfully completed task when we were canceled to a canceled task. Otherwise, return // the faulted or non-canceled task as is. - task => !task.IsFaulted && cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : task, + task => task.Status == TaskStatus.RanToCompletion && cancellationToken.IsCancellationRequested ? Task.FromCanceled(cancellationToken) : task, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Default); return taskOfTask.Unwrap();