From 530fd6d13d8de2fdffb21a02adf320d483cb94e4 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Wed, 20 Mar 2024 11:07:37 -0700 Subject: [PATCH 01/17] Copy AsyncBatchingWorkQueue from Roslyn Roslyn's `AsyncBatchingWorkQueue` is a battle-hardened utility that provides a robust mechanism to do work in batches without blocking. --- eng/Versions.props | 1 + .../TaskDelayScheduler.cs | 2 + ....AspNetCore.Razor.ProjectEngineHost.csproj | 1 + .../Threading}/CancellationSeries.cs | 60 +++- .../Threading/TaskExtensions.cs | 73 +++++ .../Utilities/AsyncBatchingWorkQueue.cs | 27 ++ .../Utilities/AsyncBatchingWorkQueue`1.cs | 43 +++ .../Utilities/AsyncBatchingWorkQueue`2.cs | 292 ++++++++++++++++++ .../Assumed.cs | 40 ++- .../Resources/SR.resx | 13 +- .../Resources/xlf/SR.cs.xlf | 21 +- .../Resources/xlf/SR.de.xlf | 21 +- .../Resources/xlf/SR.es.xlf | 21 +- .../Resources/xlf/SR.fr.xlf | 21 +- .../Resources/xlf/SR.it.xlf | 21 +- .../Resources/xlf/SR.ja.xlf | 21 +- .../Resources/xlf/SR.ko.xlf | 21 +- .../Resources/xlf/SR.pl.xlf | 21 +- .../Resources/xlf/SR.pt-BR.xlf | 21 +- .../Resources/xlf/SR.ru.xlf | 21 +- .../Resources/xlf/SR.tr.xlf | 21 +- .../Resources/xlf/SR.zh-Hans.xlf | 21 +- .../Resources/xlf/SR.zh-Hant.xlf | 21 +- .../VoidResult.cs | 21 ++ 24 files changed, 793 insertions(+), 53 deletions(-) rename src/Razor/src/{Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace => Microsoft.AspNetCore.Razor.ProjectEngineHost/Threading}/CancellationSeries.cs (51%) create mode 100644 src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Threading/TaskExtensions.cs create mode 100644 src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue.cs create mode 100644 src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`1.cs create mode 100644 src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`2.cs create mode 100644 src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/VoidResult.cs diff --git a/eng/Versions.props b/eng/Versions.props index b64226dd2ca..9a351fadb17 100644 --- a/eng/Versions.props +++ b/eng/Versions.props @@ -151,6 +151,7 @@ 4.8.0 2.17.11 4.3.0 + 4.5.4 3.11.0-beta1.24170.2 $(Tooling_MicrosoftCodeAnalysisAnalyzersPackageVersion) $(Tooling_MicrosoftCodeAnalysisAnalyzersPackageVersion) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace/TaskDelayScheduler.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace/TaskDelayScheduler.cs index 06dba9c2daa..241a62ff06f 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace/TaskDelayScheduler.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace/TaskDelayScheduler.cs @@ -1,6 +1,8 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT license. See License.txt in the project root for license information. +using Microsoft.AspNetCore.Razor.Threading; + namespace Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace; // Copied from https://github.com/dotnet/project-system/blob/e4db47666e0a49f6c38e701f8630dbc31380fb64/src/Microsoft.VisualStudio.ProjectSystem.Managed/Threading/Tasks/TaskDelayScheduler.cs diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Microsoft.AspNetCore.Razor.ProjectEngineHost.csproj b/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Microsoft.AspNetCore.Razor.ProjectEngineHost.csproj index 6c21224ce5d..6673f340afe 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Microsoft.AspNetCore.Razor.ProjectEngineHost.csproj +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Microsoft.AspNetCore.Razor.ProjectEngineHost.csproj @@ -14,6 +14,7 @@ + diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace/CancellationSeries.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Threading/CancellationSeries.cs similarity index 51% rename from src/Razor/src/Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace/CancellationSeries.cs rename to src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Threading/CancellationSeries.cs index 41b82067a73..5de367394ee 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace/CancellationSeries.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Threading/CancellationSeries.cs @@ -1,13 +1,31 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT license. See License.txt in the project root for license information. -namespace Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace; +using System; +using System.Threading; -// Copied from https://github.com/dotnet/project-system/blob/e4db47666e0a49f6c38e701f8630dbc31380fb64/src/Microsoft.VisualStudio.ProjectSystem.Managed/Threading/Tasks/CancellationSeries.cs +namespace Microsoft.AspNetCore.Razor.Threading; +// NOTE: This code is copied from dotnet/roslyn: +// https://github.com/dotnet/roslyn/blob/98cd097bf122677378692ebe952b71ab6e5bb013/src/Workspaces/Core/Portable/Utilities/CancellationSeries.cs +// +// However, it was originally derived from an implementation in dotnet/project-system: +// https://github.com/dotnet/project-system/blob/bdf69d5420ec8d894f5bf4c3d4692900b7f2479c/src/Microsoft.VisualStudio.ProjectSystem.Managed/Threading/Tasks/CancellationSeries.cs +// +// See the commentary in https://github.com/dotnet/roslyn/pull/50156 for notes on incorporating changes made to the +// reference implementation. + +/// +/// Produces a series of objects such that requesting a new token +/// causes the previously issued token to be cancelled. +/// +/// +/// Consuming code is responsible for managing overlapping asynchronous operations. +/// This class has a lock-free implementation to minimise latency and contention. +/// internal sealed class CancellationSeries : IDisposable { - private CancellationTokenSource? _cts = new(); + private CancellationTokenSource? _cts; private readonly CancellationToken _superToken; @@ -16,11 +34,21 @@ internal sealed class CancellationSeries : IDisposable /// /// An optional cancellation token that, when cancelled, cancels the last /// issued token and causes any subsequent tokens to be issued in a cancelled state. - public CancellationSeries(CancellationToken token) + public CancellationSeries(CancellationToken token = default) { + // Initialize with a pre-cancelled source to ensure HasActiveToken has the correct state + _cts = new CancellationTokenSource(); + _cts.Cancel(); + _superToken = token; } + /// + /// Determines if the cancellation series has an active token which has not been cancelled. + /// + public bool HasActiveToken + => _cts is { IsCancellationRequested: false }; + /// /// Creates the next in the series, ensuring the last issued /// token (if any) is cancelled first. @@ -37,7 +65,7 @@ public CancellationSeries(CancellationToken token) /// /// /// This object has been disposed. - public CancellationToken CreateNext(CancellationToken token) + public CancellationToken CreateNext(CancellationToken token = default) { var nextSource = CancellationTokenSource.CreateLinkedTokenSource(token, _superToken); @@ -46,9 +74,25 @@ public CancellationToken CreateNext(CancellationToken token) // This way we would return a cancelled token, which is reasonable. var nextToken = nextSource.Token; - var priorSource = Interlocked.Exchange(ref _cts, nextSource); + // The following block is identical to Interlocked.Exchange, except no replacement is made if the current + // field value is null (latch on null). This ensures state is not corrupted if CreateNext is called after + // the object is disposed. + var priorSource = Volatile.Read(ref _cts); + while (priorSource is not null) + { + var candidate = Interlocked.CompareExchange(ref _cts, nextSource, priorSource); + + if (candidate == priorSource) + { + break; + } + else + { + priorSource = candidate; + } + } - if (priorSource is null) + if (priorSource == null) { nextSource.Dispose(); @@ -73,7 +117,7 @@ public void Dispose() { var source = Interlocked.Exchange(ref _cts, null); - if (source is null) + if (source == null) { // Already disposed return; diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Threading/TaskExtensions.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Threading/TaskExtensions.cs new file mode 100644 index 00000000000..db3db984e7a --- /dev/null +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Threading/TaskExtensions.cs @@ -0,0 +1,73 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System.Threading.Tasks; + +namespace Microsoft.AspNetCore.Razor.Threading; + +internal static class TaskExtensions +{ + /// + /// Asserts the passed has already been completed. + /// + /// + /// This is useful for a specific case: sometimes you might be calling an API that is "sometimes" async, and you're + /// calling it from a synchronous method where you know it should have completed synchronously. This is an easy + /// way to assert that while silencing any compiler complaints. + /// + public static void VerifyCompleted(this Task task) + { + Assumed.True(task.IsCompleted); + + // Propagate any exceptions that may have been thrown. + task.GetAwaiter().GetResult(); + } + + /// + /// Asserts the passed has already been completed. + /// + /// + /// This is useful for a specific case: sometimes you might be calling an API that is "sometimes" async, and you're + /// calling it from a synchronous method where you know it should have completed synchronously. This is an easy + /// way to assert that while silencing any compiler complaints. + /// + public static TResult VerifyCompleted(this Task task) + { + Assumed.True(task.IsCompleted); + + // Propagate any exceptions that may have been thrown. + return task.GetAwaiter().GetResult(); + } + + /// + /// Asserts the passed has already been completed. + /// + /// + /// This is useful for a specific case: sometimes you might be calling an API that is "sometimes" async, and you're + /// calling it from a synchronous method where you know it should have completed synchronously. This is an easy + /// way to assert that while silencing any compiler complaints. + /// + public static void VerifyCompleted(this ValueTask task) + { + Assumed.True(task.IsCompleted); + + // Propagate any exceptions that may have been thrown. + task.GetAwaiter().GetResult(); + } + + /// + /// Asserts the passed has already been completed. + /// + /// + /// This is useful for a specific case: sometimes you might be calling an API that is "sometimes" async, and you're + /// calling it from a synchronous method where you know it should have completed synchronously. This is an easy + /// way to assert that while silencing any compiler complaints. + /// + public static TResult VerifyCompleted(this ValueTask task) + { + Assumed.True(task.IsCompleted); + + // Propagate any exceptions that may have been thrown. + return task.GetAwaiter().GetResult(); + } +} diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue.cs new file mode 100644 index 00000000000..4ff741e17cf --- /dev/null +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue.cs @@ -0,0 +1,27 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Razor; + +namespace Microsoft.CodeAnalysis.Razor.Utilities; + +// NOTE: This code is copied and modified slightly from dotnet/roslyn: +// https://github.com/dotnet/roslyn/blob/98cd097bf122677378692ebe952b71ab6e5bb013/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue%600.cs + +/// +internal class AsyncBatchingWorkQueue( + TimeSpan delay, + Func processBatchAsync, + CancellationToken cancellationToken) : AsyncBatchingWorkQueue(delay, Convert(processBatchAsync), EqualityComparer.Default, cancellationToken) +{ + private static Func, CancellationToken, ValueTask> Convert(Func processBatchAsync) + => (items, ct) => processBatchAsync(ct); + + public void AddWork(bool cancelExistingWork = false) + => base.AddWork(default(VoidResult), cancelExistingWork); +} diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`1.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`1.cs new file mode 100644 index 00000000000..7196793896f --- /dev/null +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`1.cs @@ -0,0 +1,43 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Razor; + +namespace Microsoft.CodeAnalysis.Razor.Utilities; + +// NOTE: This code is copied and modified slightly from dotnet/roslyn: +// https://github.com/dotnet/roslyn/blob/98cd097bf122677378692ebe952b71ab6e5bb013/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue%601.cs + +/// +internal class AsyncBatchingWorkQueue( + TimeSpan delay, + Func, CancellationToken, ValueTask> processBatchAsync, + IEqualityComparer? equalityComparer, + CancellationToken cancellationToken) : AsyncBatchingWorkQueue(delay, Convert(processBatchAsync), equalityComparer, cancellationToken) +{ + public AsyncBatchingWorkQueue( + TimeSpan delay, + Func, CancellationToken, ValueTask> processBatchAsync, + CancellationToken cancellationToken) + : this(delay, + processBatchAsync, + equalityComparer: null, + cancellationToken) + { + } + + private static Func, CancellationToken, ValueTask> Convert(Func, CancellationToken, ValueTask> processBatchAsync) + => async (items, ct) => + { + await processBatchAsync(items, ct).ConfigureAwait(false); + return default; + }; + + public new Task WaitUntilCurrentBatchCompletesAsync() + => base.WaitUntilCurrentBatchCompletesAsync(); +} diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`2.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`2.cs new file mode 100644 index 00000000000..4fc0dd9100e --- /dev/null +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`2.cs @@ -0,0 +1,292 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Collections.Immutable; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Razor; +using Microsoft.AspNetCore.Razor.PooledObjects; +using Microsoft.AspNetCore.Razor.Threading; +using Microsoft.VisualStudio.Threading; + +namespace Microsoft.CodeAnalysis.Razor.Utilities; + +// NOTE: This code is derived from dotnet/roslyn: +// https://github.com/dotnet/roslyn/blob/98cd097bf122677378692ebe952b71ab6e5bb013/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue%602.cs + +/// +/// A queue where items can be added to to be processed in batches after some delay has passed. When processing +/// happens, all the items added since the last processing point will be passed along to be worked on. Rounds of +/// processing happen serially, only starting up after a previous round has completed. +/// +/// Failure to complete a particular batch (either due to cancellation or some faulting error) will not prevent +/// further batches from executing. The only thing that will permanently stop this queue from processing items is if +/// the passed to the constructor switches to . +/// +/// +internal class AsyncBatchingWorkQueue +{ + /// + /// Delay we wait after finishing the processing of one batch and starting up on then. + /// + private readonly TimeSpan _delay; + + /// + /// Equality comparer uses to dedupe items if present. + /// + private readonly IEqualityComparer? _equalityComparer; + + /// + /// Callback to actually perform the processing of the next batch of work. + /// + private readonly Func, CancellationToken, ValueTask> _processBatchAsync; + + /// + /// Cancellation token controlling the entire queue. Once this is triggered, we don't want to do any more work + /// at all. + /// + private readonly CancellationToken _entireQueueCancellationToken; + + /// + /// Cancellation series we use so we can cancel individual batches of work if requested. The client of the + /// queue can cancel existing work by either calling directly, or passing to . Work in the queue that has not started will be + /// immediately discarded. The cancellation token passed to will be triggered + /// allowing the client callback to cooperatively cancel the current batch of work it is performing. + /// + private readonly CancellationSeries _cancellationSeries; + + #region protected by lock + + /// + /// Lock we will use to ensure the remainder of these fields can be accessed in a thread-safe + /// manner. When work is added we'll place the data into . + /// We'll then kick of a task to process this in the future if we don't already have an + /// existing task in flight for that. + /// + private readonly object _gate = new(); + + /// + /// Data added that we want to process in our next update task. + /// + private readonly ImmutableArray.Builder _nextBatch = ImmutableArray.CreateBuilder(); + + /// + /// CancellationToken controlling the next batch of items to execute. + /// + private CancellationToken _nextBatchCancellationToken; + + /// + /// Used if is present to ensure only unique items are added to . + /// + private readonly HashSet _uniqueItems; + + /// + /// Task kicked off to do the next batch of processing of . These + /// tasks form a chain so that the next task only processes when the previous one completes. + /// + private Task _updateTask = Task.FromResult(default(TResult)); + + /// + /// Whether or not there is an existing task in flight that will process the current batch + /// of . If there is an existing in flight task, we don't need to + /// kick off a new one if we receive more work before it runs. + /// + private bool _taskInFlight = false; + + #endregion + + public AsyncBatchingWorkQueue( + TimeSpan delay, + Func, CancellationToken, ValueTask> processBatchAsync, + IEqualityComparer? equalityComparer, + CancellationToken cancellationToken) + { + _delay = delay; + _processBatchAsync = processBatchAsync; + _equalityComparer = equalityComparer; + _entireQueueCancellationToken = cancellationToken; + + _uniqueItems = new HashSet(equalityComparer); + + // Combine with the queue cancellation token so that any batch is controlled by that token as well. + _cancellationSeries = new CancellationSeries(_entireQueueCancellationToken); + CancelExistingWork(); + } + + /// + /// Cancels any outstanding work in this queue. Work that has not yet started will never run. Work that is in + /// progress will request cancellation in a standard best effort fashion. + /// + public void CancelExistingWork() + { + lock (_gate) + { + // Cancel out the current executing batch, and create a new token for the next batch. + _nextBatchCancellationToken = _cancellationSeries.CreateNext(); + + // Clear out the existing items that haven't run yet. There is no point keeping them around now. + _nextBatch.Clear(); + _uniqueItems.Clear(); + } + } + + public void AddWork(TItem item, bool cancelExistingWork = false) + { + using var _ = ArrayBuilderPool.GetPooledObject(out var items); + items.Add(item); + + AddWork(items, cancelExistingWork); + } + + public void AddWork(IEnumerable items, bool cancelExistingWork = false) + { + // Don't do any more work if we've been asked to shutdown. + if (_entireQueueCancellationToken.IsCancellationRequested) + { + return; + } + + lock (_gate) + { + // if we were asked to cancel the prior set of items, do so now. + if (cancelExistingWork) + { + CancelExistingWork(); + } + + // add our work to the set we'll process in the next batch. + AddItemsToBatch(items); + + if (!_taskInFlight) + { + // No in-flight task. Kick one off to process these messages a second from now. + // We always attach the task to the previous one so that notifications to the ui + // follow the same order as the notification the OOP server sent to us. + _updateTask = ContinueAfterDelayAsync(_updateTask); + _taskInFlight = true; + } + } + + return; + + void AddItemsToBatch(IEnumerable items) + { + // no equality comparer. We want to process all items. + if (_equalityComparer == null) + { + _nextBatch.AddRange(items); + return; + } + + // We're deduping items. Only add the item if it's the first time we've seen it. + foreach (var item in items) + { + if (_uniqueItems.Add(item)) + { + _nextBatch.Add(item); + } + } + } + + async Task ContinueAfterDelayAsync(Task lastTask) + { + // Await the previous item in the task chain in a non-throwing fashion. Regardless of whether that last + // task completed successfully or not, we want to move onto the next batch. + await lastTask.NoThrowAwaitable(captureContext: false); + + // If we were asked to shutdown, immediately transition to the canceled state without doing any more work. + _entireQueueCancellationToken.ThrowIfCancellationRequested(); + + // Ensure that we always yield the current thread this is necessary for correctness as we are called + // inside a lock that _taskInFlight to true. We must ensure that the work to process the next batch + // must be on another thread that runs afterwards, can only grab the thread once we release it and will + // then reset that bool back to false + await Task.Yield().ConfigureAwait(false); + await Task.Delay(_delay, _entireQueueCancellationToken).ConfigureAwait(false); + return await ProcessNextBatchAsync().ConfigureAwait(false); + } + } + + /// + /// Waits until the current batch of work completes and returns the last value successfully computed from . If the last canceled or failed, then a + /// corresponding canceled or faulted task will be returned that propagates that outwards. + /// + public Task WaitUntilCurrentBatchCompletesAsync() + { + lock (_gate) + { +#pragma warning disable VSTHRD003 // Avoid awaiting foreign Tasks + return _updateTask; +#pragma warning restore VSTHRD003 // Avoid awaiting foreign Tasks + } + } + + private async ValueTask ProcessNextBatchAsync() + { + _entireQueueCancellationToken.ThrowIfCancellationRequested(); + try + { + var (nextBatch, batchCancellationToken) = GetNextBatchAndResetQueue(); + + // We may have no items if the entire batch was canceled (and no new work was added). + if (nextBatch.IsEmpty) + { + return default; + } + + var batchResultTask = _processBatchAsync(nextBatch, batchCancellationToken).Preserve(); + await batchResultTask.NoThrowAwaitable(false); + + if (batchResultTask.IsCompletedSuccessfully) + { + // The VS threading library analyzers warn here because we're accessing Result + // directly, which can be block. However, this is safe because we've already awaited + // the task and verified that it completed successfully. +#pragma warning disable VSTHRD103 + return batchResultTask.Result; +#pragma warning restore VSTHRD103 + } + else if (batchResultTask.IsCanceled && !_entireQueueCancellationToken.IsCancellationRequested) + { + // Don't bubble up cancellation to the queue for the nested batch cancellation. Just because we decided + // to cancel this batch isn't something that should stop processing further batches. + return default; + } + else + { + // Realize the completed result to force the exception to be thrown. + batchResultTask.VerifyCompleted(); + + return Assumed.Unreachable(); + } + } + catch (OperationCanceledException) + { + // Silently continue to allow the next batch to be processed. + } + + return Assumed.Unreachable(); + } + + private (ImmutableArray items, CancellationToken batchCancellationToken) GetNextBatchAndResetQueue() + { + lock (_gate) + { + var nextBatch = _nextBatch.ToImmutable(); + + // mark there being no existing update task so that the next OOP notification will + // kick one off. + _nextBatch.Clear(); + _uniqueItems.Clear(); + _taskInFlight = false; + + return (nextBatch, _nextBatchCancellationToken); + } + } +} diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Assumed.cs b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Assumed.cs index ae15edc66bb..8493bbb7686 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Assumed.cs +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Assumed.cs @@ -2,6 +2,7 @@ // Licensed under the MIT license. See License.txt in the project root for license information. using System; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; @@ -9,14 +10,35 @@ namespace Microsoft.AspNetCore.Razor; internal static class Assumed { + public static void False( + [DoesNotReturnIf(true)] bool condition, + [CallerFilePath] string? path = null, + [CallerLineNumber] int line = 0) + { + if (condition) + { + ThrowInvalidOperation(SR.Expected_condition_to_be_false, path, line); + } + } + + public static void True( + [DoesNotReturnIf(false)] bool condition, + [CallerFilePath] string? path = null, + [CallerLineNumber] int line = 0) + { + if (!condition) + { + ThrowInvalidOperation(SR.Expected_condition_to_be_true, path, line); + } + } + /// /// Can be called at points that are assumed to be unreachable at runtime. /// /// [DoesNotReturn] public static void Unreachable([CallerFilePath] string? path = null, [CallerLineNumber] int line = 0) - => throw new InvalidOperationException( - SR.FormatThis_program_location_is_thought_to_be_unreachable_File_0_Line_1(path, line)); + => ThrowInvalidOperation(SR.This_program_location_is_thought_to_be_unreachable, path, line); /// /// Can be called at points that are assumed to be unreachable at runtime. @@ -24,6 +46,16 @@ public static void Unreachable([CallerFilePath] string? path = null, [CallerLine /// [DoesNotReturn] public static T Unreachable([CallerFilePath] string? path = null, [CallerLineNumber] int line = 0) - => throw new InvalidOperationException( - SR.FormatThis_program_location_is_thought_to_be_unreachable_File_0_Line_1(path, line)); + { + ThrowInvalidOperation(SR.This_program_location_is_thought_to_be_unreachable, path, line); + return default; + } + + [DebuggerHidden] + [DoesNotReturn] + [MethodImpl(MethodImplOptions.NoInlining)] + private static void ThrowInvalidOperation(string message, string? path, int line) + { + throw new InvalidOperationException(message); + } } diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/SR.resx b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/SR.resx index 47d58ab81a8..d9428e9c900 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/SR.resx +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/SR.resx @@ -126,11 +126,20 @@ Destination is too short. + + Expected condition to be false. + + + Expected condition to be true. + + + File='{0}', Line={1} + Non-negative number required. - - This program location is thought to be unreachable. File='{0}', Line={1} + + This program location is thought to be unreachable. Unsupported type: '{0}'. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.cs.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.cs.xlf index 81dff74d3c8..1204cf230fa 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.cs.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.cs.xlf @@ -17,14 +17,29 @@ Cíl je příliš krátký. + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. Vyžaduje se nezáporné číslo. - - This program location is thought to be unreachable. File='{0}', Line={1} - Toto umístění programu se považuje za nedostupné. File={0}, Line={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.de.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.de.xlf index e0cf3b62e5a..50a8e67c6b5 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.de.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.de.xlf @@ -17,14 +17,29 @@ Das Ziel ist zu kurz. + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. Nicht negative Zahl erforderlich. - - This program location is thought to be unreachable. File='{0}', Line={1} - Dieser Programmspeicherort wird als nicht erreichbar betrachtet. Datei="{0}", Zeile={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.es.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.es.xlf index cedb1b57749..c9f94e4eba1 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.es.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.es.xlf @@ -17,14 +17,29 @@ El destino es demasiado corto. + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. Se requiere un número no negativo. - - This program location is thought to be unreachable. File='{0}', Line={1} - Se considera que esta ubicación del programa es inaccesible. Archivo=''{0}'', Línea={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.fr.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.fr.xlf index e76ec00fb02..041a909e43f 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.fr.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.fr.xlf @@ -17,14 +17,29 @@ La destination est trop courte. + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. Nombre non négatif obligatoire. - - This program location is thought to be unreachable. File='{0}', Line={1} - Cet emplacement du programme est considéré comme inaccessible. File='{0}', Ligne={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.it.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.it.xlf index fb2b280f8a9..dfb70c3bdb9 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.it.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.it.xlf @@ -17,14 +17,29 @@ La destinazione è troppo breve. + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. Numero non negativo obbligatorio. - - This program location is thought to be unreachable. File='{0}', Line={1} - Questo percorso del programma è considerato irraggiungibile. File='{0}', Riga={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.ja.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.ja.xlf index c5d2b39f9ed..1da26eb2d6d 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.ja.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.ja.xlf @@ -17,14 +17,29 @@ 宛先が短すぎます。 + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. 負でない数値が必要です。 - - This program location is thought to be unreachable. File='{0}', Line={1} - このプログラムの場所には到達できないと考えられます。ファイル='{0}'、行={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.ko.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.ko.xlf index 648bfcccbf7..9d2bdaf9299 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.ko.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.ko.xlf @@ -17,14 +17,29 @@ 대상이 너무 짧습니다. + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. 음수가 아닌 수가 필요합니다. - - This program location is thought to be unreachable. File='{0}', Line={1} - 이 프로그램 위치에 연결할 수 없는 것으로 간주합니다. File=’{0}’, Line={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.pl.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.pl.xlf index 123eb7e275d..b30469af398 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.pl.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.pl.xlf @@ -17,14 +17,29 @@ Miejsce docelowe jest zbyt krótkie. + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. Wymagana jest liczba nieujemna. - - This program location is thought to be unreachable. File='{0}', Line={1} - Ta lokalizacja programu jest uważana za nieosiągalną. Plik=„{0}”, Linia={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.pt-BR.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.pt-BR.xlf index fa5e1097332..3dc49eefcc8 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.pt-BR.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.pt-BR.xlf @@ -17,14 +17,29 @@ O destino é muito curto. + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. É necessário um número não negativo. - - This program location is thought to be unreachable. File='{0}', Line={1} - Este local do programa é considerado inacessível. Arquivo='{0}', Linha={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.ru.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.ru.xlf index a8e645b200a..9bef5622ce0 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.ru.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.ru.xlf @@ -17,14 +17,29 @@ Имя назначения слишком короткое. + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. Требуется неотрицательное число. - - This program location is thought to be unreachable. File='{0}', Line={1} - Похоже, что это расположение программы является недоступным. Файл="{0}", строка={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.tr.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.tr.xlf index ef28938e8f6..d97eb0c12d5 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.tr.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.tr.xlf @@ -17,14 +17,29 @@ Hedef çok kısa. + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. Negatif olmayan sayı gerekiyor. - - This program location is thought to be unreachable. File='{0}', Line={1} - Bu program konumu erişilemez olarak görülüyor. Dosya='{0}', Satır={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.zh-Hans.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.zh-Hans.xlf index 9bdfb128399..49a8680e0be 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.zh-Hans.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.zh-Hans.xlf @@ -17,14 +17,29 @@ 目标太短。 + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. 需要提供非负数。 - - This program location is thought to be unreachable. File='{0}', Line={1} - 此程序位置被视为无法访问。File='{0}'，Line={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.zh-Hant.xlf b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.zh-Hant.xlf index 6509dc15484..8ef1fb2094a 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.zh-Hant.xlf +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/Resources/xlf/SR.zh-Hant.xlf @@ -17,14 +17,29 @@ 目的地太短。 + + Expected condition to be false. + Expected condition to be false. + + + + Expected condition to be true. + Expected condition to be true. + + + + File='{0}', Line={1} + File='{0}', Line={1} + + Non-negative number required. 需要非負數。 - - This program location is thought to be unreachable. File='{0}', Line={1} - 此程式位置被認為無法連接。File='{0}', Line={1} + + This program location is thought to be unreachable. + This program location is thought to be unreachable. diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/VoidResult.cs b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/VoidResult.cs new file mode 100644 index 00000000000..2aba5111fa1 --- /dev/null +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/VoidResult.cs @@ -0,0 +1,21 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System; + +namespace Microsoft.AspNetCore.Razor; + +/// +/// Explicitly indicates result is void +/// +internal readonly struct VoidResult : IEquatable +{ + public override bool Equals(object? obj) + => obj is VoidResult; + + public override int GetHashCode() + => 0; + + public bool Equals(VoidResult other) + => true; +} From 52fe75dd7367ebf3e5750ad0debb84b99caed209 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Wed, 20 Mar 2024 11:53:50 -0700 Subject: [PATCH 02/17] Rewrite OpenDocumentGenerator to use AsyncBatchingWorkQueue --- .../OpenDocumentGenerator.cs | 146 +++++++++++------- .../ProjectSystem/IProjectSnapshot.cs | 2 + .../ProjectSystem/ProjectSnapshot.cs | 7 + .../ProjectSystem/RemoteProjectSnapshot.cs | 7 + .../EphemeralProjectSnapshot.cs | 7 + .../OpenDocumentGeneratorTest.cs | 50 +++--- 6 files changed, 128 insertions(+), 91 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs index 9933e99151e..dcb1350d2b2 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs @@ -3,11 +3,14 @@ using System; using System.Collections.Generic; -using System.Linq; +using System.Collections.Immutable; +using System.Diagnostics; using System.Threading; using System.Threading.Tasks; +using Microsoft.AspNetCore.Razor.PooledObjects; using Microsoft.CodeAnalysis.Razor; using Microsoft.CodeAnalysis.Razor.ProjectSystem; +using Microsoft.CodeAnalysis.Razor.Utilities; using Microsoft.CodeAnalysis.Razor.Workspaces; namespace Microsoft.AspNetCore.Razor.LanguageServer; @@ -19,30 +22,36 @@ internal class OpenDocumentGenerator : IRazorStartupService, IDisposable // than necessary when both C# and HTML documents change at the same time, firing our event handler // twice. Through testing 10ms was a good balance towards providing some de-bouncing but having minimal // to no impact on results. + // // It's worth noting that the queue implementation means that this delay is not restarted with each new - // work item, so even in very high speed typing, with changings coming in at sub-10-millisecond speed, + // work item, so even in very high speed typing, with changes coming in at sub-10-millisecond speed, // the queue will still process documents even if the user doesn't pause at all, but also will not process // a document for each keystroke. - private static readonly TimeSpan s_batchingTimeSpan = TimeSpan.FromMilliseconds(10); + private static readonly TimeSpan s_delay = TimeSpan.FromMilliseconds(10); + private readonly ImmutableArray _listeners; private readonly IProjectSnapshotManager _projectManager; private readonly ProjectSnapshotManagerDispatcher _dispatcher; private readonly LanguageServerFeatureOptions _options; - private readonly IReadOnlyList _listeners; - private readonly BatchingWorkQueue _workQueue; + + private readonly AsyncBatchingWorkQueue _workQueue; + private readonly HashSet _processedFilePaths; + private readonly CancellationTokenSource _disposeTokenSource; public OpenDocumentGenerator( IEnumerable listeners, IProjectSnapshotManager projectManager, ProjectSnapshotManagerDispatcher dispatcher, - LanguageServerFeatureOptions options, - IErrorReporter errorReporter) + LanguageServerFeatureOptions options) { - _listeners = listeners.ToArray(); + _listeners = listeners.ToImmutableArray(); _projectManager = projectManager; _dispatcher = dispatcher; _options = options; - _workQueue = new BatchingWorkQueue(s_batchingTimeSpan, FilePathComparer.Instance, errorReporter); + + _processedFilePaths = new(FilePathComparer.Instance); + _disposeTokenSource = new(); + _workQueue = new AsyncBatchingWorkQueue(s_delay, ProcessBatchAsync, _disposeTokenSource.Token); _projectManager.Changed += ProjectManager_Changed; @@ -54,7 +63,60 @@ public OpenDocumentGenerator( public void Dispose() { - _workQueue.Dispose(); + _disposeTokenSource.Cancel(); + _disposeTokenSource.Dispose(); + } + + private async ValueTask ProcessBatchAsync(ImmutableArray items, CancellationToken token) + { + Debug.Assert(_processedFilePaths.Count == 0); + + using var itemsToProcess = new PooledArrayBuilder(capacity: items.Length); + + // Walk items in reverse to ensure that we only process the most recent document snapshots. + for (var i = items.Length - 1; i >= 0; i--) + { + var document = items[i]; + var filePath = document.FilePath.AssumeNotNull(); + + if (_processedFilePaths.Add(filePath)) + { + itemsToProcess.Push(document); + } + } + + while (itemsToProcess.Count > 0) + { + if (token.IsCancellationRequested) + { + return; + } + + var document = itemsToProcess.Pop(); + var codeDocument = await document.GetGeneratedOutputAsync().ConfigureAwait(false); + + await _dispatcher + .RunAsync( + static state => + { + var (codeDocument, document, listeners, token) = state; + + foreach (var listener in listeners) + { + if (token.IsCancellationRequested) + { + return; + } + + listener.DocumentProcessed(codeDocument, document); + } + }, + state: (codeDocument, document, _listeners, token), + token) + .ConfigureAwait(false); + } + + _processedFilePaths.Clear(); } private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args) @@ -65,8 +127,6 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args) return; } - _dispatcher.AssertRunningOnDispatcher(); - switch (args.Kind) { case ProjectChangeKind.ProjectChanged: @@ -75,9 +135,9 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args) foreach (var documentFilePath in newProject.DocumentFilePaths) { - if (newProject.GetDocument(documentFilePath) is { } document) + if (newProject.TryGetDocument(documentFilePath, out var document)) { - TryEnqueue(document); + EnqueueIfNecessary(document); } } @@ -89,13 +149,13 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args) var newProject = args.Newer.AssumeNotNull(); var documentFilePath = args.DocumentFilePath.AssumeNotNull(); - if (newProject.GetDocument(documentFilePath) is { } document) + if (newProject.TryGetDocument(documentFilePath, out var document)) { - // We don't enqueue the current document because added documents are by default closed. + // We don't enqueue the current document because added documents are initially closed. foreach (var relatedDocument in newProject.GetRelatedDocuments(document)) { - TryEnqueue(relatedDocument); + EnqueueIfNecessary(relatedDocument); } } @@ -107,13 +167,13 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args) var newProject = args.Newer.AssumeNotNull(); var documentFilePath = args.DocumentFilePath.AssumeNotNull(); - if (newProject.GetDocument(documentFilePath) is { } document) + if (newProject.TryGetDocument(documentFilePath, out var document)) { - TryEnqueue(document); + EnqueueIfNecessary(document); foreach (var relatedDocument in newProject.GetRelatedDocuments(document)) { - TryEnqueue(relatedDocument); + EnqueueIfNecessary(relatedDocument); } } @@ -126,15 +186,15 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args) var oldProject = args.Older.AssumeNotNull(); var documentFilePath = args.DocumentFilePath.AssumeNotNull(); - if (oldProject.GetDocument(documentFilePath) is { } document) + if (oldProject.TryGetDocument(documentFilePath, out var document)) { foreach (var relatedDocument in oldProject.GetRelatedDocuments(document)) { var relatedDocumentFilePath = relatedDocument.FilePath.AssumeNotNull(); - if (newProject.GetDocument(relatedDocumentFilePath) is { } newRelatedDocument) + if (newProject.TryGetDocument(relatedDocumentFilePath, out var newRelatedDocument)) { - TryEnqueue(newRelatedDocument); + EnqueueIfNecessary(newRelatedDocument); } } } @@ -149,49 +209,15 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args) } } - void TryEnqueue(IDocumentSnapshot document) + void EnqueueIfNecessary(IDocumentSnapshot document) { - var documentFilePath = document.FilePath.AssumeNotNull(); - - if (!_projectManager.IsDocumentOpen(documentFilePath) && + if (!_projectManager.IsDocumentOpen(document.FilePath.AssumeNotNull()) && !_options.UpdateBuffersForClosedDocuments) { return; } - var key = $"{document.Project.Key.Id}:{documentFilePath}"; - var workItem = new ProcessWorkItem(document, _listeners, _dispatcher); - _workQueue.Enqueue(key, workItem); - } - } - - private class ProcessWorkItem : BatchableWorkItem - { - private readonly IDocumentSnapshot _latestDocument; - private readonly IEnumerable _documentProcessedListeners; - private readonly ProjectSnapshotManagerDispatcher _dispatcher; - - public ProcessWorkItem( - IDocumentSnapshot latestDocument, - IReadOnlyList documentProcessedListeners, - ProjectSnapshotManagerDispatcher dispatcher) - { - _latestDocument = latestDocument; - _documentProcessedListeners = documentProcessedListeners; - _dispatcher = dispatcher; - } - - public override async ValueTask ProcessAsync(CancellationToken cancellationToken) - { - var codeDocument = await _latestDocument.GetGeneratedOutputAsync().ConfigureAwait(false); - - await _dispatcher.RunAsync(() => - { - foreach (var listener in _documentProcessedListeners) - { - listener.DocumentProcessed(codeDocument, _latestDocument); - } - }, cancellationToken).ConfigureAwait(false); + _workQueue.AddWork(document); } } } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/IProjectSnapshot.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/IProjectSnapshot.cs index c356c05241f..56459d179bc 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/IProjectSnapshot.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/IProjectSnapshot.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Razor.Language; @@ -37,6 +38,7 @@ internal interface IProjectSnapshot RazorProjectEngine GetProjectEngine(); IDocumentSnapshot? GetDocument(string filePath); + bool TryGetDocument(string filePath, [NotNullWhen(true)] out IDocumentSnapshot? document); bool IsImportDocument(IDocumentSnapshot document); /// diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectSnapshot.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectSnapshot.cs index 4d20ef59d55..ff80c8c93a7 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectSnapshot.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/ProjectSystem/ProjectSnapshot.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Razor; @@ -72,6 +73,12 @@ public ProjectSnapshot(ProjectState state) } } + public bool TryGetDocument(string filePath, [NotNullWhen(true)] out IDocumentSnapshot? document) + { + document = GetDocument(filePath); + return document is not null; + } + public bool IsImportDocument(IDocumentSnapshot document) { if (document is null) diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteProjectSnapshot.cs b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteProjectSnapshot.cs index edcc45adbe4..f3b701980f7 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteProjectSnapshot.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Remote.Razor/ProjectSystem/RemoteProjectSnapshot.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Linq; using System.Threading; @@ -113,6 +114,12 @@ public async ValueTask> GetTagHelpersAsync(C return _documentSnapshotFactory.GetOrCreate(textDocument); } + public bool TryGetDocument(string filePath, [NotNullWhen(true)] out IDocumentSnapshot? document) + { + document = GetDocument(filePath); + return document is not null; + } + public RazorProjectEngine GetProjectEngine() => throw new InvalidOperationException("Should not be called for cohosted projects."); /// diff --git a/src/Razor/src/Microsoft.VisualStudio.LegacyEditor.Razor/EphemeralProjectSnapshot.cs b/src/Razor/src/Microsoft.VisualStudio.LegacyEditor.Razor/EphemeralProjectSnapshot.cs index d79e89ca2a0..73b26263e09 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LegacyEditor.Razor/EphemeralProjectSnapshot.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LegacyEditor.Razor/EphemeralProjectSnapshot.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics.CodeAnalysis; using System.IO; using System.Threading; using System.Threading.Tasks; @@ -65,6 +66,12 @@ public EphemeralProjectSnapshot(IProjectEngineFactoryProvider projectEngineFacto return null; } + public bool TryGetDocument(string filePath, [NotNullWhen(true)] out IDocumentSnapshot? document) + { + document = null; + return false; + } + public bool IsImportDocument(IDocumentSnapshot document) { if (document is null) diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/OpenDocumentGeneratorTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/OpenDocumentGeneratorTest.cs index 406e5b808ad..0fe6a9dc551 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/OpenDocumentGeneratorTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/OpenDocumentGeneratorTest.cs @@ -1,8 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT license. See License.txt in the project root for license information. -#nullable disable - using System; using System.Threading; using System.Threading.Tasks; @@ -19,24 +17,16 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer; -public class OpenDocumentGeneratorTest : LanguageServerTestBase +public class OpenDocumentGeneratorTest(ITestOutputHelper testOutput) : LanguageServerTestBase(testOutput) { - private readonly HostDocument[] _documents; - private readonly HostProject _hostProject1; - private readonly HostProject _hostProject2; + private readonly HostDocument[] _documents = + [ + new HostDocument("c:/Test1/Index.cshtml", "Index.cshtml"), + new HostDocument("c:/Test1/Components/Counter.cshtml", "Components/Counter.cshtml"), + ]; - public OpenDocumentGeneratorTest(ITestOutputHelper testOutput) - : base(testOutput) - { - _documents = - [ - new HostDocument("c:/Test1/Index.cshtml", "Index.cshtml"), - new HostDocument("c:/Test1/Components/Counter.cshtml", "Components/Counter.cshtml"), - ]; - - _hostProject1 = new HostProject("c:/Test1/Test1.csproj", "c:/Test1/obj", RazorConfiguration.Default, "TestRootNamespace"); - _hostProject2 = new HostProject("c:/Test2/Test2.csproj", "c:/Test2/obj", RazorConfiguration.Default, "TestRootNamespace"); - } + private readonly HostProject _hostProject1 = new("c:/Test1/Test1.csproj", "c:/Test1/obj", RazorConfiguration.Default, "TestRootNamespace"); + private readonly HostProject _hostProject2 = new("c:/Test2/Test2.csproj", "c:/Test2/obj", RazorConfiguration.Default, "TestRootNamespace"); [Fact] public async Task DocumentAdded_IgnoresClosedDocument() @@ -44,7 +34,7 @@ public async Task DocumentAdded_IgnoresClosedDocument() // Arrange var projectManager = CreateProjectSnapshotManager(); var listener = new TestDocumentProcessedListener(); - var queue = new TestOpenDocumentGenerator(projectManager, Dispatcher, ErrorReporter, listener); + using var generator = new TestOpenDocumentGenerator(projectManager, Dispatcher, listener); await projectManager.UpdateAsync(updater => { @@ -52,7 +42,7 @@ await projectManager.UpdateAsync(updater => updater.ProjectAdded(_hostProject2); // Act - updater.DocumentAdded(_hostProject1.Key, _documents[0], null); + updater.DocumentAdded(_hostProject1.Key, _documents[0], null!); }); // Assert @@ -65,13 +55,13 @@ public async Task DocumentChanged_IgnoresClosedDocument() // Arrange var projectManager = CreateProjectSnapshotManager(); var listener = new TestDocumentProcessedListener(); - var queue = new TestOpenDocumentGenerator(projectManager, Dispatcher, ErrorReporter, listener); + using var generator = new TestOpenDocumentGenerator(projectManager, Dispatcher, listener); await projectManager.UpdateAsync(updater => { updater.ProjectAdded(_hostProject1); updater.ProjectAdded(_hostProject2); - updater.DocumentAdded(_hostProject1.Key, _documents[0], null); + updater.DocumentAdded(_hostProject1.Key, _documents[0], null!); // Act updater.DocumentChanged(_hostProject1.Key, _documents[0].FilePath, SourceText.From("new")); @@ -87,13 +77,13 @@ public async Task DocumentChanged_ProcessesOpenDocument() // Arrange var projectManager = CreateProjectSnapshotManager(); var listener = new TestDocumentProcessedListener(); - var queue = new TestOpenDocumentGenerator(projectManager, Dispatcher, ErrorReporter, listener); + using var generator = new TestOpenDocumentGenerator(projectManager, Dispatcher, listener); await projectManager.UpdateAsync(updater => { updater.ProjectAdded(_hostProject1); updater.ProjectAdded(_hostProject2); - updater.DocumentAdded(_hostProject1.Key, _documents[0], null); + updater.DocumentAdded(_hostProject1.Key, _documents[0], null!); updater.DocumentOpened(_hostProject1.Key, _documents[0].FilePath, SourceText.From(string.Empty)); // Act @@ -112,13 +102,13 @@ public async Task ProjectChanged_IgnoresClosedDocument() // Arrange var projectManager = CreateProjectSnapshotManager(); var listener = new TestDocumentProcessedListener(); - var queue = new TestOpenDocumentGenerator(projectManager, Dispatcher, ErrorReporter, listener); + using var generator = new TestOpenDocumentGenerator(projectManager, Dispatcher, listener); await projectManager.UpdateAsync(updater => { updater.ProjectAdded(_hostProject1); updater.ProjectAdded(_hostProject2); - updater.DocumentAdded(_hostProject1.Key, _documents[0], null); + updater.DocumentAdded(_hostProject1.Key, _documents[0], null!); // Act updater.ProjectWorkspaceStateChanged(_hostProject1.Key, @@ -135,13 +125,13 @@ public async Task ProjectChanged_ProcessesOpenDocument() // Arrange var projectManager = CreateProjectSnapshotManager(); var listener = new TestDocumentProcessedListener(); - var queue = new TestOpenDocumentGenerator(projectManager, Dispatcher, ErrorReporter, listener); + using var generator = new TestOpenDocumentGenerator(projectManager, Dispatcher, listener); await projectManager.UpdateAsync(updater => { updater.ProjectAdded(_hostProject1); updater.ProjectAdded(_hostProject2); - updater.DocumentAdded(_hostProject1.Key, _documents[0], null); + updater.DocumentAdded(_hostProject1.Key, _documents[0], null!); updater.DocumentOpened(_hostProject1.Key, _documents[0].FilePath, SourceText.From(string.Empty)); // Act @@ -150,7 +140,6 @@ await projectManager.UpdateAsync(updater => }); // Assert - var document = await listener.GetProcessedDocumentAsync(cancelAfter: TimeSpan.FromSeconds(10)); Assert.Equal(document.FilePath, _documents[0].FilePath); } @@ -158,9 +147,8 @@ await projectManager.UpdateAsync(updater => private class TestOpenDocumentGenerator( IProjectSnapshotManager projectManager, ProjectSnapshotManagerDispatcher dispatcher, - IErrorReporter errorReporter, params DocumentProcessedListener[] listeners) - : OpenDocumentGenerator(listeners, projectManager, dispatcher, TestLanguageServerFeatureOptions.Instance, errorReporter); + : OpenDocumentGenerator(listeners, projectManager, dispatcher, TestLanguageServerFeatureOptions.Instance); private class TestDocumentProcessedListener : DocumentProcessedListener { From bcb584282c404fb21bdda93abead20c023fe1c3b Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Wed, 20 Mar 2024 13:09:50 -0700 Subject: [PATCH 03/17] Don't use BatchingWorkQueue to control semanticTokens/refresh notifications `WorkspaceSemanticTokensRefreshNotifier` (previously `WorkspaceSemanticTokensRefreshPublisher`) uses a `BatchingWorkQueue` for debouncing, even though it isn't actually batching work. I've replaced this with much simpler code that just tracks a `Task` generated with `Task.Delay(...)`. --- .../IServiceCollectionExtensions.cs | 2 +- ...WorkspaceSemanticTokensRefreshNotifier.cs} | 4 +- .../RazorSemanticTokensRefreshEndpoint.cs | 6 +- .../WorkspaceSemanticTokensRefreshNotifier.cs | 113 ++++++++++++++ ...WorkspaceSemanticTokensRefreshPublisher.cs | 104 ------------- .../WorkspaceSemanticTokensRefreshTrigger.cs | 6 +- .../RazorSemanticTokensRefreshEndpointTest.cs | 7 +- ...kspaceSemanticTokensRefreshNotifierTest.cs | 132 ++++++++++++++++ ...spaceSemanticTokensRefreshPublisherTest.cs | 147 ------------------ ...rkspaceSemanticTokensRefreshTriggerTest.cs | 8 +- 10 files changed, 261 insertions(+), 268 deletions(-) rename src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/{IWorkspaceSemanticTokensRefreshPublisher.cs => IWorkspaceSemanticTokensRefreshNotifier.cs} (65%) create mode 100644 src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshNotifier.cs delete mode 100644 src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshPublisher.cs create mode 100644 src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/WorkspaceSemanticTokensRefreshNotifierTest.cs delete mode 100644 src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/WorkspaceSemanticTokensRefreshPublisherTest.cs diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Extensions/IServiceCollectionExtensions.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Extensions/IServiceCollectionExtensions.cs index 040a4ac7482..49a0f97b6fd 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Extensions/IServiceCollectionExtensions.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Extensions/IServiceCollectionExtensions.cs @@ -139,7 +139,7 @@ public static void AddSemanticTokensServices(this IServiceCollection services, L services.AddHandler(); - services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); } diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/IWorkspaceSemanticTokensRefreshPublisher.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/IWorkspaceSemanticTokensRefreshNotifier.cs similarity index 65% rename from src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/IWorkspaceSemanticTokensRefreshPublisher.cs rename to src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/IWorkspaceSemanticTokensRefreshNotifier.cs index 48e639e8179..5421062fe0c 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/IWorkspaceSemanticTokensRefreshPublisher.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/IWorkspaceSemanticTokensRefreshNotifier.cs @@ -3,7 +3,7 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer; -internal interface IWorkspaceSemanticTokensRefreshPublisher +internal interface IWorkspaceSemanticTokensRefreshNotifier { - void EnqueueWorkspaceSemanticTokensRefresh(); + void NotifyWorkspaceSemanticTokensRefresh(); } diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Semantic/RazorSemanticTokensRefreshEndpoint.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Semantic/RazorSemanticTokensRefreshEndpoint.cs index 2ae39e9e090..e0294a33f09 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Semantic/RazorSemanticTokensRefreshEndpoint.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Semantic/RazorSemanticTokensRefreshEndpoint.cs @@ -9,16 +9,16 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer.Semantic; [RazorLanguageServerEndpoint(CustomMessageNames.RazorSemanticTokensRefreshEndpoint)] -internal sealed class RazorSemanticTokensRefreshEndpoint(IWorkspaceSemanticTokensRefreshPublisher semanticTokensRefreshPublisher) : IRazorNotificationHandler +internal sealed class RazorSemanticTokensRefreshEndpoint(IWorkspaceSemanticTokensRefreshNotifier semanticTokensRefreshPublisher) : IRazorNotificationHandler { - private readonly IWorkspaceSemanticTokensRefreshPublisher _semanticTokensRefreshPublisher = semanticTokensRefreshPublisher; + private readonly IWorkspaceSemanticTokensRefreshNotifier _semanticTokensRefreshPublisher = semanticTokensRefreshPublisher; public bool MutatesSolutionState { get; } = false; public Task HandleNotificationAsync(SemanticTokensRefreshParams request, RazorRequestContext requestContext, CancellationToken cancellationToken) { // We have to invalidate the tokens cache since it may no longer be up to date. - _semanticTokensRefreshPublisher.EnqueueWorkspaceSemanticTokensRefresh(); + _semanticTokensRefreshPublisher.NotifyWorkspaceSemanticTokensRefresh(); return Task.CompletedTask; } diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshNotifier.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshNotifier.cs new file mode 100644 index 00000000000..89c1ceae40f --- /dev/null +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshNotifier.cs @@ -0,0 +1,113 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System; +using System.Threading; +using System.Threading.Tasks; +using Microsoft.CodeAnalysis.Razor.Workspaces.Protocol; +using Microsoft.VisualStudio.LanguageServer.Protocol; +using Microsoft.VisualStudio.Threading; + +namespace Microsoft.AspNetCore.Razor.LanguageServer; + +internal class WorkspaceSemanticTokensRefreshNotifier : IWorkspaceSemanticTokensRefreshNotifier, IDisposable +{ + private static readonly TimeSpan s_delay = TimeSpan.FromMilliseconds(250); + + private readonly IClientCapabilitiesService _clientCapabilitiesService; + private readonly IClientConnection _clientConnection; + private readonly CancellationTokenSource _disposeCancellationSource; + private readonly IDisposable _optionsChangeListener; + + private readonly object _gate = new(); + private bool _waitingToRefresh; + private Task _refreshTask = Task.CompletedTask; + + private bool _isColoringBackground; + + public WorkspaceSemanticTokensRefreshNotifier( + IClientCapabilitiesService clientCapabilitiesService, + IClientConnection clientConnection, + RazorLSPOptionsMonitor optionsMonitor) + { + _clientCapabilitiesService = clientCapabilitiesService; + _clientConnection = clientConnection; + + _disposeCancellationSource = new(); + + _isColoringBackground = optionsMonitor.CurrentValue.ColorBackground; + _optionsChangeListener = optionsMonitor.OnChange(HandleOptionsChange); + } + + public void Dispose() + { + _optionsChangeListener.Dispose(); + + _disposeCancellationSource.Cancel(); + _disposeCancellationSource.Dispose(); + } + + private void HandleOptionsChange(RazorLSPOptions options, string _) + { + if (options.ColorBackground != _isColoringBackground) + { + _isColoringBackground = options.ColorBackground; + NotifyWorkspaceSemanticTokensRefresh(); + } + } + + public void NotifyWorkspaceSemanticTokensRefresh() + { + if (_disposeCancellationSource.IsCancellationRequested) + { + return; + } + + lock (_gate) + { + if (_waitingToRefresh) + { + // We're going to refresh shortly. + return; + } + + var capabilities = _clientCapabilitiesService.ClientCapabilities; + var useWorkspaceRefresh = capabilities?.Workspace?.SemanticTokens?.RefreshSupport ?? false; + + if (useWorkspaceRefresh) + { + _refreshTask = RefreshAfterDelayAsync(); + _waitingToRefresh = true; + } + } + + async Task RefreshAfterDelayAsync() + { + await Task.Delay(s_delay).ConfigureAwait(false); + + _clientConnection + .SendNotificationAsync(Methods.WorkspaceSemanticTokensRefreshName, _disposeCancellationSource.Token) + .Forget(); + + _waitingToRefresh = false; + } + } + + internal TestAccessor GetTestAccessor() + => new(this); + + internal class TestAccessor(WorkspaceSemanticTokensRefreshNotifier instance) + { + public async Task WaitForNotificationAsync() + { + Task refreshTask; + + lock (instance._gate) + { + refreshTask = instance._refreshTask; + } + + await refreshTask.ConfigureAwait(false); + } + } +} diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshPublisher.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshPublisher.cs deleted file mode 100644 index b497f2f394b..00000000000 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshPublisher.cs +++ /dev/null @@ -1,104 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the MIT license. See License.txt in the project root for license information. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Razor; -using Microsoft.CodeAnalysis.Razor.Workspaces; -using Microsoft.CodeAnalysis.Razor.Workspaces.Protocol; -using Microsoft.VisualStudio.LanguageServer.Protocol; - -namespace Microsoft.AspNetCore.Razor.LanguageServer; - -internal class WorkspaceSemanticTokensRefreshPublisher : IWorkspaceSemanticTokensRefreshPublisher, IDisposable -{ - private const string WorkspaceSemanticTokensRefreshKey = "WorkspaceSemanticTokensRefresh"; - private static readonly TimeSpan s_debounceTimeSpan = TimeSpan.FromMilliseconds(250); - - private readonly IClientCapabilitiesService _clientCapabilitiesService; - private readonly IClientConnection _clientConnection; - private readonly BatchingWorkQueue _workQueue; - - private bool _isColoringBackground; - - public WorkspaceSemanticTokensRefreshPublisher( - IClientCapabilitiesService clientCapabilitiesService, - IClientConnection clientConnection, - IErrorReporter errorReporter, - RazorLSPOptionsMonitor razorLSPOptionsMonitor) - { - _clientCapabilitiesService = clientCapabilitiesService; - _clientConnection = clientConnection; - _workQueue = new BatchingWorkQueue(s_debounceTimeSpan, StringComparer.Ordinal, errorReporter: errorReporter); - - _isColoringBackground = razorLSPOptionsMonitor.CurrentValue.ColorBackground; - razorLSPOptionsMonitor.OnChange(HandleOptionsChange); - } - - private void HandleOptionsChange(RazorLSPOptions options, string _) - { - if (options.ColorBackground != _isColoringBackground) - { - _isColoringBackground = options.ColorBackground; - EnqueueWorkspaceSemanticTokensRefresh(); - } - } - - public void EnqueueWorkspaceSemanticTokensRefresh() - { - var capabilities = _clientCapabilitiesService.ClientCapabilities; - var useWorkspaceRefresh = capabilities?.Workspace?.SemanticTokens?.RefreshSupport ?? false; - - if (useWorkspaceRefresh) - { - var workItem = new SemanticTokensRefreshWorkItem(_clientConnection); - _workQueue.Enqueue(WorkspaceSemanticTokensRefreshKey, workItem); - } - } - - public void Dispose() - { - _workQueue.Dispose(); - } - - private class SemanticTokensRefreshWorkItem : BatchableWorkItem - { - private readonly IClientConnection _clientConnection; - - public SemanticTokensRefreshWorkItem(IClientConnection clientConnection) - { - _clientConnection = clientConnection; - } - - public override ValueTask ProcessAsync(CancellationToken cancellationToken) - { - var task = _clientConnection.SendNotificationAsync(Methods.WorkspaceSemanticTokensRefreshName, cancellationToken); - - return new ValueTask(task); - } - } - - internal TestAccessor GetTestAccessor() - => new(this); - - internal class TestAccessor - { - private readonly WorkspaceSemanticTokensRefreshPublisher _publisher; - - internal TestAccessor(WorkspaceSemanticTokensRefreshPublisher publisher) - { - _publisher = publisher; - } - - public void WaitForEmpty() - { - var workQueueTestAccessor = _publisher._workQueue.GetTestAccessor(); - workQueueTestAccessor.NotifyBackgroundWorkCompleted = new ManualResetEventSlim(initialState: false); - while (workQueueTestAccessor.IsScheduledOrRunning) - { - workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(TimeSpan.FromMilliseconds(50)); - } - } - } -} diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshTrigger.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshTrigger.cs index b90663c566a..daaa3ea43bf 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshTrigger.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshTrigger.cs @@ -11,11 +11,11 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer; /// internal class WorkspaceSemanticTokensRefreshTrigger : IRazorStartupService { - private readonly IWorkspaceSemanticTokensRefreshPublisher _publisher; + private readonly IWorkspaceSemanticTokensRefreshNotifier _publisher; private readonly IProjectSnapshotManager _projectManager; public WorkspaceSemanticTokensRefreshTrigger( - IWorkspaceSemanticTokensRefreshPublisher publisher, + IWorkspaceSemanticTokensRefreshNotifier publisher, IProjectSnapshotManager projectManager) { _publisher = publisher; @@ -30,7 +30,7 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args) // is edited and if a parameter or type change is made it should be reflected as a ProjectChanged. if (args.Kind != ProjectChangeKind.DocumentChanged) { - _publisher.EnqueueWorkspaceSemanticTokensRefresh(); + _publisher.NotifyWorkspaceSemanticTokensRefresh(); } } } diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Semantic/RazorSemanticTokensRefreshEndpointTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Semantic/RazorSemanticTokensRefreshEndpointTest.cs index 12987d9d009..0705a8999b2 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Semantic/RazorSemanticTokensRefreshEndpointTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/Semantic/RazorSemanticTokensRefreshEndpointTest.cs @@ -37,16 +37,15 @@ public async Task Handle_QueuesRefresh() }; var clientCapabilitiesService = new TestClientCapabilitiesService(clientCapabilities); var serverClient = new TestClient(); - var errorReporter = new TestErrorReporter(); var optionsMonitor = GetOptionsMonitor(); - using var semanticTokensRefreshPublisher = new WorkspaceSemanticTokensRefreshPublisher(clientCapabilitiesService, serverClient, errorReporter, optionsMonitor); - var refreshEndpoint = new RazorSemanticTokensRefreshEndpoint(semanticTokensRefreshPublisher); + using var publisher = new WorkspaceSemanticTokensRefreshNotifier(clientCapabilitiesService, serverClient, optionsMonitor); + var refreshEndpoint = new RazorSemanticTokensRefreshEndpoint(publisher); var refreshParams = new SemanticTokensRefreshParams(); var requestContext = new RazorRequestContext(); // Act await refreshEndpoint.HandleNotificationAsync(refreshParams, requestContext, DisposalToken); - semanticTokensRefreshPublisher.GetTestAccessor().WaitForEmpty(); + await publisher.GetTestAccessor().WaitForNotificationAsync(); // Assert Assert.Equal(Methods.WorkspaceSemanticTokensRefreshName, serverClient.Requests.Single().Method); diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/WorkspaceSemanticTokensRefreshNotifierTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/WorkspaceSemanticTokensRefreshNotifierTest.cs new file mode 100644 index 00000000000..67c33c328cc --- /dev/null +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/WorkspaceSemanticTokensRefreshNotifierTest.cs @@ -0,0 +1,132 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System.Threading; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Razor.LanguageServer.Test; +using Microsoft.AspNetCore.Razor.Test.Common; +using Microsoft.AspNetCore.Razor.Test.Common.LanguageServer; +using Microsoft.Extensions.Options; +using Microsoft.VisualStudio.LanguageServer.Protocol; +using Moq; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.AspNetCore.Razor.LanguageServer; + +public class WorkspaceSemanticTokensRefreshNotifierTest(ITestOutputHelper testOutput) : LanguageServerTestBase(testOutput) +{ + [Fact] + public async Task PublishWorkspaceChanged_DoesNotSendWorkspaceRefreshRequest_WhenNotSupported() + { + // Arrange + var clientCapabilitiesService = GetClientCapabilitiesService(semanticRefreshEnabled: false); + var serverClient = new TestClient(); + var optionsMonitor = GetOptionsMonitor(); + using var notifier = new WorkspaceSemanticTokensRefreshNotifier(clientCapabilitiesService, serverClient, optionsMonitor); + var testAccessor = notifier.GetTestAccessor(); + + // Act + notifier.NotifyWorkspaceSemanticTokensRefresh(); + await testAccessor.WaitForNotificationAsync(); + + // Assert + Assert.Empty(serverClient.Requests); + } + + [Fact] + public async Task PublishWorkspaceChanged_SendsWorkspaceRefreshRequest_WhenSupported() + { + // Arrange + var clientCapabilitiesService = GetClientCapabilitiesService(semanticRefreshEnabled: true); + var serverClient = new TestClient(); + var optionsMonitor = GetOptionsMonitor(); + using var notifier = new WorkspaceSemanticTokensRefreshNotifier(clientCapabilitiesService, serverClient, optionsMonitor); + var testAccessor = notifier.GetTestAccessor(); + + // Act + notifier.NotifyWorkspaceSemanticTokensRefresh(); + await testAccessor.WaitForNotificationAsync(); + + // Assert + var request = Assert.Single(serverClient.Requests); + Assert.Equal(Methods.WorkspaceSemanticTokensRefreshName, request.Method); + } + + [Fact] + public async Task PublishWorkspaceChanged_DebouncesWorkspaceRefreshRequest() + { + // Arrange + var clientCapabilitiesService = GetClientCapabilitiesService(semanticRefreshEnabled: true); + var serverClient = new TestClient(); + var optionsMonitor = GetOptionsMonitor(); + using var notifier = new WorkspaceSemanticTokensRefreshNotifier(clientCapabilitiesService, serverClient, optionsMonitor); + var testAccessor = notifier.GetTestAccessor(); + + // Act + notifier.NotifyWorkspaceSemanticTokensRefresh(); + notifier.NotifyWorkspaceSemanticTokensRefresh(); + await testAccessor.WaitForNotificationAsync(); + notifier.NotifyWorkspaceSemanticTokensRefresh(); + notifier.NotifyWorkspaceSemanticTokensRefresh(); + await testAccessor.WaitForNotificationAsync(); + + // Assert + Assert.Collection(serverClient.Requests, + r => Assert.Equal(Methods.WorkspaceSemanticTokensRefreshName, r.Method), + r => Assert.Equal(Methods.WorkspaceSemanticTokensRefreshName, r.Method)); + } + + [Fact] + public async Task PublishWorkspaceChanged_SendsWorkspaceRefreshRequest_WhenOptionChanges() + { + // Arrange + var clientCapabilitiesService = GetClientCapabilitiesService(semanticRefreshEnabled: true); + var serverClient = new TestClient(); + var optionsMonitor = GetOptionsMonitor(withCSharpBackground: true); + using var notifier = new WorkspaceSemanticTokensRefreshNotifier(clientCapabilitiesService, serverClient, optionsMonitor); + var testAccessor = notifier.GetTestAccessor(); + + // Act + await optionsMonitor.UpdateAsync(DisposalToken); + await testAccessor.WaitForNotificationAsync(); + + // Assert + var request = Assert.Single(serverClient.Requests); + Assert.Equal(Methods.WorkspaceSemanticTokensRefreshName, request.Method); + } + + private static TestRazorLSPOptionsMonitor GetOptionsMonitor(bool withCSharpBackground = false) + { + var configurationSyncService = new StrictMock(); + + var options = RazorLSPOptions.Default with { ColorBackground = withCSharpBackground }; + configurationSyncService + .Setup(c => c.GetLatestOptionsAsync(It.IsAny())) + .Returns(Task.FromResult(options)); + + var optionsMonitorCache = new OptionsCache(); + + var optionsMonitor = TestRazorLSPOptionsMonitor.Create( + configurationSyncService.Object, + optionsMonitorCache); + + return optionsMonitor; + } + + private static TestClientCapabilitiesService GetClientCapabilitiesService(bool semanticRefreshEnabled) + { + var clientCapabilities = new VSInternalClientCapabilities + { + Workspace = new WorkspaceClientCapabilities + { + SemanticTokens = new SemanticTokensWorkspaceSetting + { + RefreshSupport = semanticRefreshEnabled + }, + } + }; + + return new TestClientCapabilitiesService(clientCapabilities); + } +} diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/WorkspaceSemanticTokensRefreshPublisherTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/WorkspaceSemanticTokensRefreshPublisherTest.cs deleted file mode 100644 index 40c21c8434d..00000000000 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/WorkspaceSemanticTokensRefreshPublisherTest.cs +++ /dev/null @@ -1,147 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the MIT license. See License.txt in the project root for license information. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Razor.LanguageServer.Test; -using Microsoft.AspNetCore.Razor.Test.Common.LanguageServer; -using Microsoft.CodeAnalysis; -using Microsoft.CodeAnalysis.Razor; -using Microsoft.CodeAnalysis.Razor.ProjectSystem; -using Microsoft.CodeAnalysis.Razor.Workspaces.Protocol; -using Microsoft.Extensions.Options; -using Microsoft.VisualStudio.LanguageServer.Protocol; -using Moq; -using Xunit; -using Xunit.Abstractions; - -namespace Microsoft.AspNetCore.Razor.LanguageServer; - -public class WorkspaceSemanticTokensRefreshPublisherTest(ITestOutputHelper testOutput) : LanguageServerTestBase(testOutput) -{ - [Fact] - public void PublishWorkspaceChanged_DoesNotSendWorkspaceRefreshRequest_WhenNotSupported() - { - // Arrange - var clientCapabilitiesService = GetClientCapabilitiesService(semanticRefreshEnabled: false); - var serverClient = new TestClient(); - var errorReporter = new TestErrorReporter(); - var optionsMonitor = GetOptionsMonitor(); - using var defaultWorkspaceChangedPublisher = new WorkspaceSemanticTokensRefreshPublisher(clientCapabilitiesService, serverClient, errorReporter, optionsMonitor); - var testAccessor = defaultWorkspaceChangedPublisher.GetTestAccessor(); - - // Act - defaultWorkspaceChangedPublisher.EnqueueWorkspaceSemanticTokensRefresh(); - testAccessor.WaitForEmpty(); - - // Assert - Assert.Empty(serverClient.Requests); - } - - [Fact] - public void PublishWorkspaceChanged_SendsWorkspaceRefreshRequest_WhenSupported() - { - // Arrange - var clientCapabilitiesService = GetClientCapabilitiesService(semanticRefreshEnabled: true); - var serverClient = new TestClient(); - var errorReporter = new TestErrorReporter(); - var optionsMonitor = GetOptionsMonitor(); - using var defaultWorkspaceChangedPublisher = new WorkspaceSemanticTokensRefreshPublisher(clientCapabilitiesService, serverClient, errorReporter, optionsMonitor); - var testAccessor = defaultWorkspaceChangedPublisher.GetTestAccessor(); - - // Act - defaultWorkspaceChangedPublisher.EnqueueWorkspaceSemanticTokensRefresh(); - testAccessor.WaitForEmpty(); - - // Assert - Assert.Collection(serverClient.Requests, - r => Assert.Equal(Methods.WorkspaceSemanticTokensRefreshName, r.Method)); - } - - [Fact] - public void PublishWorkspaceChanged_DebouncesWorkspaceRefreshRequest() - { - // Arrange - var clientCapabilitiesService = GetClientCapabilitiesService(semanticRefreshEnabled: true); - var serverClient = new TestClient(); - var errorReporter = new TestErrorReporter(); - var optionsMonitor = GetOptionsMonitor(); - using var defaultWorkspaceChangedPublisher = new WorkspaceSemanticTokensRefreshPublisher(clientCapabilitiesService, serverClient, errorReporter, optionsMonitor); - var testAccessor = defaultWorkspaceChangedPublisher.GetTestAccessor(); - - // Act - defaultWorkspaceChangedPublisher.EnqueueWorkspaceSemanticTokensRefresh(); - defaultWorkspaceChangedPublisher.EnqueueWorkspaceSemanticTokensRefresh(); - testAccessor.WaitForEmpty(); - defaultWorkspaceChangedPublisher.EnqueueWorkspaceSemanticTokensRefresh(); - defaultWorkspaceChangedPublisher.EnqueueWorkspaceSemanticTokensRefresh(); - testAccessor.WaitForEmpty(); - - // Assert - Assert.Collection(serverClient.Requests, - r => Assert.Equal(Methods.WorkspaceSemanticTokensRefreshName, r.Method), - r => Assert.Equal(Methods.WorkspaceSemanticTokensRefreshName, r.Method)); - } - - [Fact] - public async Task PublishWorkspaceChanged_SendsWorkspaceRefreshRequest_WhenOptionChanges() - { - // Arrange - var clientCapabilitiesService = GetClientCapabilitiesService(semanticRefreshEnabled: true); - var serverClient = new TestClient(); - var errorReporter = new TestErrorReporter(); - var optionsMonitor = GetOptionsMonitor(withCSharpBackground: true); - using var defaultWorkspaceChangedPublisher = new WorkspaceSemanticTokensRefreshPublisher(clientCapabilitiesService, serverClient, errorReporter, optionsMonitor); - var testAccessor = defaultWorkspaceChangedPublisher.GetTestAccessor(); - - // Act - await optionsMonitor.UpdateAsync(DisposalToken); - testAccessor.WaitForEmpty(); - - // Assert - Assert.Collection(serverClient.Requests, - r => Assert.Equal(Methods.WorkspaceSemanticTokensRefreshName, r.Method)); - } - - private static RazorLSPOptionsMonitor GetOptionsMonitor(bool withCSharpBackground = false) - { - var configurationSyncService = new Mock(MockBehavior.Strict); - - var options = RazorLSPOptions.Default with { ColorBackground = withCSharpBackground }; - configurationSyncService - .Setup(c => c.GetLatestOptionsAsync(It.IsAny())) - .Returns(Task.FromResult(options)); - - var optionsMonitorCache = new OptionsCache(); - - var optionsMonitor = TestRazorLSPOptionsMonitor.Create( - configurationSyncService.Object, - optionsMonitorCache); - - return optionsMonitor; - } - - private static IClientCapabilitiesService GetClientCapabilitiesService(bool semanticRefreshEnabled) - { - var clientCapabilities = new VSInternalClientCapabilities - { - Workspace = new WorkspaceClientCapabilities - { - SemanticTokens = new SemanticTokensWorkspaceSetting - { - RefreshSupport = semanticRefreshEnabled - }, - } - }; - - return new TestClientCapabilitiesService(clientCapabilities); - } - - private class TestErrorReporter : IErrorReporter - { - public void ReportError(Exception exception) => throw new NotImplementedException(); - public void ReportError(Exception exception, IProjectSnapshot? project) => throw new NotImplementedException(); - public void ReportError(Exception exception, Project workspaceProject) => throw new NotImplementedException(); - } -} diff --git a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/WorkspaceSemanticTokensRefreshTriggerTest.cs b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/WorkspaceSemanticTokensRefreshTriggerTest.cs index 143887e4274..da819d2d94b 100644 --- a/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/WorkspaceSemanticTokensRefreshTriggerTest.cs +++ b/src/Razor/test/Microsoft.AspNetCore.Razor.LanguageServer.Test/WorkspaceSemanticTokensRefreshTriggerTest.cs @@ -35,12 +35,12 @@ protected override Task InitializeAsync() } [Fact] - public async Task PublishesOnWorkspaceUpdate() + public async Task NotifiesOnWorkspaceUpdate() { // Arrange - var publisher = new StrictMock(); + var publisher = new StrictMock(); publisher - .Setup(w => w.EnqueueWorkspaceSemanticTokensRefresh()) + .Setup(w => w.NotifyWorkspaceSemanticTokensRefresh()) .Verifiable(); var refreshTrigger = new TestWorkspaceSemanticTokensRefreshTrigger(publisher.Object, _projectManager); @@ -56,7 +56,7 @@ await _projectManager.UpdateAsync(updater => } private class TestWorkspaceSemanticTokensRefreshTrigger( - IWorkspaceSemanticTokensRefreshPublisher publisher, + IWorkspaceSemanticTokensRefreshNotifier publisher, IProjectSnapshotManager projectManager) : WorkspaceSemanticTokensRefreshTrigger(publisher, projectManager); } From 7b4733d77091380fd719efe4451ab5fde6b90f8f Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Thu, 21 Mar 2024 11:04:47 -0700 Subject: [PATCH 04/17] Rewrite WorkspaceProjectStateChangeDetector to use AsyncBatchingWorkQueue --- .../Extensions/SolutionExtensions.cs | 20 +- .../WorkspaceProjectStateChangeDetector.cs | 598 ++++++++---------- .../TestProjectWorkspaceStateGenerator.cs | 33 +- ...WorkspaceProjectStateChangeDetectorTest.cs | 283 ++++----- ...UpdatesProjectSnapshotChangeTriggerTest.cs | 10 +- 5 files changed, 416 insertions(+), 528 deletions(-) diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Extensions/SolutionExtensions.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Extensions/SolutionExtensions.cs index f3f8ef0a80c..5030209aee7 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Extensions/SolutionExtensions.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Extensions/SolutionExtensions.cs @@ -9,23 +9,7 @@ internal static class SolutionExtensions { internal static Project GetRequiredProject(this Solution solution, ProjectId projectId) { - if (solution is null) - { - throw new ArgumentNullException(nameof(solution)); - } - - if (projectId is null) - { - throw new ArgumentNullException(nameof(projectId)); - } - - var project = solution.GetProject(projectId); - - if (project is null) - { - throw new InvalidOperationException($"The projectId {projectId} did not exist in {solution}."); - } - - return project; + return solution.GetProject(projectId) + ?? throw new InvalidOperationException($"The projectId {projectId} did not exist in {solution}."); } } diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.cs index 8478c6d6585..085ae6b5b1a 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.cs @@ -2,6 +2,8 @@ // Licensed under the MIT license. See License.txt in the project root for license information. using System; +using System.Collections.Generic; +using System.Collections.Immutable; using System.ComponentModel.Composition; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; @@ -10,337 +12,273 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Razor; using Microsoft.AspNetCore.Razor.Language.Components; +using Microsoft.AspNetCore.Razor.PooledObjects; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Razor; +using Microsoft.CodeAnalysis.Razor.Logging; using Microsoft.CodeAnalysis.Razor.ProjectSystem; +using Microsoft.CodeAnalysis.Razor.Utilities; using Microsoft.CodeAnalysis.Razor.Workspaces; +using Microsoft.Extensions.Logging; namespace Microsoft.VisualStudio.LanguageServices.Razor; [Export(typeof(IRazorStartupService))] internal class WorkspaceProjectStateChangeDetector : IRazorStartupService, IDisposable { - private static readonly TimeSpan s_batchingDelay = TimeSpan.FromSeconds(1); + private static readonly TimeSpan s_delay = TimeSpan.FromSeconds(1); - private readonly IProjectWorkspaceStateGenerator _workspaceStateGenerator; + private readonly IProjectWorkspaceStateGenerator _generator; private readonly IProjectSnapshotManager _projectManager; private readonly LanguageServerFeatureOptions _options; - private readonly IWorkspaceProvider _workspaceProvider; + private readonly Workspace _workspace; private readonly ProjectSnapshotManagerDispatcher _dispatcher; - private readonly IErrorReporter _errorReporter; + private readonly ILogger _logger; - private readonly object _disposedLock = new(); - private readonly object _workQueueAccessLock = new(); + private readonly CancellationTokenSource _disposeTokenSource; + private readonly AsyncBatchingWorkQueue<(Project?, IProjectSnapshot)> _workQueue; + private readonly HashSet _processedIds; - private BatchingWorkQueue? _workQueue; - private bool _disposed; + private WorkspaceChangedListener? _workspaceChangedListener; [ImportingConstructor] public WorkspaceProjectStateChangeDetector( - IProjectWorkspaceStateGenerator workspaceStateGenerator, + IProjectWorkspaceStateGenerator generator, IProjectSnapshotManager projectManager, LanguageServerFeatureOptions options, IWorkspaceProvider workspaceProvider, ProjectSnapshotManagerDispatcher dispatcher, - IErrorReporter errorReporter) + IRazorLoggerFactory loggerFactory) { - _workspaceStateGenerator = workspaceStateGenerator; + _generator = generator; _projectManager = projectManager; _options = options; - _workspaceProvider = workspaceProvider; _dispatcher = dispatcher; - _errorReporter = errorReporter; + _logger = loggerFactory.CreateLogger(); - Initialize(); - } - - // Internal for testing - internal WorkspaceProjectStateChangeDetector( - IProjectWorkspaceStateGenerator workspaceStateGenerator, - IProjectSnapshotManager projectManager, - LanguageServerFeatureOptions options, - IWorkspaceProvider workspaceProvider, - IErrorReporter errorReporter, - ProjectSnapshotManagerDispatcher dispatcher, - BatchingWorkQueue workQueue) - { - _workspaceStateGenerator = workspaceStateGenerator; - _projectManager = projectManager; - _options = options; - _workspaceProvider = workspaceProvider; - _dispatcher = dispatcher; - _errorReporter = errorReporter; - _workQueue = workQueue; - - Initialize(); - } - - private void Initialize() - { - EnsureWorkQueue(); + _disposeTokenSource = new(); + _processedIds = new(FilePathComparer.Instance); + _workQueue = new AsyncBatchingWorkQueue<(Project?, IProjectSnapshot)>(s_delay, ProcessBatchAsync, _disposeTokenSource.Token); _projectManager.Changed += ProjectManager_Changed; - var workspace = _workspaceProvider.GetWorkspace(); - workspace.WorkspaceChanged += Workspace_WorkspaceChanged; + _workspace = workspaceProvider.GetWorkspace(); + _workspace.WorkspaceChanged += Workspace_WorkspaceChanged; // This will usually no-op, in the case that another project snapshot change trigger // immediately adds projects we want to be able to handle those projects. - InitializeSolution(workspace.CurrentSolution); + InitializeSolution(_workspace.CurrentSolution); } - public ManualResetEventSlim? NotifyWorkspaceChangedEventComplete { get; set; } - - private void EnsureWorkQueue() + public void Dispose() { - lock (_disposedLock) - { - lock (_workQueueAccessLock) - { - if (_projectManager is not { } projectManager || _disposed) - { - return; - } + _projectManager.Changed -= ProjectManager_Changed; + _workspace.WorkspaceChanged -= Workspace_WorkspaceChanged; - _workQueue ??= new BatchingWorkQueue( - s_batchingDelay, - FilePathComparer.Instance, - _errorReporter); - } - } + _disposeTokenSource.Cancel(); + _disposeTokenSource.Dispose(); } - // Internal for testing, virtual for temporary VSCode workaround - internal virtual void Workspace_WorkspaceChanged(object? sender, WorkspaceChangeEventArgs e) + private async ValueTask ProcessBatchAsync(ImmutableArray<(Project? Project, IProjectSnapshot ProjectSnapshot)> items, CancellationToken token) { - _ = Workspace_WorkspaceChangedAsync(e, CancellationToken.None); - } + Debug.Assert(_processedIds.Count == 0); - private async Task Workspace_WorkspaceChangedAsync(WorkspaceChangeEventArgs e, CancellationToken cancellationToken) - { - try + using var itemsToProcess = new PooledArrayBuilder<(Project?, IProjectSnapshot)>(capacity: items.Length); + + // We loop through the items in reverse order to de-dupe them and only handle the + // most recent items. So, we push them onto a stack so that we process items + // in the original order. + for (var i = items.Length - 1; i >= 0; i--) { - // Method needs to be run on the project snapshot manager's specialized thread - // due to project snapshot manager access. - switch (e.Kind) + var (project, projectSnapshot) = items[i]; + var key = projectSnapshot.Key.Id; + + if (_processedIds.Add(key)) { - case WorkspaceChangeKind.ProjectAdded: - await _dispatcher - .RunAsync( - static state => - { - var (@this, eventArgs) = state; - var projectId = eventArgs.ProjectId.AssumeNotNull(); - var newSolution = eventArgs.NewSolution; - - var project = newSolution.GetRequiredProject(projectId); - @this.EnqueueUpdateOnProjectAndDependencies(project, newSolution); - }, - (self: this, eventArgs: e), - cancellationToken) - .ConfigureAwait(false); - break; - - case WorkspaceChangeKind.ProjectChanged: - case WorkspaceChangeKind.ProjectReloaded: - await _dispatcher - .RunAsync( - static state => - { - var (@this, eventArgs) = state; - var projectId = eventArgs.ProjectId.AssumeNotNull(); - var newSolution = eventArgs.NewSolution; - - var project = newSolution.GetRequiredProject(projectId); - @this.EnqueueUpdateOnProjectAndDependencies(project, newSolution); - }, - (self: this, eventArgs: e), - cancellationToken) - .ConfigureAwait(false); - break; - - case WorkspaceChangeKind.ProjectRemoved: - await _dispatcher - .RunAsync( - static state => - { - var (@this, eventArgs) = state; - var projectId = eventArgs.ProjectId.AssumeNotNull(); - var oldSolution = eventArgs.OldSolution; - - var project = oldSolution.GetRequiredProject(projectId); - if (@this.TryGetProjectSnapshot(project, out var projectSnapshot)) - { - @this.EnqueueUpdateOnProjectAndDependencies(projectId, project: null, oldSolution, projectSnapshot); - } - }, - (self: this, eventArgs: e), - cancellationToken) - .ConfigureAwait(false); - break; - - case WorkspaceChangeKind.DocumentAdded: - await _dispatcher - .RunAsync( - static state => - { - var (@this, eventArgs) = state; - var projectId = eventArgs.ProjectId.AssumeNotNull(); - var documentId = eventArgs.DocumentId.AssumeNotNull(); - var newSolution = eventArgs.NewSolution; - - // This is the case when a component declaration file changes on disk. We have an MSBuild - // generator configured by the SDK that will poke these files on disk when a component - // is saved, or loses focus in the editor. - var project = newSolution.GetRequiredProject(projectId); - var newDocument = project.GetDocument(documentId); - - if (newDocument?.FilePath is null) - { - return; - } - - if (@this.IsRazorFileOrRazorVirtual(newDocument)) - { - @this.EnqueueUpdateOnProjectAndDependencies(project, newSolution); - return; - } - - // We now know we're not operating directly on a Razor file. However, it's possible the user - // is operating on a partial class that is associated with a Razor file. - if (IsPartialComponentClass(newDocument)) - { - @this.EnqueueUpdateOnProjectAndDependencies(project, newSolution); - } - }, - (self: this, eventArgs: e), - cancellationToken) - .ConfigureAwait(false); - break; - - case WorkspaceChangeKind.DocumentRemoved: - await _dispatcher - .RunAsync( - static state => - { - var (@this, eventArgs) = state; - var projectId = eventArgs.ProjectId.AssumeNotNull(); - var documentId = eventArgs.DocumentId.AssumeNotNull(); - var oldSolution = eventArgs.OldSolution; - var newSolution = eventArgs.NewSolution; - - var project = oldSolution.GetRequiredProject(projectId); - var removedDocument = project.GetRequiredDocument(documentId); - - if (removedDocument.FilePath is null) - { - return; - } - - if (@this.IsRazorFileOrRazorVirtual(removedDocument)) - { - @this.EnqueueUpdateOnProjectAndDependencies(project, newSolution); - return; - } - - // We now know we're not operating directly on a Razor file. However, it's possible the user - // is operating on a partial class that is associated with a Razor file. - - if (IsPartialComponentClass(removedDocument)) - { - @this.EnqueueUpdateOnProjectAndDependencies(project, newSolution); - } - }, - (self: this, eventArgs: e), - cancellationToken) - .ConfigureAwait(false); - break; - - case WorkspaceChangeKind.DocumentChanged: - case WorkspaceChangeKind.DocumentReloaded: - await _dispatcher - .RunAsync( - static state => - { - var (@this, eventArgs) = state; - var projectId = eventArgs.ProjectId.AssumeNotNull(); - var documentId = eventArgs.DocumentId.AssumeNotNull(); - var oldSolution = eventArgs.OldSolution; - var newSolution = eventArgs.NewSolution; - - // This is the case when a component declaration file changes on disk. We have an MSBuild - // generator configured by the SDK that will poke these files on disk when a component - // is saved, or loses focus in the editor. - var project = oldSolution.GetRequiredProject(projectId); - var document = project.GetRequiredDocument(documentId); - - if (document.FilePath is null) - { - return; - } - - if (@this.IsRazorFileOrRazorVirtual(document)) - { - var newProject = newSolution.GetRequiredProject(projectId); - @this.EnqueueUpdateOnProjectAndDependencies(newProject, newSolution); - return; - } - - // We now know we're not operating directly on a Razor file. However, it's possible the user is operating on a partial class that is associated with a Razor file. - - if (IsPartialComponentClass(document)) - { - var newProject = newSolution.GetRequiredProject(projectId); - @this.EnqueueUpdateOnProjectAndDependencies(newProject, newSolution); - } - }, - (self: this, eventArgs: e), - cancellationToken) - .ConfigureAwait(false); - break; - - case WorkspaceChangeKind.SolutionAdded: - case WorkspaceChangeKind.SolutionChanged: - case WorkspaceChangeKind.SolutionCleared: - case WorkspaceChangeKind.SolutionReloaded: - case WorkspaceChangeKind.SolutionRemoved: - await _dispatcher - .RunAsync( - static state => - { - var (@this, eventArgs) = state; - var oldSolution = eventArgs.OldSolution; - var newSolution = eventArgs.NewSolution; - - foreach (var project in oldSolution.Projects) - { - if (@this.TryGetProjectSnapshot(project, out var projectSnapshot)) - { - @this.EnqueueUpdate(project: null, projectSnapshot); - } - } - - @this.InitializeSolution(newSolution); - }, - (self: this, eventArgs: e), - cancellationToken) - .ConfigureAwait(false); - break; + itemsToProcess.Push((project, projectSnapshot)); } + } - // Let tests know that this event has completed - NotifyWorkspaceChangedEventComplete?.Set(); + while (itemsToProcess.Count > 0) + { + var (project, projectSnapshot) = itemsToProcess.Pop(); + + _logger.LogTrace("Process update: {DisplayName}", projectSnapshot.DisplayName); + + await _dispatcher.RunAsync( + static state => + { + var (generator, project, projectSnapshot, token) = state; + generator.Update(project, projectSnapshot, token); + }, + state: (_generator, project, projectSnapshot, token), + token); } - catch (Exception ex) + + _processedIds.Clear(); + } + + private void Workspace_WorkspaceChanged(object? sender, WorkspaceChangeEventArgs e) + { + _logger.LogTrace("Workspace change: {Kind}", e.Kind); + + switch (e.Kind) { - Debug.Fail($""" - WorkspaceProjectStateChangeDetector.Workspace_WorkspaceChanged threw exception: - {ex.Message} - Stack trace: - {ex.StackTrace} - """); + case WorkspaceChangeKind.ProjectAdded: + case WorkspaceChangeKind.ProjectChanged: + case WorkspaceChangeKind.ProjectReloaded: + { + var projectId = e.ProjectId.AssumeNotNull(); + var newSolution = e.NewSolution; + + var project = newSolution.GetRequiredProject(projectId); + EnqueueUpdateOnProjectAndDependencies(project, newSolution); + } + + break; + + case WorkspaceChangeKind.ProjectRemoved: + { + var projectId = e.ProjectId.AssumeNotNull(); + var oldSolution = e.OldSolution; + + var project = oldSolution.GetRequiredProject(projectId); + + if (TryGetProjectSnapshot(project, out var projectSnapshot)) + { + EnqueueUpdateOnProjectAndDependencies(projectId, project: null, oldSolution, projectSnapshot); + } + } + + break; + + case WorkspaceChangeKind.DocumentAdded: + { + var projectId = e.ProjectId.AssumeNotNull(); + var documentId = e.DocumentId.AssumeNotNull(); + var newSolution = e.NewSolution; + + // This is the case when a component declaration file changes on disk. We have an MSBuild + // generator configured by the SDK that will poke these files on disk when a component + // is saved, or loses focus in the editor. + var project = newSolution.GetRequiredProject(projectId); + + if (project.GetDocument(documentId) is not Document newDocument || + newDocument.FilePath is null) + { + return; + } + + if (IsRazorFileOrRazorVirtual(newDocument)) + { + EnqueueUpdateOnProjectAndDependencies(project, newSolution); + return; + } + + // We now know we're not operating directly on a Razor file. However, it's possible the user + // is operating on a partial class that is associated with a Razor file. + if (IsPartialComponentClass(newDocument)) + { + EnqueueUpdateOnProjectAndDependencies(project, newSolution); + } + } + + break; + + case WorkspaceChangeKind.DocumentRemoved: + { + var projectId = e.ProjectId.AssumeNotNull(); + var documentId = e.DocumentId.AssumeNotNull(); + var oldSolution = e.OldSolution; + var newSolution = e.NewSolution; + + var project = oldSolution.GetRequiredProject(projectId); + var removedDocument = project.GetRequiredDocument(documentId); + + if (removedDocument.FilePath is null) + { + return; + } + + if (IsRazorFileOrRazorVirtual(removedDocument)) + { + EnqueueUpdateOnProjectAndDependencies(project, newSolution); + return; + } + + // We now know we're not operating directly on a Razor file. However, it's possible the user + // is operating on a partial class that is associated with a Razor file. + + if (IsPartialComponentClass(removedDocument)) + { + EnqueueUpdateOnProjectAndDependencies(project, newSolution); + } + } + + break; + + case WorkspaceChangeKind.DocumentChanged: + case WorkspaceChangeKind.DocumentReloaded: + { + var projectId = e.ProjectId.AssumeNotNull(); + var documentId = e.DocumentId.AssumeNotNull(); + var oldSolution = e.OldSolution; + var newSolution = e.NewSolution; + + // This is the case when a component declaration file changes on disk. We have an MSBuild + // generator configured by the SDK that will poke these files on disk when a component + // is saved, or loses focus in the editor. + var project = oldSolution.GetRequiredProject(projectId); + var document = project.GetRequiredDocument(documentId); + + if (document.FilePath is null) + { + return; + } + + if (IsRazorFileOrRazorVirtual(document)) + { + var newProject = newSolution.GetRequiredProject(projectId); + EnqueueUpdateOnProjectAndDependencies(newProject, newSolution); + return; + } + + // We now know we're not operating directly on a Razor file. However, it's possible the user + // is operating on a partial class that is associated with a Razor file. + if (IsPartialComponentClass(document)) + { + var newProject = newSolution.GetRequiredProject(projectId); + EnqueueUpdateOnProjectAndDependencies(newProject, newSolution); + } + } + + break; + + case WorkspaceChangeKind.SolutionAdded: + case WorkspaceChangeKind.SolutionChanged: + case WorkspaceChangeKind.SolutionCleared: + case WorkspaceChangeKind.SolutionReloaded: + case WorkspaceChangeKind.SolutionRemoved: + { + var oldSolution = e.OldSolution; + var newSolution = e.NewSolution; + + foreach (var project in oldSolution.Projects) + { + if (TryGetProjectSnapshot(project, out var projectSnapshot)) + { + EnqueueUpdate(project: null, projectSnapshot); + } + } + + InitializeSolution(newSolution); + } + + break; } + + _workspaceChangedListener?.WorkspaceChanged(e.Kind); } private bool IsRazorFileOrRazorVirtual(Document document) @@ -405,8 +343,7 @@ internal static bool IsPartialComponentClass(Document document) return false; } - // Virtual for temporary VSCode workaround - protected virtual void InitializeSolution(Solution solution) + private void InitializeSolution(Solution solution) { foreach (var project in solution.Projects) { @@ -422,7 +359,7 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs e) // Don't do any work if the solution is closing. Any work in the queue will be cancelled on disposal if (e.SolutionIsClosing) { - ClearWorkQueue(); + _workQueue.CancelExistingWork(); return; } @@ -431,8 +368,7 @@ private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs e) case ProjectChangeKind.ProjectAdded: case ProjectChangeKind.DocumentRemoved: case ProjectChangeKind.DocumentAdded: - var workspace = _workspaceProvider.GetWorkspace(); - var currentSolution = workspace.CurrentSolution; + var currentSolution = _workspace.CurrentSolution; var associatedWorkspaceProject = currentSolution.Projects .FirstOrDefault(project => e.ProjectKey == ProjectKey.From(project)); @@ -481,11 +417,8 @@ private void EnqueueUpdateOnProjectAndDependencies(ProjectId projectId, Project? private void EnqueueUpdate(Project? project, IProjectSnapshot projectSnapshot) { - var workItem = new UpdateWorkspaceWorkItem(project, projectSnapshot, _workspaceStateGenerator, _dispatcher); - - EnsureWorkQueue(); - - _workQueue?.Enqueue(projectSnapshot.Key.Id, workItem); + _logger.LogTrace("Enqueue update: {DisplayName}", projectSnapshot.DisplayName); + _workQueue.AddWork((project, projectSnapshot)); } private bool TryGetProjectSnapshot(Project? project, [NotNullWhen(true)] out IProjectSnapshot? projectSnapshot) @@ -501,60 +434,65 @@ private bool TryGetProjectSnapshot(Project? project, [NotNullWhen(true)] out IPr return _projectManager.TryGetLoadedProject(projectKey, out projectSnapshot); } - public void Dispose() + internal TestAccessor GetTestAccessor() => new(this); + + internal sealed class TestAccessor(WorkspaceProjectStateChangeDetector instance) { - lock (_disposedLock) + public void CancelExistingWork() + { + instance._workQueue.CancelExistingWork(); + } + + public async Task WaitUntilCurrentBatchCompletesAsync() + { + await instance._workQueue.WaitUntilCurrentBatchCompletesAsync(); + } + + public Task ListenForWorkspaceChangesAsync(params WorkspaceChangeKind[] kinds) { - if (_disposed) + if (instance._workspaceChangedListener is not null) { - return; + throw new InvalidOperationException($"There's already a {nameof(WorkspaceChangedListener)} installed."); } - _disposed = true; - ClearWorkQueue(); + var listener = new WorkspaceChangedListener(kinds.ToImmutableArray()); + instance._workspaceChangedListener = listener; + + return listener.Task; } - } - private void ClearWorkQueue() - { - lock (_workQueueAccessLock) + public void WorkspaceChanged(WorkspaceChangeEventArgs e) { - _workQueue?.Dispose(); - _workQueue = null; + instance.Workspace_WorkspaceChanged(instance, e); } } - private class UpdateWorkspaceWorkItem : BatchableWorkItem + private class WorkspaceChangedListener(ImmutableArray kinds) { - private readonly Project? _workspaceProject; - private readonly IProjectSnapshot _projectSnapshot; - private readonly IProjectWorkspaceStateGenerator _workspaceStateGenerator; - private readonly ProjectSnapshotManagerDispatcher _dispatcher; - - public UpdateWorkspaceWorkItem( - Project? workspaceProject, - IProjectSnapshot projectSnapshot, - IProjectWorkspaceStateGenerator workspaceStateGenerator, - ProjectSnapshotManagerDispatcher dispatcher) - { - _workspaceProject = workspaceProject; - _projectSnapshot = projectSnapshot ?? throw new ArgumentNullException(nameof(projectSnapshot)); - _workspaceStateGenerator = workspaceStateGenerator ?? throw new ArgumentNullException(nameof(workspaceStateGenerator)); - _dispatcher = dispatcher ?? throw new ArgumentNullException(nameof(dispatcher)); - } + private readonly ImmutableArray _kinds = kinds; + private readonly TaskCompletionSource _completionSource = new(); + private int _index; + + public Task Task => _completionSource.Task; - public override ValueTask ProcessAsync(CancellationToken cancellationToken) + public void WorkspaceChanged(WorkspaceChangeKind kind) { - var task = _dispatcher.RunAsync( - static state => - { - var (@this, cancellationToken) = state; - @this._workspaceStateGenerator.Update(@this._workspaceProject, @this._projectSnapshot, cancellationToken); - }, - state: (this, cancellationToken), - cancellationToken); + if (_index == _kinds.Length) + { + throw new InvalidOperationException($"Expected {_kinds.Length} WorkspaceChanged events but received another {kind}."); + } - return new ValueTask(task); + if (_kinds[_index] != kind) + { + throw new InvalidOperationException($"Expected WorkspaceChanged event #{_index + 1} to be {_kinds[_index]} but it was {kind}."); + } + + _index++; + + if (_index == _kinds.Length) + { + _completionSource.TrySetResult(true); + } } } } diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/TestProjectWorkspaceStateGenerator.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/TestProjectWorkspaceStateGenerator.cs index d51cffe283d..d21c46efb42 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/TestProjectWorkspaceStateGenerator.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/TestProjectWorkspaceStateGenerator.cs @@ -1,12 +1,12 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the MIT license. See License.txt in the project root for license information. -#nullable disable - using System.Collections.Generic; using Microsoft.CodeAnalysis.Razor.ProjectSystem; using Microsoft.CodeAnalysis; using System.Threading; +using System.Text; +using Microsoft.AspNetCore.Razor.PooledObjects; namespace Microsoft.VisualStudio.LanguageServices.Razor.Test; @@ -19,18 +19,39 @@ public TestProjectWorkspaceStateGenerator() _updates = new List(); } - public IReadOnlyList UpdateQueue => _updates; + public IReadOnlyList Updates => _updates; - public void Update(Project workspaceProject, IProjectSnapshot projectSnapshot, CancellationToken cancellationToken) + public void Update(Project? workspaceProject, IProjectSnapshot projectSnapshot, CancellationToken cancellationToken) { var update = new TestUpdate(workspaceProject, projectSnapshot, cancellationToken); _updates.Add(update); } - public void ClearQueue() + public void Clear() { _updates.Clear(); } - public record TestUpdate(Project WorkspaceProject, IProjectSnapshot ProjectSnapshot, CancellationToken CancellationToken); + public record TestUpdate(Project? WorkspaceProject, IProjectSnapshot ProjectSnapshot, CancellationToken CancellationToken) + { + public override string ToString() + { + using var _ = StringBuilderPool.GetPooledObject(out var builder); + + builder.Append($"{{{nameof(WorkspaceProject)} = "); + + if (WorkspaceProject is null) + { + builder.Append(""); + } + else + { + builder.Append(WorkspaceProject.Name); + } + + builder.Append($", {nameof(ProjectSnapshot)} = {ProjectSnapshot.DisplayName}}}"); + + return builder.ToString(); + } + } } diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/WorkspaceProjectStateChangeDetectorTest.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/WorkspaceProjectStateChangeDetectorTest.cs index 3833c861c51..2aeb63e9154 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/WorkspaceProjectStateChangeDetectorTest.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/ProjectSystem/WorkspaceProjectStateChangeDetectorTest.cs @@ -3,14 +3,11 @@ #nullable disable -using System; -using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Razor.Language.Components; using Microsoft.AspNetCore.Razor.Test.Common.VisualStudio; using Microsoft.AspNetCore.Razor.Test.Common.Workspaces; using Microsoft.CodeAnalysis.CSharp; -using Microsoft.CodeAnalysis.Razor.Workspaces; using Microsoft.CodeAnalysis.Text; using Microsoft.VisualStudio.LanguageServices.Razor; using Microsoft.VisualStudio.LanguageServices.Razor.Test; @@ -23,8 +20,6 @@ namespace Microsoft.CodeAnalysis.Razor.ProjectSystem; public class WorkspaceProjectStateChangeDetectorTest : VisualStudioWorkspaceTestBase { - private readonly BatchingWorkQueue _workQueue; - private readonly BatchingWorkQueue.TestAccessor _workQueueTestAccessor; private readonly HostProject _hostProjectOne; private readonly HostProject _hostProjectTwo; private readonly HostProject _hostProjectThree; @@ -121,24 +116,20 @@ public WorkspaceProjectStateChangeDetectorTest(ITestOutputHelper testOutput) _hostProjectOne = new HostProject("One.csproj", "obj1", FallbackRazorConfiguration.MVC_1_1, "One"); _hostProjectTwo = new HostProject("Two.csproj", "obj2", FallbackRazorConfiguration.MVC_1_1, "Two"); _hostProjectThree = new HostProject("Three.csproj", "obj3", FallbackRazorConfiguration.MVC_1_1, "Three"); - - _workQueue = new BatchingWorkQueue(TimeSpan.FromMilliseconds(1), StringComparer.Ordinal, ErrorReporter); - AddDisposable(_workQueue); - - _workQueueTestAccessor = _workQueue.GetTestAccessor(); - _workQueue.GetTestAccessor().NotifyBackgroundWorkCompleted = null; - _workQueueTestAccessor.NotifyBackgroundWorkCompleted = new ManualResetEventSlim(initialState: false); } [UIFact] public async Task SolutionClosing_StopsActiveWork() { // Arrange - var workspaceStateGenerator = new TestProjectWorkspaceStateGenerator(); + var generator = new TestProjectWorkspaceStateGenerator(); var projectManager = CreateProjectSnapshotManager(); - var detector = new WorkspaceProjectStateChangeDetector(workspaceStateGenerator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, ErrorReporter, Dispatcher, _workQueue); - _workQueueTestAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); - _workQueueTestAccessor.NotifyBackgroundWorkStarting = new ManualResetEventSlim(initialState: false); + using var detector = new WorkspaceProjectStateChangeDetector(generator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, Dispatcher, LoggerFactory); + var detectorAccessor = detector.GetTestAccessor(); + + var workspaceChangedTask = detectorAccessor.ListenForWorkspaceChangesAsync( + WorkspaceChangeKind.ProjectAdded, + WorkspaceChangeKind.ProjectAdded); Workspace.TryApplyChanges(_solutionWithTwoProjects); @@ -147,8 +138,10 @@ await projectManager.UpdateAsync(updater => updater.ProjectAdded(_hostProjectOne); }); - workspaceStateGenerator.ClearQueue(); - _workQueueTestAccessor.NotifyBackgroundWorkStarting.Wait(); + await workspaceChangedTask; + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); + + generator.Clear(); // Act await projectManager.UpdateAsync(updater => @@ -160,14 +153,8 @@ await projectManager.UpdateAsync(updater => }); // Assert - // - // The change hasn't come through yet. - Assert.Empty(workspaceStateGenerator.UpdateQueue); - - _workQueueTestAccessor.BlockBackgroundWorkStart.Set(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); - Assert.Empty(workspaceStateGenerator.UpdateQueue); + Assert.Empty(generator.Updates); } [UITheory] @@ -177,14 +164,10 @@ await projectManager.UpdateAsync(updater => public async Task WorkspaceChanged_DocumentEvents_EnqueuesUpdatesForDependentProjects(WorkspaceChangeKind kind) { // Arrange - var workspaceStateGenerator = new TestProjectWorkspaceStateGenerator(); + var generator = new TestProjectWorkspaceStateGenerator(); var projectManager = CreateProjectSnapshotManager(); - var detector = new WorkspaceProjectStateChangeDetector(workspaceStateGenerator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, ErrorReporter, Dispatcher, _workQueue) - { - NotifyWorkspaceChangedEventComplete = new ManualResetEventSlim(initialState: false), - }; - - _workQueueTestAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); + using var detector = new WorkspaceProjectStateChangeDetector(generator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, Dispatcher, LoggerFactory); + var detectorAccessor = detector.GetTestAccessor(); await projectManager.UpdateAsync(updater => { @@ -195,9 +178,7 @@ await projectManager.UpdateAsync(updater => // Initialize with a project. This will get removed. var e = new WorkspaceChangeEventArgs(WorkspaceChangeKind.SolutionAdded, oldSolution: _emptySolution, newSolution: _solutionWithOneProject); - detector.Workspace_WorkspaceChanged(Workspace, e); - detector.NotifyWorkspaceChangedEventComplete.Wait(); - detector.NotifyWorkspaceChangedEventComplete.Reset(); + detectorAccessor.WorkspaceChanged(e); e = new WorkspaceChangeEventArgs(kind, oldSolution: _solutionWithOneProject, newSolution: _solutionWithDependentProject); @@ -206,18 +187,15 @@ await projectManager.UpdateAsync(updater => e = new WorkspaceChangeEventArgs(kind, oldSolution: _solutionWithDependentProject, newSolution: solution, projectId: _projectNumberThree.Id, documentId: _razorDocumentId); // Act - detector.Workspace_WorkspaceChanged(Workspace, e); - detector.NotifyWorkspaceChangedEventComplete.Wait(); + detectorAccessor.WorkspaceChanged(e); + + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); // Assert - Assert.Equal(3, _workQueueTestAccessor.Work.Count); - Assert.Contains(_workQueueTestAccessor.Work, u => u.Key == ProjectKey.From(_projectNumberOne).Id); - Assert.Contains(_workQueueTestAccessor.Work, u => u.Key == ProjectKey.From(_projectNumberTwo).Id); - Assert.Contains(_workQueueTestAccessor.Work, u => u.Key == ProjectKey.From(_projectNumberThree).Id); - - _workQueueTestAccessor.BlockBackgroundWorkStart.Set(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); - Assert.Empty(_workQueueTestAccessor.Work); + Assert.Equal(3, generator.Updates.Count); + Assert.Contains(generator.Updates, u => u.ProjectSnapshot.Key == ProjectKey.From(_projectNumberOne)); + Assert.Contains(generator.Updates, u => u.ProjectSnapshot.Key == ProjectKey.From(_projectNumberTwo)); + Assert.Contains(generator.Updates, u => u.ProjectSnapshot.Key == ProjectKey.From(_projectNumberThree)); } [UITheory] @@ -227,14 +205,10 @@ await projectManager.UpdateAsync(updater => public async Task WorkspaceChanged_ProjectEvents_EnqueuesUpdatesForDependentProjects(WorkspaceChangeKind kind) { // Arrange - var workspaceStateGenerator = new TestProjectWorkspaceStateGenerator(); + var generator = new TestProjectWorkspaceStateGenerator(); var projectManager = CreateProjectSnapshotManager(); - var detector = new WorkspaceProjectStateChangeDetector(workspaceStateGenerator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, ErrorReporter, Dispatcher, _workQueue) - { - NotifyWorkspaceChangedEventComplete = new ManualResetEventSlim(initialState: false), - }; - - _workQueueTestAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); + var detector = new WorkspaceProjectStateChangeDetector(generator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, Dispatcher, LoggerFactory); + var detectorAccessor = detector.GetTestAccessor(); await projectManager.UpdateAsync(updater => { @@ -245,9 +219,7 @@ await projectManager.UpdateAsync(updater => // Initialize with a project. This will get removed. var e = new WorkspaceChangeEventArgs(WorkspaceChangeKind.SolutionAdded, oldSolution: _emptySolution, newSolution: _solutionWithOneProject); - detector.Workspace_WorkspaceChanged(Workspace, e); - detector.NotifyWorkspaceChangedEventComplete.Wait(); - detector.NotifyWorkspaceChangedEventComplete.Reset(); + detectorAccessor.WorkspaceChanged(e); e = new WorkspaceChangeEventArgs(kind, oldSolution: _solutionWithOneProject, newSolution: _solutionWithDependentProject); @@ -256,18 +228,15 @@ await projectManager.UpdateAsync(updater => e = new WorkspaceChangeEventArgs(kind, oldSolution: _solutionWithDependentProject, newSolution: solution, projectId: _projectNumberThree.Id); // Act - detector.Workspace_WorkspaceChanged(Workspace, e); - detector.NotifyWorkspaceChangedEventComplete.Wait(); + detectorAccessor.WorkspaceChanged(e); + + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); // Assert - Assert.Equal(3, _workQueueTestAccessor.Work.Count); - Assert.Contains(_workQueueTestAccessor.Work, u => u.Key == ProjectKey.From(_projectNumberOne).Id); - Assert.Contains(_workQueueTestAccessor.Work, u => u.Key == ProjectKey.From(_projectNumberTwo).Id); - Assert.Contains(_workQueueTestAccessor.Work, u => u.Key == ProjectKey.From(_projectNumberThree).Id); - - _workQueueTestAccessor.BlockBackgroundWorkStart.Set(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); - Assert.Empty(_workQueueTestAccessor.Work); + Assert.Equal(3, generator.Updates.Count); + Assert.Contains(generator.Updates, u => u.ProjectSnapshot.Key == ProjectKey.From(_projectNumberOne)); + Assert.Contains(generator.Updates, u => u.ProjectSnapshot.Key == ProjectKey.From(_projectNumberTwo)); + Assert.Contains(generator.Updates, u => u.ProjectSnapshot.Key == ProjectKey.From(_projectNumberThree)); } [UITheory] @@ -279,12 +248,10 @@ await projectManager.UpdateAsync(updater => public async Task WorkspaceChanged_SolutionEvents_EnqueuesUpdatesForProjectsInSolution(WorkspaceChangeKind kind) { // Arrange - var workspaceStateGenerator = new TestProjectWorkspaceStateGenerator(); + var generator = new TestProjectWorkspaceStateGenerator(); var projectManager = CreateProjectSnapshotManager(); - var detector = new WorkspaceProjectStateChangeDetector(workspaceStateGenerator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, ErrorReporter, Dispatcher, _workQueue) - { - NotifyWorkspaceChangedEventComplete = new ManualResetEventSlim(initialState: false), - }; + var detector = new WorkspaceProjectStateChangeDetector(generator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, Dispatcher, LoggerFactory); + var detectorAccessor = detector.GetTestAccessor(); await projectManager.UpdateAsync(updater => { @@ -295,13 +262,13 @@ await projectManager.UpdateAsync(updater => var e = new WorkspaceChangeEventArgs(kind, oldSolution: _emptySolution, newSolution: _solutionWithTwoProjects); // Act - detector.Workspace_WorkspaceChanged(Workspace, e); - detector.NotifyWorkspaceChangedEventComplete.Wait(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); + detectorAccessor.WorkspaceChanged(e); + + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); // Assert Assert.Collection( - workspaceStateGenerator.UpdateQueue, + generator.Updates, p => Assert.Equal(_projectNumberOne.Id, p.WorkspaceProject.Id), p => Assert.Equal(_projectNumberTwo.Id, p.WorkspaceProject.Id)); } @@ -315,12 +282,10 @@ await projectManager.UpdateAsync(updater => public async Task WorkspaceChanged_SolutionEvents_EnqueuesStateClear_EnqueuesSolutionProjectUpdates(WorkspaceChangeKind kind) { // Arrange - var workspaceStateGenerator = new TestProjectWorkspaceStateGenerator(); + var generator = new TestProjectWorkspaceStateGenerator(); var projectManager = CreateProjectSnapshotManager(); - var detector = new WorkspaceProjectStateChangeDetector(workspaceStateGenerator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, ErrorReporter, Dispatcher, _workQueue) - { - NotifyWorkspaceChangedEventComplete = new ManualResetEventSlim(initialState: false), - }; + var detector = new WorkspaceProjectStateChangeDetector(generator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, Dispatcher, LoggerFactory); + var detectorAccessor = detector.GetTestAccessor(); await projectManager.UpdateAsync(updater => { @@ -331,22 +296,20 @@ await projectManager.UpdateAsync(updater => // Initialize with a project. This will get removed. var e = new WorkspaceChangeEventArgs(WorkspaceChangeKind.SolutionAdded, oldSolution: _emptySolution, newSolution: _solutionWithOneProject); - detector.Workspace_WorkspaceChanged(Workspace, e); - detector.NotifyWorkspaceChangedEventComplete.Wait(); - detector.NotifyWorkspaceChangedEventComplete.Reset(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Reset(); + detectorAccessor.WorkspaceChanged(e); + + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); e = new WorkspaceChangeEventArgs(kind, oldSolution: _solutionWithOneProject, newSolution: _solutionWithTwoProjects); // Act - detector.Workspace_WorkspaceChanged(Workspace, e); - detector.NotifyWorkspaceChangedEventComplete.Wait(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); + detectorAccessor.WorkspaceChanged(e); + + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); // Assert Assert.Collection( - workspaceStateGenerator.UpdateQueue, + generator.Updates, p => Assert.Equal(_projectNumberThree.Id, p.WorkspaceProject.Id), p => Assert.Null(p.WorkspaceProject), p => Assert.Equal(_projectNumberOne.Id, p.WorkspaceProject.Id), @@ -359,31 +322,34 @@ await projectManager.UpdateAsync(updater => public async Task WorkspaceChanged_ProjectChangeEvents_UpdatesProjectState_AfterDelay(WorkspaceChangeKind kind) { // Arrange - var workspaceStateGenerator = new TestProjectWorkspaceStateGenerator(); + var generator = new TestProjectWorkspaceStateGenerator(); var projectManager = CreateProjectSnapshotManager(); - var detector = new WorkspaceProjectStateChangeDetector(workspaceStateGenerator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, ErrorReporter, Dispatcher, _workQueue); - _workQueueTestAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); + var detector = new WorkspaceProjectStateChangeDetector(generator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, Dispatcher, LoggerFactory); + var detectorAccessor = detector.GetTestAccessor(); await projectManager.UpdateAsync(updater => { updater.ProjectAdded(_hostProjectOne); }); + // Stop any existing work and clear out any updates that we might have received. + detectorAccessor.CancelExistingWork(); + generator.Clear(); + + // Create a listener for the workspace change we're about to send. + var listenerTask = detectorAccessor.ListenForWorkspaceChangesAsync(kind); + var solution = _solutionWithTwoProjects.WithProjectAssemblyName(_projectNumberOne.Id, "Changed"); var e = new WorkspaceChangeEventArgs(kind, oldSolution: _solutionWithTwoProjects, newSolution: solution, projectId: _projectNumberOne.Id); // Act - detector.Workspace_WorkspaceChanged(Workspace, e); + detectorAccessor.WorkspaceChanged(e); - // Assert - // - // The change hasn't come through yet. - Assert.Empty(workspaceStateGenerator.UpdateQueue); - - _workQueueTestAccessor.BlockBackgroundWorkStart.Set(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); + await listenerTask; + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); - var update = Assert.Single(workspaceStateGenerator.UpdateQueue); + // Assert + var update = Assert.Single(generator.Updates); Assert.Equal(update.WorkspaceProject.Id, _projectNumberOne.Id); Assert.Equal(update.ProjectSnapshot.FilePath, _hostProjectOne.FilePath); } @@ -392,10 +358,10 @@ await projectManager.UpdateAsync(updater => public async Task WorkspaceChanged_DocumentChanged_BackgroundVirtualCS_UpdatesProjectState_AfterDelay() { // Arrange - var workspaceStateGenerator = new TestProjectWorkspaceStateGenerator(); + var generator = new TestProjectWorkspaceStateGenerator(); var projectManager = CreateProjectSnapshotManager(); - var detector = new WorkspaceProjectStateChangeDetector(workspaceStateGenerator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, ErrorReporter, Dispatcher, _workQueue); - _workQueueTestAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); + var detector = new WorkspaceProjectStateChangeDetector(generator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, Dispatcher, LoggerFactory); + var detectorAccessor = detector.GetTestAccessor(); Workspace.TryApplyChanges(_solutionWithTwoProjects); @@ -404,23 +370,18 @@ await projectManager.UpdateAsync(updater => updater.ProjectAdded(_hostProjectOne); }); - workspaceStateGenerator.ClearQueue(); + generator.Clear(); var solution = _solutionWithTwoProjects.WithDocumentText(_backgroundVirtualCSharpDocumentId, SourceText.From("public class Foo{}")); var e = new WorkspaceChangeEventArgs(WorkspaceChangeKind.DocumentChanged, oldSolution: _solutionWithTwoProjects, newSolution: solution, projectId: _projectNumberOne.Id, _backgroundVirtualCSharpDocumentId); // Act - detector.Workspace_WorkspaceChanged(Workspace, e); - - // Assert - // - // The change hasn't come through yet. - Assert.Empty(workspaceStateGenerator.UpdateQueue); + detectorAccessor.WorkspaceChanged(e); - _workQueueTestAccessor.BlockBackgroundWorkStart.Set(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); - var update = Assert.Single(workspaceStateGenerator.UpdateQueue); + // Assert + var update = Assert.Single(generator.Updates); Assert.Equal(update.WorkspaceProject.Id, _projectNumberOne.Id); Assert.Equal(update.ProjectSnapshot.FilePath, _hostProjectOne.FilePath); } @@ -429,10 +390,10 @@ await projectManager.UpdateAsync(updater => public async Task WorkspaceChanged_DocumentChanged_CSHTML_UpdatesProjectState_AfterDelay() { // Arrange - var workspaceStateGenerator = new TestProjectWorkspaceStateGenerator(); + var generator = new TestProjectWorkspaceStateGenerator(); var projectManager = CreateProjectSnapshotManager(); - var detector = new WorkspaceProjectStateChangeDetector(workspaceStateGenerator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, ErrorReporter, Dispatcher, _workQueue); - _workQueueTestAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); + var detector = new WorkspaceProjectStateChangeDetector(generator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, Dispatcher, LoggerFactory); + var detectorAccessor = detector.GetTestAccessor(); Workspace.TryApplyChanges(_solutionWithTwoProjects); @@ -441,23 +402,18 @@ await projectManager.UpdateAsync(updater => updater.ProjectAdded(_hostProjectOne); }); - workspaceStateGenerator.ClearQueue(); + generator.Clear(); var solution = _solutionWithTwoProjects.WithDocumentText(_cshtmlDocumentId, SourceText.From("Hello World")); var e = new WorkspaceChangeEventArgs(WorkspaceChangeKind.DocumentChanged, oldSolution: _solutionWithTwoProjects, newSolution: solution, projectId: _projectNumberOne.Id, _cshtmlDocumentId); // Act - detector.Workspace_WorkspaceChanged(Workspace, e); - - // Assert - // - // The change hasn't come through yet. - Assert.Empty(workspaceStateGenerator.UpdateQueue); + detectorAccessor.WorkspaceChanged(e); - _workQueueTestAccessor.BlockBackgroundWorkStart.Set(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); - var update = Assert.Single(workspaceStateGenerator.UpdateQueue); + // Assert + var update = Assert.Single(generator.Updates); Assert.Equal(update.WorkspaceProject.Id, _projectNumberOne.Id); Assert.Equal(update.ProjectSnapshot.FilePath, _hostProjectOne.FilePath); } @@ -466,10 +422,10 @@ await projectManager.UpdateAsync(updater => public async Task WorkspaceChanged_DocumentChanged_Razor_UpdatesProjectState_AfterDelay() { // Arrange - var workspaceStateGenerator = new TestProjectWorkspaceStateGenerator(); + var generator = new TestProjectWorkspaceStateGenerator(); var projectManager = CreateProjectSnapshotManager(); - var detector = new WorkspaceProjectStateChangeDetector(workspaceStateGenerator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, ErrorReporter, Dispatcher, _workQueue); - _workQueueTestAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); + var detector = new WorkspaceProjectStateChangeDetector(generator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, Dispatcher, LoggerFactory); + var detectorAccessor = detector.GetTestAccessor(); Workspace.TryApplyChanges(_solutionWithTwoProjects); @@ -478,23 +434,18 @@ await projectManager.UpdateAsync(updater => updater.ProjectAdded(_hostProjectOne); }); - workspaceStateGenerator.ClearQueue(); + generator.Clear(); var solution = _solutionWithTwoProjects.WithDocumentText(_razorDocumentId, SourceText.From("Hello World")); var e = new WorkspaceChangeEventArgs(WorkspaceChangeKind.DocumentChanged, oldSolution: _solutionWithTwoProjects, newSolution: solution, projectId: _projectNumberOne.Id, _razorDocumentId); // Act - detector.Workspace_WorkspaceChanged(Workspace, e); + detectorAccessor.WorkspaceChanged(e); - // Assert - // - // The change hasn't come through yet. - Assert.Empty(workspaceStateGenerator.UpdateQueue); - - _workQueueTestAccessor.BlockBackgroundWorkStart.Set(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); - var update = Assert.Single(workspaceStateGenerator.UpdateQueue); + // Assert + var update = Assert.Single(generator.Updates); Assert.Equal(update.WorkspaceProject.Id, _projectNumberOne.Id); Assert.Equal(update.ProjectSnapshot.FilePath, _hostProjectOne.FilePath); } @@ -503,10 +454,10 @@ await projectManager.UpdateAsync(updater => public async Task WorkspaceChanged_DocumentChanged_PartialComponent_UpdatesProjectState_AfterDelay() { // Arrange - var workspaceStateGenerator = new TestProjectWorkspaceStateGenerator(); + var generator = new TestProjectWorkspaceStateGenerator(); var projectManager = CreateProjectSnapshotManager(); - var detector = new WorkspaceProjectStateChangeDetector(workspaceStateGenerator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, ErrorReporter, Dispatcher, _workQueue); - _workQueueTestAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); + var detector = new WorkspaceProjectStateChangeDetector(generator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, Dispatcher, LoggerFactory); + var detectorAccessor = detector.GetTestAccessor(); Workspace.TryApplyChanges(_solutionWithTwoProjects); @@ -515,7 +466,8 @@ await projectManager.UpdateAsync(updater => updater.ProjectAdded(_hostProjectOne); }); - workspaceStateGenerator.ClearQueue(); + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); + generator.Clear(); var sourceText = SourceText.From($$""" public partial class TestComponent : {{ComponentsApi.IComponent.MetadataName}} {} @@ -537,18 +489,12 @@ public interface IComponent {} var e = new WorkspaceChangeEventArgs(WorkspaceChangeKind.DocumentChanged, oldSolution: solution, newSolution: solution, projectId: _projectNumberOne.Id, _partialComponentClassDocumentId); // Act - detector.Workspace_WorkspaceChanged(Workspace, e); - - // Assert - // - // The change hasn't come through yet. - Assert.Empty(workspaceStateGenerator.UpdateQueue); + detectorAccessor.WorkspaceChanged(e); - _workQueueTestAccessor.BlockBackgroundWorkStart.Set(); + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); - - var update = Assert.Single(workspaceStateGenerator.UpdateQueue); + // Assert + var update = Assert.Single(generator.Updates); Assert.Equal(update.WorkspaceProject.Id, _projectNumberOne.Id); Assert.Equal(update.ProjectSnapshot.FilePath, _hostProjectOne.FilePath); } @@ -557,12 +503,10 @@ public interface IComponent {} public async Task WorkspaceChanged_ProjectRemovedEvent_QueuesProjectStateRemoval() { // Arrange - var workspaceStateGenerator = new TestProjectWorkspaceStateGenerator(); + var generator = new TestProjectWorkspaceStateGenerator(); var projectManager = CreateProjectSnapshotManager(); - var detector = new WorkspaceProjectStateChangeDetector(workspaceStateGenerator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, ErrorReporter, Dispatcher, _workQueue) - { - NotifyWorkspaceChangedEventComplete = new ManualResetEventSlim(initialState: false), - }; + var detector = new WorkspaceProjectStateChangeDetector(generator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, Dispatcher, LoggerFactory); + var detectorAccessor = detector.GetTestAccessor(); await projectManager.UpdateAsync(updater => { @@ -574,12 +518,12 @@ await projectManager.UpdateAsync(updater => var e = new WorkspaceChangeEventArgs(WorkspaceChangeKind.ProjectRemoved, oldSolution: _solutionWithTwoProjects, newSolution: solution, projectId: _projectNumberOne.Id); // Act - detector.Workspace_WorkspaceChanged(Workspace, e); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); + detectorAccessor.WorkspaceChanged(e); + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); // Assert Assert.Single( - workspaceStateGenerator.UpdateQueue, + generator.Updates, p => p.WorkspaceProject is null); } @@ -587,12 +531,10 @@ await projectManager.UpdateAsync(updater => public async Task WorkspaceChanged_ProjectAddedEvent_AddsProject() { // Arrange - var workspaceStateGenerator = new TestProjectWorkspaceStateGenerator(); + var generator = new TestProjectWorkspaceStateGenerator(); var projectManager = CreateProjectSnapshotManager(); - var detector = new WorkspaceProjectStateChangeDetector(workspaceStateGenerator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, ErrorReporter, Dispatcher, _workQueue) - { - NotifyWorkspaceChangedEventComplete = new ManualResetEventSlim(initialState: false), - }; + var detector = new WorkspaceProjectStateChangeDetector(generator, projectManager, TestLanguageServerFeatureOptions.Instance, WorkspaceProvider, Dispatcher, LoggerFactory); + var detectorAccessor = detector.GetTestAccessor(); await projectManager.UpdateAsync(updater => { @@ -603,13 +545,14 @@ await projectManager.UpdateAsync(updater => var e = new WorkspaceChangeEventArgs(WorkspaceChangeKind.ProjectAdded, oldSolution: _emptySolution, newSolution: solution, projectId: _projectNumberThree.Id); // Act - detector.Workspace_WorkspaceChanged(Workspace, e); - detector.NotifyWorkspaceChangedEventComplete.Wait(); - _workQueueTestAccessor.NotifyBackgroundWorkCompleted.Wait(); + var listenerTask = detectorAccessor.ListenForWorkspaceChangesAsync(WorkspaceChangeKind.ProjectAdded); + detectorAccessor.WorkspaceChanged(e); + await listenerTask; + await detectorAccessor.WaitUntilCurrentBatchCompletesAsync(); // Assert Assert.Single( - workspaceStateGenerator.UpdateQueue, + generator.Updates, p => p.WorkspaceProject.Id == _projectNumberThree.Id); } diff --git a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/VsSolutionUpdatesProjectSnapshotChangeTriggerTest.cs b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/VsSolutionUpdatesProjectSnapshotChangeTriggerTest.cs index 0ba0c05f85d..7358cce23c1 100644 --- a/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/VsSolutionUpdatesProjectSnapshotChangeTriggerTest.cs +++ b/src/Razor/test/Microsoft.VisualStudio.LanguageServices.Razor.Test/VsSolutionUpdatesProjectSnapshotChangeTriggerTest.cs @@ -195,7 +195,8 @@ await projectManager.UpdateAsync(updater => updater.ProjectRemoved(s_someProject.Key); }); - var update = Assert.Single(workspaceStateGenerator.UpdateQueue); + var update = Assert.Single(workspaceStateGenerator.Updates); + Assert.NotNull(update.WorkspaceProject); Assert.Equal(update.WorkspaceProject.Id, _someWorkspaceProject.Id); Assert.Same(expectedProjectSnapshot, update.ProjectSnapshot); Assert.True(update.CancellationToken.IsCancellationRequested); @@ -240,7 +241,8 @@ public async Task OnProjectBuiltAsync_KnownProject_EnqueuesProjectStateUpdate() await testAccessor.OnProjectBuiltAsync(vsHierarchyMock.Object, DisposalToken); // Assert - var update = Assert.Single(workspaceStateGenerator.UpdateQueue); + var update = Assert.Single(workspaceStateGenerator.Updates); + Assert.NotNull(update.WorkspaceProject); Assert.Equal(update.WorkspaceProject.Id, _someWorkspaceProject.Id); Assert.Same(expectedProjectSnapshot, update.ProjectSnapshot); } @@ -285,7 +287,7 @@ await projectManager.UpdateAsync(updater => await testAccessor.OnProjectBuiltAsync(StrictMock.Of(), DisposalToken); // Assert - Assert.Empty(workspaceStateGenerator.UpdateQueue); + Assert.Empty(workspaceStateGenerator.Updates); } [UIFact] @@ -322,6 +324,6 @@ public async Task OnProjectBuiltAsync_UnknownProject_DoesNotEnqueueUpdate() await testAccessor.OnProjectBuiltAsync(StrictMock.Of(), DisposalToken); // Assert - Assert.Empty(workspaceStateGenerator.UpdateQueue); + Assert.Empty(workspaceStateGenerator.Updates); } } From f9f7d17dc91eca8daaf4a618a9f52ab87043ded8 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Thu, 21 Mar 2024 12:02:33 -0700 Subject: [PATCH 05/17] Add a few AsyncBatchingWorkQueue tests --- .../Utilities/AsyncBatchingWorkQueueTest.cs | 146 ++++++++++++++++++ 1 file changed, 146 insertions(+) create mode 100644 src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Utilities/AsyncBatchingWorkQueueTest.cs diff --git a/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Utilities/AsyncBatchingWorkQueueTest.cs b/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Utilities/AsyncBatchingWorkQueueTest.cs new file mode 100644 index 00000000000..4bb9c5d5f73 --- /dev/null +++ b/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Utilities/AsyncBatchingWorkQueueTest.cs @@ -0,0 +1,146 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Razor.Test.Common; +using Microsoft.CodeAnalysis.Razor.Utilities; +using Xunit; +using Xunit.Abstractions; + +namespace Microsoft.CodeAnalysis.Razor.Workspaces.Test.Utilities; + +public class AsyncBatchingWorkQueueTest(ITestOutputHelper output) : ToolingTestBase(output) +{ + [Fact] + public async Task AddItemsAndWaitForBatch() + { + var list = new List(); + + var workQueue = new AsyncBatchingWorkQueue( + delay: TimeSpan.FromMilliseconds(1), + processBatchAsync: (items, cancellationToken) => + { + foreach (var item in items) + { + list.Add(item); + } + + return default; + }, + DisposalToken); + + for (var i = 0; i < 1000; i++) + { + workQueue.AddWork(i); + } + + await workQueue.WaitUntilCurrentBatchCompletesAsync(); + + for (var i = 0; i < 1000; i++) + { + Assert.Equal(i, list[i]); + } + } + + [Fact] + public async Task DedupesItems() + { + var uniqueItems = new HashSet(); + + var workQueue = new AsyncBatchingWorkQueue( + delay: TimeSpan.FromMilliseconds(1), + processBatchAsync: (items, cancellationToken) => + { + // Verify that items doesn't contain any duplicates. + // We use a local set to verify the items that were + // passed to us, since there could be duplicates + // across batches. + var set = new HashSet(); + + foreach (var item in items) + { + Assert.True(set.Add(item)); + + // Add to the final set that we'll check at the very end. + uniqueItems.Add(item); + } + + return default; + }, + equalityComparer: EqualityComparer.Default, + DisposalToken); + + for (var i = 0; i < 1000; i++) + { + workQueue.AddWork(i); + workQueue.AddWork(i); + } + + await workQueue.WaitUntilCurrentBatchCompletesAsync(); + + for (var i = 0; i < 1000; i++) + { + Assert.Contains(i, uniqueItems); + } + } + + [Fact] + public async Task CancelExistingWork() + { + var cancelled = 0; + var batchStarted = 0; + var done = 0; + + var workQueue = new AsyncBatchingWorkQueue( + TimeSpan.FromMilliseconds(1), + cancellationToken => + { + // If the batch was already started once, bail. + if (batchStarted == 1) + { + return default; + } + + batchStarted++; + + while (true) // infinite loop + { + if (cancellationToken.IsCancellationRequested) + { + cancelled++; + break; + } + } + + done++; + + return default; + }, + DisposalToken); + + // Add first bit of work + workQueue.AddWork(); + + // Wait until the batch is started. + while (batchStarted == 0) + { + await Task.Delay(10); + } + + // Add another bit of work, cancelling the previous work. + workQueue.AddWork(cancelExistingWork: true); + + // Wait until the batch finishes. + while (done == 0) + { + await Task.Delay(10); + } + + // Assert that the batch was cancelled. + Assert.Equal(1, batchStarted); + Assert.Equal(1, cancelled); + Assert.Equal(1, done); + } +} From 536fc085beda4fee311ca9318c1c9795e882c3eb Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Thu, 21 Mar 2024 12:03:08 -0700 Subject: [PATCH 06/17] Delete old BatchingWorkQueue --- .../BatchableWorkItem.cs | 21 -- .../BatchingWorkQueue.cs | 284 ------------------ .../BatchingWorkQueueTest.cs | 256 ---------------- 3 files changed, 561 deletions(-) delete mode 100644 src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/BatchableWorkItem.cs delete mode 100644 src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/BatchingWorkQueue.cs delete mode 100644 src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/BatchingWorkQueueTest.cs diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/BatchableWorkItem.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/BatchableWorkItem.cs deleted file mode 100644 index 5128e97e91e..00000000000 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/BatchableWorkItem.cs +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the MIT license. See License.txt in the project root for license information. - -using System.Threading; -using System.Threading.Tasks; - -namespace Microsoft.CodeAnalysis.Razor.Workspaces; - -/// -/// A work item that represents a unit of work. This is intended to be overridden so consumers can represent any -/// unit of work that fits their need. -/// -internal abstract class BatchableWorkItem -{ - /// - /// Processes a unit of work. - /// - /// A cancellation token for the unit of work - /// A task - public abstract ValueTask ProcessAsync(CancellationToken cancellationToken); -} diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/BatchingWorkQueue.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/BatchingWorkQueue.cs deleted file mode 100644 index d392c0fae93..00000000000 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/BatchingWorkQueue.cs +++ /dev/null @@ -1,284 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the MIT license. See License.txt in the project root for license information. - -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.Linq; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.Extensions.Internal; - -namespace Microsoft.CodeAnalysis.Razor.Workspaces; - -internal sealed class BatchingWorkQueue : IDisposable -{ - // Interactions with this collection should always take a lock on the collection and - // be careful about interactions it may have with the on-going timer. The reasons - // stem from the transactional manner that we use to modify the collection. For instance - // we'll capture workloads and then after processing a lot of work items we'll leave open - // the opportunity to re-start our processing loop to ensure things get processed at an - // efficient pace. - private readonly Dictionary _work; - private readonly TimeSpan _batchingTimeSpan; - private readonly IErrorReporter _errorReporter; - private readonly CancellationTokenSource _disposalCts; - private Timer? _timer; - private bool _disposed; - - public BatchingWorkQueue( - TimeSpan batchingTimeSpan, - StringComparer keyComparer, - IErrorReporter errorReporter) - { - if (keyComparer is null) - { - throw new ArgumentNullException(nameof(keyComparer)); - } - - if (errorReporter is null) - { - throw new ArgumentNullException(nameof(errorReporter)); - } - - _batchingTimeSpan = batchingTimeSpan; - _errorReporter = errorReporter; - _disposalCts = new CancellationTokenSource(); - _work = new Dictionary(keyComparer); - } - - private bool IsScheduledOrRunning => _timer != null; - - // Used in unit tests to ensure we can control when background work starts. - private ManualResetEventSlim? BlockBackgroundWorkStart { get; set; } - - // Used in unit tests to ensure we can know when background work finishes. - private ManualResetEventSlim? NotifyBackgroundWorkStarting { get; set; } - - // Used in unit tests to ensure we can know when background has captured its current workload. - private ManualResetEventSlim? NotifyBackgroundCapturedWorkload { get; set; } - - // Used in unit tests to ensure we can control when background work completes. - private ManualResetEventSlim? BlockBackgroundWorkCompleting { get; set; } - - // Used in unit tests to ensure we can know when background work finishes. - private ManualResetEventSlim? NotifyBackgroundWorkCompleted { get; set; } - - // Used in unit tests to ensure we can know when errors are reported - private ManualResetEventSlim? NotifyErrorBeingReported { get; set; } - - // Used in unit tests to ensure we can know when background workloads are completing - private ManualResetEventSlim? NotifyBackgroundWorkCompleting { get; set; } - - /// - /// Adds the provided to a work queue under the specified . - /// Multiple enqueues under the same will use the last enqueued . - /// - /// An identifier used to track 's. - /// An item to process - public void Enqueue(string key, BatchableWorkItem workItem) - { - lock (_work) - { - if (_disposed) - { - throw new ObjectDisposedException(nameof(BatchingWorkQueue)); - } - - // We only want to store the last 'seen' work item. That way when we pick one to process it's - // always the latest version to use. - _work[key] = workItem; - - StartWorker(); - } - } - - public void Dispose() - { - lock (_work) - { - if (_disposed) - { - return; - } - - _disposed = true; - _timer?.Dispose(); - _timer = null; - _work.Clear(); - _disposalCts.Cancel(); - _disposalCts.Dispose(); - } - } - - private void StartWorker() - { - // Access to the timer is protected by the lock in Enqueue and in Timer_TickAsync - // Timer will fire after a fixed delay, but only once. - _timer ??= NonCapturingTimer.Create( - state => - { - Assumes.NotNull(state); - _ = ((BatchingWorkQueue)state).Timer_TickAsync(); - }, - this, - _batchingTimeSpan, - Timeout.InfiniteTimeSpan); - } - - private async Task Timer_TickAsync() - { - try - { - _timer?.Change(Timeout.Infinite, Timeout.Infinite); - - OnStartingBackgroundWork(); - - KeyValuePair[] work; - lock (_work) - { - work = _work.ToArray(); - _work.Clear(); - } - - OnBackgroundCapturedWorkload(); - - for (var i = 0; i < work.Length; i++) - { - var workItem = work[i].Value; - try - { - await workItem.ProcessAsync(_disposalCts.Token).ConfigureAwait(false); - } - catch (OperationCanceledException) when (_disposalCts.IsCancellationRequested) - { - // Expected shutdown case, lets not log an error. - } - catch (Exception ex) - { - // Work item failed to process, allow the other process events to continue. - _errorReporter.ReportError(ex); - } - } - - OnCompletingBackgroundWork(); - - lock (_work) - { - // Suppress analyzer that suggests using DisposeAsync(). -#pragma warning disable VSTHRD103 // Call async methods when in an async method - - // Resetting the timer allows another batch of work to start. - _timer?.Dispose(); - _timer = null; - -#pragma warning restore VSTHRD103 - - // If more work came in while we were running start the worker again if we're still alive - if (_work.Count > 0 && !_disposed) - { - StartWorker(); - } - } - - OnCompletedBackgroundWork(); - } - catch (Exception ex) - { - // This is something totally unexpected - Debug.Fail("Batching work queue failed unexpectedly"); - _errorReporter.ReportError(ex); - } - } - - private void OnStartingBackgroundWork() - { - NotifyBackgroundWorkStarting?.Set(); - - if (BlockBackgroundWorkStart != null) - { - BlockBackgroundWorkStart.Wait(); - BlockBackgroundWorkStart.Reset(); - } - } - - private void OnCompletingBackgroundWork() - { - NotifyBackgroundWorkCompleting?.Set(); - - if (BlockBackgroundWorkCompleting != null) - { - BlockBackgroundWorkCompleting.Wait(); - BlockBackgroundWorkCompleting.Reset(); - } - } - - private void OnCompletedBackgroundWork() - { - NotifyBackgroundWorkCompleted?.Set(); - } - - private void OnBackgroundCapturedWorkload() - { - NotifyBackgroundCapturedWorkload?.Set(); - } - - internal TestAccessor GetTestAccessor() - => new(this); - - internal class TestAccessor - { - private readonly BatchingWorkQueue _queue; - - public TestAccessor(BatchingWorkQueue queue) - { - _queue = queue; - } - - public bool IsScheduledOrRunning => _queue.IsScheduledOrRunning; - - public Dictionary Work => _queue._work; - - public ManualResetEventSlim? BlockBackgroundWorkStart - { - get => _queue.BlockBackgroundWorkStart; - set => _queue.BlockBackgroundWorkStart = value; - } - - public ManualResetEventSlim? NotifyBackgroundWorkStarting - { - get => _queue.NotifyBackgroundWorkStarting; - set => _queue.NotifyBackgroundWorkStarting = value; - } - - public ManualResetEventSlim? NotifyBackgroundCapturedWorkload - { - get => _queue.NotifyBackgroundCapturedWorkload; - set => _queue.NotifyBackgroundCapturedWorkload = value; - } - - public ManualResetEventSlim? BlockBackgroundWorkCompleting - { - get => _queue.BlockBackgroundWorkCompleting; - set => _queue.BlockBackgroundWorkCompleting = value; - } - - public ManualResetEventSlim? NotifyBackgroundWorkCompleted - { - get => _queue.NotifyBackgroundWorkCompleted; - set => _queue.NotifyBackgroundWorkCompleted = value; - } - - public ManualResetEventSlim? NotifyErrorBeingReported - { - get => _queue.NotifyErrorBeingReported; - set => _queue.NotifyErrorBeingReported = value; - } - - public ManualResetEventSlim? NotifyBackgroundWorkCompleting - { - get => _queue.NotifyBackgroundWorkCompleting; - set => _queue.NotifyBackgroundWorkCompleting = value; - } - } -} diff --git a/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/BatchingWorkQueueTest.cs b/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/BatchingWorkQueueTest.cs deleted file mode 100644 index ebeae645a65..00000000000 --- a/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/BatchingWorkQueueTest.cs +++ /dev/null @@ -1,256 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the MIT license. See License.txt in the project root for license information. - -#nullable disable - -using System; -using System.Collections.Generic; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Razor.Test.Common; -using Microsoft.CodeAnalysis.Razor.ProjectSystem; -using Xunit; -using Xunit.Abstractions; - -namespace Microsoft.CodeAnalysis.Razor.Workspaces; - -public class BatchingWorkQueueTest : ToolingTestBase -{ - private readonly BatchingWorkQueue _workQueue; - private readonly BatchingWorkQueue.TestAccessor _testAccessor; - private readonly TestErrorReporter _errorReporter; - - public BatchingWorkQueueTest(ITestOutputHelper testOutput) - : base(testOutput) - { - _errorReporter = new TestErrorReporter(); - _workQueue = new BatchingWorkQueue(TimeSpan.FromMilliseconds(1), StringComparer.Ordinal, _errorReporter); - _testAccessor = _workQueue.GetTestAccessor(); - _testAccessor.NotifyBackgroundWorkCompleted = new ManualResetEventSlim(initialState: false); - - AddDisposable(_workQueue); - } - - [Fact] - public void Enqueue_ProcessesNotifications_AndGoesBackToSleep() - { - // Arrange - var workItem = new TestBatchableWorkItem(); - _testAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); - _testAccessor.BlockBackgroundWorkCompleting = new ManualResetEventSlim(initialState: false); - - // Act - _workQueue.Enqueue("key", workItem); - - // Assert - Assert.True(_testAccessor.IsScheduledOrRunning, "Queue should be scheduled during Enqueue"); - Assert.NotEmpty(_testAccessor.Work); - - // Allow the background work to proceed. - _testAccessor.BlockBackgroundWorkStart.Set(); - _testAccessor.BlockBackgroundWorkCompleting.Set(); - - _testAccessor.NotifyBackgroundWorkCompleted.Wait(TimeSpan.FromSeconds(3)); - - Assert.False(_testAccessor.IsScheduledOrRunning, "Queue should not have restarted"); - Assert.Empty(_testAccessor.Work); - Assert.True(workItem.Processed); - Assert.Empty(_errorReporter.ReportedExceptions); - } - - [Fact] - public void Enqueue_BatchesNotificationsByKey_ProcessesLast() - { - // Arrange - var originalWorkItem = new ThrowingBatchableWorkItem(); - var newestWorkItem = new TestBatchableWorkItem(); - _testAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); - - // Act - _workQueue.Enqueue("key", originalWorkItem); - _workQueue.Enqueue("key", newestWorkItem); - - // Assert - Assert.True(_testAccessor.IsScheduledOrRunning, "Queue should be scheduled during Enqueue"); - Assert.NotEmpty(_testAccessor.Work); - - // Allow the background work to start. - _testAccessor.BlockBackgroundWorkStart.Set(); - _testAccessor.NotifyBackgroundWorkCompleted.Wait(TimeSpan.FromSeconds(3)); - - Assert.Empty(_testAccessor.Work); - - Assert.False(originalWorkItem.Processed); - Assert.True(newestWorkItem.Processed); - Assert.Empty(_errorReporter.ReportedExceptions); - } - - [Fact] - public void Enqueue_ProcessesNotifications_AndRestarts() - { - // Arrange - var initialWorkItem = new TestBatchableWorkItem(); - var workItemToCauseRestart = new TestBatchableWorkItem(); - _testAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); - _testAccessor.NotifyBackgroundWorkStarting = new ManualResetEventSlim(initialState: false); - _testAccessor.NotifyBackgroundCapturedWorkload = new ManualResetEventSlim(initialState: false); - _testAccessor.BlockBackgroundWorkCompleting = new ManualResetEventSlim(initialState: false); - _testAccessor.NotifyBackgroundWorkCompleted = new ManualResetEventSlim(initialState: false); - - // Act & Assert - _workQueue.Enqueue("key", initialWorkItem); - - Assert.True(_testAccessor.IsScheduledOrRunning, "Queue should be scheduled during Enqueue"); - Assert.NotEmpty(_testAccessor.Work); - - // Allow the background work to start. - _testAccessor.BlockBackgroundWorkStart.Set(); - - _testAccessor.NotifyBackgroundWorkStarting.Wait(TimeSpan.FromSeconds(3)); - - Assert.True(_testAccessor.IsScheduledOrRunning, "Worker should be processing now"); - - _testAccessor.NotifyBackgroundCapturedWorkload.Wait(TimeSpan.FromSeconds(3)); - Assert.Empty(_testAccessor.Work); - - _workQueue.Enqueue("key", workItemToCauseRestart); - Assert.NotEmpty(_testAccessor.Work); // Now we should see the worker restart when it finishes. - - // Allow work to complete, which should restart the timer. - _testAccessor.BlockBackgroundWorkCompleting.Set(); - - _testAccessor.NotifyBackgroundWorkCompleted.Wait(TimeSpan.FromSeconds(3)); - _testAccessor.NotifyBackgroundWorkCompleted.Reset(); - - // It should start running again right away. - Assert.True(_testAccessor.IsScheduledOrRunning, "Queue should be scheduled during Enqueue"); - Assert.NotEmpty(_testAccessor.Work); - - // Allow the background work to proceed. - _testAccessor.BlockBackgroundWorkStart.Set(); - - _testAccessor.BlockBackgroundWorkCompleting.Set(); - _testAccessor.NotifyBackgroundWorkCompleted.Wait(TimeSpan.FromSeconds(3)); - - Assert.False(_testAccessor.IsScheduledOrRunning, "Queue should not have restarted"); - Assert.Empty(_testAccessor.Work); - Assert.True(initialWorkItem.Processed); - Assert.True(workItemToCauseRestart.Processed); - Assert.Empty(_errorReporter.ReportedExceptions); - } - - [Fact] - public void Enqueue_ThrowingWorkItem_DoesNotPreventProcessingSubsequentItems() - { - // Arrange - var throwingWorkItem = new ThrowingBatchableWorkItem(); - var validWorkItem = new TestBatchableWorkItem(); - _testAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); - - // Act - _workQueue.Enqueue("key", throwingWorkItem); - _workQueue.Enqueue("key2", validWorkItem); - - // Assert - Assert.True(_testAccessor.IsScheduledOrRunning, "Queue should be scheduled during Enqueue"); - Assert.NotEmpty(_testAccessor.Work); - - // Allow the background work to start. - _testAccessor.BlockBackgroundWorkStart.Set(); - _testAccessor.NotifyBackgroundWorkCompleted.Wait(TimeSpan.FromSeconds(3)); - - Assert.Empty(_testAccessor.Work); - - Assert.True(throwingWorkItem.Processed); - Assert.True(validWorkItem.Processed); - Assert.Single(_errorReporter.ReportedExceptions); - } - - [Fact] - public void Enqueue_DisposedPreventsRestart() - { - // Arrange - var initialWorkItem = new TestBatchableWorkItem(); - var workItemToCauseRestart = new TestBatchableWorkItem(); - _testAccessor.BlockBackgroundWorkStart = new ManualResetEventSlim(initialState: false); - _testAccessor.NotifyBackgroundWorkStarting = new ManualResetEventSlim(initialState: false); - _testAccessor.NotifyBackgroundCapturedWorkload = new ManualResetEventSlim(initialState: false); - _testAccessor.NotifyBackgroundWorkCompleting = new ManualResetEventSlim(initialState: false); - _testAccessor.BlockBackgroundWorkCompleting = new ManualResetEventSlim(initialState: false); - _testAccessor.NotifyBackgroundWorkCompleted = new ManualResetEventSlim(initialState: false); - - // Act & Assert - _workQueue.Enqueue("key", initialWorkItem); - - Assert.True(_testAccessor.IsScheduledOrRunning, "Queue should be scheduled during Enqueue"); - Assert.NotEmpty(_testAccessor.Work); - - // Allow the background work to start. - _testAccessor.BlockBackgroundWorkStart.Set(); - - _testAccessor.NotifyBackgroundWorkStarting.Wait(TimeSpan.FromSeconds(3)); - - Assert.True(_testAccessor.IsScheduledOrRunning, "Worker should be processing now"); - - _testAccessor.NotifyBackgroundCapturedWorkload.Wait(TimeSpan.FromSeconds(3)); - Assert.Empty(_testAccessor.Work); - - // Wait for the background workload to complete - _testAccessor.NotifyBackgroundWorkCompleting.Wait(TimeSpan.FromSeconds(5)); - - _workQueue.Enqueue("key", workItemToCauseRestart); - Assert.NotEmpty(_testAccessor.Work); - - // Disposing before the queue has a chance to restart; - _workQueue.Dispose(); - - // Allow work to complete, which should restart the timer. - _testAccessor.BlockBackgroundWorkCompleting.Set(); - - _testAccessor.NotifyBackgroundWorkCompleted.Wait(TimeSpan.FromSeconds(3)); - _testAccessor.NotifyBackgroundWorkCompleted.Reset(); - - // It should start running again right away. - Assert.False(_testAccessor.IsScheduledOrRunning, "Queue should be scheduled during Enqueue"); - - // Dispose clears the work queue - Assert.Empty(_testAccessor.Work); - - Assert.True(initialWorkItem.Processed); - Assert.False(workItemToCauseRestart.Processed); - Assert.Empty(_errorReporter.ReportedExceptions); - } - - private class TestBatchableWorkItem : BatchableWorkItem - { - public bool Processed { get; private set; } - - public override ValueTask ProcessAsync(CancellationToken cancellationToken) - { - Processed = true; - return new ValueTask(); - } - } - - private class ThrowingBatchableWorkItem : TestBatchableWorkItem - { - public override ValueTask ProcessAsync(CancellationToken cancellationToken) - { - _ = base.ProcessAsync(cancellationToken); - throw new InvalidOperationException(); - } - } - - private class TestErrorReporter : IErrorReporter - { - private readonly List _reportedExceptions = new(); - - public IReadOnlyList ReportedExceptions => _reportedExceptions; - - public void ReportError(Exception exception) => _reportedExceptions.Add(exception); - - public void ReportError(Exception exception, IProjectSnapshot project) => _reportedExceptions.Add(exception); - - public void ReportError(Exception exception, Project workspaceProject) => _reportedExceptions.Add(exception); - } -} From e710932936dafd1bf33f31a1bcf380010f6ff8d6 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Thu, 21 Mar 2024 17:37:09 -0700 Subject: [PATCH 07/17] Small tweaks to CancellationSeries Remove invalid link from comment and make code style consistent. --- .../Threading/CancellationSeries.cs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Threading/CancellationSeries.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Threading/CancellationSeries.cs index 5de367394ee..3003774fdf0 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Threading/CancellationSeries.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.ProjectEngineHost/Threading/CancellationSeries.cs @@ -11,9 +11,6 @@ namespace Microsoft.AspNetCore.Razor.Threading; // // However, it was originally derived from an implementation in dotnet/project-system: // https://github.com/dotnet/project-system/blob/bdf69d5420ec8d894f5bf4c3d4692900b7f2479c/src/Microsoft.VisualStudio.ProjectSystem.Managed/Threading/Tasks/CancellationSeries.cs -// -// See the commentary in https://github.com/dotnet/roslyn/pull/50156 for notes on incorporating changes made to the -// reference implementation. /// /// Produces a series of objects such that requesting a new token @@ -92,7 +89,7 @@ public CancellationToken CreateNext(CancellationToken token = default) } } - if (priorSource == null) + if (priorSource is null) { nextSource.Dispose(); @@ -117,7 +114,7 @@ public void Dispose() { var source = Interlocked.Exchange(ref _cts, null); - if (source == null) + if (source is null) { // Already disposed return; From 286bfc740d394c86edf6a80eaee895b580f44336 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Thu, 21 Mar 2024 17:42:34 -0700 Subject: [PATCH 08/17] Cache SemanticTokens.RefreshSupport value and check it sooner --- .../WorkspaceSemanticTokensRefreshNotifier.cs | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshNotifier.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshNotifier.cs index 89c1ceae40f..8b13e6f27ee 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshNotifier.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshNotifier.cs @@ -16,10 +16,11 @@ internal class WorkspaceSemanticTokensRefreshNotifier : IWorkspaceSemanticTokens private readonly IClientCapabilitiesService _clientCapabilitiesService; private readonly IClientConnection _clientConnection; - private readonly CancellationTokenSource _disposeCancellationSource; + private readonly CancellationTokenSource _disposeTokenSource; private readonly IDisposable _optionsChangeListener; private readonly object _gate = new(); + private bool? _supportsRefresh; private bool _waitingToRefresh; private Task _refreshTask = Task.CompletedTask; @@ -33,7 +34,7 @@ public WorkspaceSemanticTokensRefreshNotifier( _clientCapabilitiesService = clientCapabilitiesService; _clientConnection = clientConnection; - _disposeCancellationSource = new(); + _disposeTokenSource = new(); _isColoringBackground = optionsMonitor.CurrentValue.ColorBackground; _optionsChangeListener = optionsMonitor.OnChange(HandleOptionsChange); @@ -43,8 +44,8 @@ public void Dispose() { _optionsChangeListener.Dispose(); - _disposeCancellationSource.Cancel(); - _disposeCancellationSource.Dispose(); + _disposeTokenSource.Cancel(); + _disposeTokenSource.Dispose(); } private void HandleOptionsChange(RazorLSPOptions options, string _) @@ -58,7 +59,7 @@ private void HandleOptionsChange(RazorLSPOptions options, string _) public void NotifyWorkspaceSemanticTokensRefresh() { - if (_disposeCancellationSource.IsCancellationRequested) + if (_disposeTokenSource.IsCancellationRequested) { return; } @@ -71,14 +72,15 @@ public void NotifyWorkspaceSemanticTokensRefresh() return; } - var capabilities = _clientCapabilitiesService.ClientCapabilities; - var useWorkspaceRefresh = capabilities?.Workspace?.SemanticTokens?.RefreshSupport ?? false; + _supportsRefresh ??= _clientCapabilitiesService.ClientCapabilities.Workspace?.SemanticTokens?.RefreshSupport ?? false; - if (useWorkspaceRefresh) + if (_supportsRefresh is false) { - _refreshTask = RefreshAfterDelayAsync(); - _waitingToRefresh = true; + return; } + + _refreshTask = RefreshAfterDelayAsync(); + _waitingToRefresh = true; } async Task RefreshAfterDelayAsync() @@ -86,7 +88,7 @@ async Task RefreshAfterDelayAsync() await Task.Delay(s_delay).ConfigureAwait(false); _clientConnection - .SendNotificationAsync(Methods.WorkspaceSemanticTokensRefreshName, _disposeCancellationSource.Token) + .SendNotificationAsync(Methods.WorkspaceSemanticTokensRefreshName, _disposeTokenSource.Token) .Forget(); _waitingToRefresh = false; From 2a3c3c24fc09fc4e452c80df7756e13d1af3ab44 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Thu, 21 Mar 2024 17:42:59 -0700 Subject: [PATCH 09/17] Pass cancellation token to Task.Delay --- .../WorkspaceSemanticTokensRefreshNotifier.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshNotifier.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshNotifier.cs index 8b13e6f27ee..db9f027caca 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshNotifier.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/WorkspaceSemanticTokensRefreshNotifier.cs @@ -85,7 +85,7 @@ public void NotifyWorkspaceSemanticTokensRefresh() async Task RefreshAfterDelayAsync() { - await Task.Delay(s_delay).ConfigureAwait(false); + await Task.Delay(s_delay, _disposeTokenSource.Token).ConfigureAwait(false); _clientConnection .SendNotificationAsync(Methods.WorkspaceSemanticTokensRefreshName, _disposeTokenSource.Token) From b5de9940963d786bfa0f3501a8b3ee0ccbbd581e Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Thu, 21 Mar 2024 18:26:22 -0700 Subject: [PATCH 10/17] Provide AsyncBatchingWorkQueue mechansim to control deduping --- .../OpenDocumentGenerator.Comparer.cs | 39 ++++++++++++++ .../OpenDocumentGenerator.cs | 34 +++--------- .../Utilities/AsyncBatchingWorkQueue.cs | 2 +- .../Utilities/AsyncBatchingWorkQueue`1.cs | 4 +- .../Utilities/AsyncBatchingWorkQueue`2.cs | 53 ++++++++++++++++++- ...paceProjectStateChangeDetector.Comparer.cs | 36 +++++++++++++ .../WorkspaceProjectStateChangeDetector.cs | 37 ++++--------- .../Utilities/AsyncBatchingWorkQueueTest.cs | 1 + 8 files changed, 150 insertions(+), 56 deletions(-) create mode 100644 src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.Comparer.cs create mode 100644 src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.Comparer.cs diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.Comparer.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.Comparer.cs new file mode 100644 index 00000000000..483f45fbb39 --- /dev/null +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.Comparer.cs @@ -0,0 +1,39 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System.Collections.Generic; +using Microsoft.CodeAnalysis.Razor; +using Microsoft.CodeAnalysis.Razor.ProjectSystem; + +namespace Microsoft.AspNetCore.Razor.LanguageServer; + +internal partial class OpenDocumentGenerator +{ + private sealed class Comparer : IEqualityComparer + { + public static readonly Comparer Instance = new(); + + private Comparer() + { + } + + public bool Equals(IDocumentSnapshot? x, IDocumentSnapshot? y) + { + if (x is null) + { + return y is null; + } + else if (y is null) + { + return false; + } + + return FilePathComparer.Instance.Equals(x.FilePath, y.FilePath); + } + + public int GetHashCode(IDocumentSnapshot obj) + { + return FilePathComparer.Instance.GetHashCode(obj); + } + } +} diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs index dcb1350d2b2..f72e75115ec 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs @@ -4,10 +4,8 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; -using System.Diagnostics; using System.Threading; using System.Threading.Tasks; -using Microsoft.AspNetCore.Razor.PooledObjects; using Microsoft.CodeAnalysis.Razor; using Microsoft.CodeAnalysis.Razor.ProjectSystem; using Microsoft.CodeAnalysis.Razor.Utilities; @@ -15,7 +13,7 @@ namespace Microsoft.AspNetCore.Razor.LanguageServer; -internal class OpenDocumentGenerator : IRazorStartupService, IDisposable +internal partial class OpenDocumentGenerator : IRazorStartupService, IDisposable { // Using 10 milliseconds for the delay here because we want document synchronization to be very fast, // so that features like completion are not delayed, but at the same time we don't want to do more work @@ -35,7 +33,6 @@ internal class OpenDocumentGenerator : IRazorStartupService, IDisposable private readonly LanguageServerFeatureOptions _options; private readonly AsyncBatchingWorkQueue _workQueue; - private readonly HashSet _processedFilePaths; private readonly CancellationTokenSource _disposeTokenSource; public OpenDocumentGenerator( @@ -49,9 +46,13 @@ public OpenDocumentGenerator( _dispatcher = dispatcher; _options = options; - _processedFilePaths = new(FilePathComparer.Instance); _disposeTokenSource = new(); - _workQueue = new AsyncBatchingWorkQueue(s_delay, ProcessBatchAsync, _disposeTokenSource.Token); + _workQueue = new AsyncBatchingWorkQueue( + s_delay, + ProcessBatchAsync, + Comparer.Instance, + preferMostRecentItems: true, + _disposeTokenSource.Token); _projectManager.Changed += ProjectManager_Changed; @@ -69,30 +70,13 @@ public void Dispose() private async ValueTask ProcessBatchAsync(ImmutableArray items, CancellationToken token) { - Debug.Assert(_processedFilePaths.Count == 0); - - using var itemsToProcess = new PooledArrayBuilder(capacity: items.Length); - - // Walk items in reverse to ensure that we only process the most recent document snapshots. - for (var i = items.Length - 1; i >= 0; i--) - { - var document = items[i]; - var filePath = document.FilePath.AssumeNotNull(); - - if (_processedFilePaths.Add(filePath)) - { - itemsToProcess.Push(document); - } - } - - while (itemsToProcess.Count > 0) + foreach (var document in items) { if (token.IsCancellationRequested) { return; } - var document = itemsToProcess.Pop(); var codeDocument = await document.GetGeneratedOutputAsync().ConfigureAwait(false); await _dispatcher @@ -115,8 +99,6 @@ await _dispatcher token) .ConfigureAwait(false); } - - _processedFilePaths.Clear(); } private void ProjectManager_Changed(object? sender, ProjectChangeEventArgs args) diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue.cs index 4ff741e17cf..4637cf31e68 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue.cs @@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.Razor.Utilities; internal class AsyncBatchingWorkQueue( TimeSpan delay, Func processBatchAsync, - CancellationToken cancellationToken) : AsyncBatchingWorkQueue(delay, Convert(processBatchAsync), EqualityComparer.Default, cancellationToken) + CancellationToken cancellationToken) : AsyncBatchingWorkQueue(delay, Convert(processBatchAsync), EqualityComparer.Default, false, cancellationToken) { private static Func, CancellationToken, ValueTask> Convert(Func processBatchAsync) => (items, ct) => processBatchAsync(ct); diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`1.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`1.cs index 7196793896f..dcdfcdfb211 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`1.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`1.cs @@ -18,7 +18,8 @@ internal class AsyncBatchingWorkQueue( TimeSpan delay, Func, CancellationToken, ValueTask> processBatchAsync, IEqualityComparer? equalityComparer, - CancellationToken cancellationToken) : AsyncBatchingWorkQueue(delay, Convert(processBatchAsync), equalityComparer, cancellationToken) + bool preferMostRecentItems, + CancellationToken cancellationToken) : AsyncBatchingWorkQueue(delay, Convert(processBatchAsync), equalityComparer, preferMostRecentItems, cancellationToken) { public AsyncBatchingWorkQueue( TimeSpan delay, @@ -27,6 +28,7 @@ public AsyncBatchingWorkQueue( : this(delay, processBatchAsync, equalityComparer: null, + preferMostRecentItems: false, cancellationToken) { } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`2.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`2.cs index 4fc0dd9100e..872141e3c74 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`2.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`2.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; +using System.Diagnostics; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Razor; @@ -15,6 +16,9 @@ namespace Microsoft.CodeAnalysis.Razor.Utilities; // NOTE: This code is derived from dotnet/roslyn: // https://github.com/dotnet/roslyn/blob/98cd097bf122677378692ebe952b71ab6e5bb013/src/Workspaces/Core/Portable/Shared/Utilities/AsyncBatchingWorkQueue%602.cs +// +// The key change to the Roslyn implementation is the addition of 'preferMostRecentItems' which controls which +// version of an item is preferred when deduping is employed. /// /// A queue where items can be added to to be processed in batches after some delay has passed. When processing @@ -39,6 +43,11 @@ internal class AsyncBatchingWorkQueue /// private readonly IEqualityComparer? _equalityComparer; + /// + /// Determines whether the most recent items are preferred when deduping. + /// + private readonly bool _preferMostRecentItems; + /// /// Callback to actually perform the processing of the next batch of work. /// @@ -104,11 +113,13 @@ public AsyncBatchingWorkQueue( TimeSpan delay, Func, CancellationToken, ValueTask> processBatchAsync, IEqualityComparer? equalityComparer, + bool preferMostRecentItems, CancellationToken cancellationToken) { _delay = delay; _processBatchAsync = processBatchAsync; _equalityComparer = equalityComparer; + _preferMostRecentItems = preferMostRecentItems; _entireQueueCancellationToken = cancellationToken; _uniqueItems = new HashSet(equalityComparer); @@ -183,7 +194,15 @@ void AddItemsToBatch(IEnumerable items) return; } - // We're deduping items. Only add the item if it's the first time we've seen it. + // In the case that we want to dedupe and prefer the most recent + // items, we add everything to the batch now and will remove them later. + if (_preferMostRecentItems) + { + _nextBatch.AddRange(items); + return; + } + + // Otherwise, we dedupe and prefer the first item that we see. foreach (var item in items) { if (_uniqueItems.Add(item)) @@ -278,6 +297,38 @@ void AddItemsToBatch(IEnumerable items) { lock (_gate) { + // If we're deduping and preferring the most recent items, we need + // to compare items and adjust the next batch here. + if (_nextBatch.Count > 0 && _equalityComparer is not null && _preferMostRecentItems) + { + Debug.Assert(_uniqueItems.Count == 0, "Deduping should not have already occurred."); + + using var stack = new PooledArrayBuilder(capacity: _nextBatch.Count); + + // Walk the next batch in reverse and to identify unique items. + // We push them on a stack so that we can pop them in order later + for (var i = _nextBatch.Count - 1; i >= 0; i--) + { + var item = _nextBatch[i]; + + if (_uniqueItems.Add(item)) + { + stack.Push(item); + } + } + + // Did we actually dedupe anything? If so, adjust the next batch. + if (stack.Count < _nextBatch.Count) + { + _nextBatch.Clear(); + + while (stack.Count > 0) + { + _nextBatch.Add(stack.Pop()); + } + } + } + var nextBatch = _nextBatch.ToImmutable(); // mark there being no existing update task so that the next OOP notification will diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.Comparer.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.Comparer.cs new file mode 100644 index 00000000000..ba093cb7eff --- /dev/null +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.Comparer.cs @@ -0,0 +1,36 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System.Collections.Generic; +using Microsoft.CodeAnalysis; +using Microsoft.CodeAnalysis.Razor; +using Microsoft.CodeAnalysis.Razor.ProjectSystem; + +namespace Microsoft.VisualStudio.LanguageServices.Razor; + +internal partial class WorkspaceProjectStateChangeDetector +{ + private sealed class Comparer : IEqualityComparer<(Project?, IProjectSnapshot)> + { + public static readonly Comparer Instance = new(); + + private Comparer() + { + } + + public bool Equals((Project?, IProjectSnapshot) x, (Project?, IProjectSnapshot) y) + { + var (_, snapshotX) = x; + var (_, snapshotY) = y; + + return FilePathComparer.Instance.Equals(snapshotX.Key.Id, snapshotY.Key.Id); + } + + public int GetHashCode((Project?, IProjectSnapshot) obj) + { + var (_, snapshot) = obj; + + return FilePathComparer.Instance.GetHashCode(snapshot.Key.Id); + } + } +} diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.cs index 085ae6b5b1a..41bb11d6ff6 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.cs @@ -2,17 +2,14 @@ // Licensed under the MIT license. See License.txt in the project root for license information. using System; -using System.Collections.Generic; using System.Collections.Immutable; using System.ComponentModel.Composition; -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Razor; using Microsoft.AspNetCore.Razor.Language.Components; -using Microsoft.AspNetCore.Razor.PooledObjects; using Microsoft.CodeAnalysis; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Razor; @@ -25,7 +22,7 @@ namespace Microsoft.VisualStudio.LanguageServices.Razor; [Export(typeof(IRazorStartupService))] -internal class WorkspaceProjectStateChangeDetector : IRazorStartupService, IDisposable +internal partial class WorkspaceProjectStateChangeDetector : IRazorStartupService, IDisposable { private static readonly TimeSpan s_delay = TimeSpan.FromSeconds(1); @@ -38,7 +35,6 @@ internal class WorkspaceProjectStateChangeDetector : IRazorStartupService, IDisp private readonly CancellationTokenSource _disposeTokenSource; private readonly AsyncBatchingWorkQueue<(Project?, IProjectSnapshot)> _workQueue; - private readonly HashSet _processedIds; private WorkspaceChangedListener? _workspaceChangedListener; @@ -58,8 +54,12 @@ public WorkspaceProjectStateChangeDetector( _logger = loggerFactory.CreateLogger(); _disposeTokenSource = new(); - _processedIds = new(FilePathComparer.Instance); - _workQueue = new AsyncBatchingWorkQueue<(Project?, IProjectSnapshot)>(s_delay, ProcessBatchAsync, _disposeTokenSource.Token); + _workQueue = new AsyncBatchingWorkQueue<(Project?, IProjectSnapshot)>( + s_delay, + ProcessBatchAsync, + Comparer.Instance, + preferMostRecentItems: true, + _disposeTokenSource.Token); _projectManager.Changed += ProjectManager_Changed; @@ -82,27 +82,12 @@ public void Dispose() private async ValueTask ProcessBatchAsync(ImmutableArray<(Project? Project, IProjectSnapshot ProjectSnapshot)> items, CancellationToken token) { - Debug.Assert(_processedIds.Count == 0); - - using var itemsToProcess = new PooledArrayBuilder<(Project?, IProjectSnapshot)>(capacity: items.Length); - - // We loop through the items in reverse order to de-dupe them and only handle the - // most recent items. So, we push them onto a stack so that we process items - // in the original order. - for (var i = items.Length - 1; i >= 0; i--) + foreach (var (project, projectSnapshot) in items) { - var (project, projectSnapshot) = items[i]; - var key = projectSnapshot.Key.Id; - - if (_processedIds.Add(key)) + if (token.IsCancellationRequested) { - itemsToProcess.Push((project, projectSnapshot)); + return; } - } - - while (itemsToProcess.Count > 0) - { - var (project, projectSnapshot) = itemsToProcess.Pop(); _logger.LogTrace("Process update: {DisplayName}", projectSnapshot.DisplayName); @@ -115,8 +100,6 @@ await _dispatcher.RunAsync( state: (_generator, project, projectSnapshot, token), token); } - - _processedIds.Clear(); } private void Workspace_WorkspaceChanged(object? sender, WorkspaceChangeEventArgs e) diff --git a/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Utilities/AsyncBatchingWorkQueueTest.cs b/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Utilities/AsyncBatchingWorkQueueTest.cs index 4bb9c5d5f73..6b3e6b2bc0f 100644 --- a/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Utilities/AsyncBatchingWorkQueueTest.cs +++ b/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Utilities/AsyncBatchingWorkQueueTest.cs @@ -70,6 +70,7 @@ public async Task DedupesItems() return default; }, equalityComparer: EqualityComparer.Default, + preferMostRecentItems: false, DisposalToken); for (var i = 0; i < 1000; i++) From 7456e259be615c84ebbb717b2d7f544b7a4dade5 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Mon, 25 Mar 2024 13:43:22 -0700 Subject: [PATCH 11/17] Add ImmutableArray.GetMostRecentUniqueItems() helper --- .../ImmutableArrayExtensionsTests.cs | 34 ++++++++++++ .../ImmutableArrayExtensions.cs | 52 +++++++++++++++++-- 2 files changed, 81 insertions(+), 5 deletions(-) create mode 100644 src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/ImmutableArrayExtensionsTests.cs diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/ImmutableArrayExtensionsTests.cs b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/ImmutableArrayExtensionsTests.cs new file mode 100644 index 00000000000..28109500f72 --- /dev/null +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/ImmutableArrayExtensionsTests.cs @@ -0,0 +1,34 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT license. See License.txt in the project root for license information. + +using System; +using System.Collections.Immutable; +using Xunit; + +namespace Microsoft.AspNetCore.Razor.Utilities.Shared.Test; + +public class ImmutableArrayExtensionsTests +{ + [Fact] + public void GetMostRecentUniqueItems() + { + ImmutableArray items = + [ + "Hello", + "HELLO", + "HeLlO", + ", ", + ", ", + "World", + "WORLD", + "WoRlD" + ]; + + var mostRecent = items.GetMostRecentUniqueItems(StringComparer.OrdinalIgnoreCase); + + Assert.Collection(mostRecent, + s => Assert.Equal("HeLlO", s), + s => Assert.Equal(", ", s), + s => Assert.Equal("WoRlD", s)); + } +} diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs index b9507d6f21e..700fd18c341 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs @@ -64,11 +64,11 @@ public static ImmutableArray SelectAsArray(this ImmutableAr { return source switch { - [] => ImmutableArray.Empty, - [var item] => ImmutableArray.Create(selector(item)), - [var item1, var item2] => ImmutableArray.Create(selector(item1), selector(item2)), - [var item1, var item2, var item3] => ImmutableArray.Create(selector(item1), selector(item2), selector(item3)), - [var item1, var item2, var item3, var item4] => ImmutableArray.Create(selector(item1), selector(item2), selector(item3), selector(item4)), + [] => ImmutableArray.Empty, + [var item] => ImmutableArray.Create(selector(item)), + [var item1, var item2] => ImmutableArray.Create(selector(item1), selector(item2)), + [var item1, var item2, var item3] => ImmutableArray.Create(selector(item1), selector(item2), selector(item3)), + [var item1, var item2, var item3, var item4] => ImmutableArray.Create(selector(item1), selector(item2), selector(item3), selector(item4)), var items => BuildResult(items, selector) }; @@ -121,6 +121,48 @@ public static ImmutableArray WhereAsArray(this ImmutableArray source, F return builder.DrainToImmutable(); } + /// + /// Returns an that contains no duplicates from the array + /// and returns the most recent copy of each item. + /// + public static ImmutableArray GetMostRecentUniqueItems(this ImmutableArray source, IEqualityComparer comparer) + { +#if !NETSTANDARD2_0 + var uniqueItems = new HashSet(capacity: source.Length, comparer); +#else + var uniqueItems = new HashSet(comparer); +#endif + + using var stack = new PooledArrayBuilder(capacity: source.Length); + + // Walk the next batch in reverse and to identify unique items. + // We push them on a stack so that we can pop them in order later + for (var i = source.Length - 1; i >= 0; i--) + { + var item = source[i]; + + if (uniqueItems.Add(item)) + { + stack.Push(item); + } + } + + // Did we actually dedupe anything? If not, just return the original. + if (stack.Count == source.Length) + { + return source; + } + + using var result = new PooledArrayBuilder(capacity: stack.Count); + + while (stack.Count > 0) + { + result.Add(stack.Pop()); + } + + return result.DrainToImmutable(); + } + /// /// Executes a binary search over an array, but allows the caller to decide what constitutes a match /// From c5bf6e521eab032a24a1d262832c79e477f78fb5 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Mon, 25 Mar 2024 14:03:47 -0700 Subject: [PATCH 12/17] Use GetMostRecentUniqueItems(...) helper and remove mechanism from AsyncBatchingWorkQueue --- .../OpenDocumentGenerator.cs | 4 +- .../Utilities/AsyncBatchingWorkQueue.cs | 2 +- .../Utilities/AsyncBatchingWorkQueue`1.cs | 4 +- .../Utilities/AsyncBatchingWorkQueue`2.cs | 48 ------------------- .../WorkspaceProjectStateChangeDetector.cs | 4 +- .../Utilities/AsyncBatchingWorkQueueTest.cs | 1 - 6 files changed, 4 insertions(+), 59 deletions(-) diff --git a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs index f72e75115ec..a00c58bf0ba 100644 --- a/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs +++ b/src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/OpenDocumentGenerator.cs @@ -50,8 +50,6 @@ public OpenDocumentGenerator( _workQueue = new AsyncBatchingWorkQueue( s_delay, ProcessBatchAsync, - Comparer.Instance, - preferMostRecentItems: true, _disposeTokenSource.Token); _projectManager.Changed += ProjectManager_Changed; @@ -70,7 +68,7 @@ public void Dispose() private async ValueTask ProcessBatchAsync(ImmutableArray items, CancellationToken token) { - foreach (var document in items) + foreach (var document in items.GetMostRecentUniqueItems(Comparer.Instance)) { if (token.IsCancellationRequested) { diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue.cs index 4637cf31e68..4ff741e17cf 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue.cs @@ -17,7 +17,7 @@ namespace Microsoft.CodeAnalysis.Razor.Utilities; internal class AsyncBatchingWorkQueue( TimeSpan delay, Func processBatchAsync, - CancellationToken cancellationToken) : AsyncBatchingWorkQueue(delay, Convert(processBatchAsync), EqualityComparer.Default, false, cancellationToken) + CancellationToken cancellationToken) : AsyncBatchingWorkQueue(delay, Convert(processBatchAsync), EqualityComparer.Default, cancellationToken) { private static Func, CancellationToken, ValueTask> Convert(Func processBatchAsync) => (items, ct) => processBatchAsync(ct); diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`1.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`1.cs index dcdfcdfb211..7196793896f 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`1.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`1.cs @@ -18,8 +18,7 @@ internal class AsyncBatchingWorkQueue( TimeSpan delay, Func, CancellationToken, ValueTask> processBatchAsync, IEqualityComparer? equalityComparer, - bool preferMostRecentItems, - CancellationToken cancellationToken) : AsyncBatchingWorkQueue(delay, Convert(processBatchAsync), equalityComparer, preferMostRecentItems, cancellationToken) + CancellationToken cancellationToken) : AsyncBatchingWorkQueue(delay, Convert(processBatchAsync), equalityComparer, cancellationToken) { public AsyncBatchingWorkQueue( TimeSpan delay, @@ -28,7 +27,6 @@ public AsyncBatchingWorkQueue( : this(delay, processBatchAsync, equalityComparer: null, - preferMostRecentItems: false, cancellationToken) { } diff --git a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`2.cs b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`2.cs index 872141e3c74..f267145fb2f 100644 --- a/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`2.cs +++ b/src/Razor/src/Microsoft.CodeAnalysis.Razor.Workspaces/Utilities/AsyncBatchingWorkQueue`2.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Collections.Immutable; -using System.Diagnostics; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Razor; @@ -43,11 +42,6 @@ internal class AsyncBatchingWorkQueue /// private readonly IEqualityComparer? _equalityComparer; - /// - /// Determines whether the most recent items are preferred when deduping. - /// - private readonly bool _preferMostRecentItems; - /// /// Callback to actually perform the processing of the next batch of work. /// @@ -113,13 +107,11 @@ public AsyncBatchingWorkQueue( TimeSpan delay, Func, CancellationToken, ValueTask> processBatchAsync, IEqualityComparer? equalityComparer, - bool preferMostRecentItems, CancellationToken cancellationToken) { _delay = delay; _processBatchAsync = processBatchAsync; _equalityComparer = equalityComparer; - _preferMostRecentItems = preferMostRecentItems; _entireQueueCancellationToken = cancellationToken; _uniqueItems = new HashSet(equalityComparer); @@ -194,14 +186,6 @@ void AddItemsToBatch(IEnumerable items) return; } - // In the case that we want to dedupe and prefer the most recent - // items, we add everything to the batch now and will remove them later. - if (_preferMostRecentItems) - { - _nextBatch.AddRange(items); - return; - } - // Otherwise, we dedupe and prefer the first item that we see. foreach (var item in items) { @@ -297,38 +281,6 @@ void AddItemsToBatch(IEnumerable items) { lock (_gate) { - // If we're deduping and preferring the most recent items, we need - // to compare items and adjust the next batch here. - if (_nextBatch.Count > 0 && _equalityComparer is not null && _preferMostRecentItems) - { - Debug.Assert(_uniqueItems.Count == 0, "Deduping should not have already occurred."); - - using var stack = new PooledArrayBuilder(capacity: _nextBatch.Count); - - // Walk the next batch in reverse and to identify unique items. - // We push them on a stack so that we can pop them in order later - for (var i = _nextBatch.Count - 1; i >= 0; i--) - { - var item = _nextBatch[i]; - - if (_uniqueItems.Add(item)) - { - stack.Push(item); - } - } - - // Did we actually dedupe anything? If so, adjust the next batch. - if (stack.Count < _nextBatch.Count) - { - _nextBatch.Clear(); - - while (stack.Count > 0) - { - _nextBatch.Add(stack.Pop()); - } - } - } - var nextBatch = _nextBatch.ToImmutable(); // mark there being no existing update task so that the next OOP notification will diff --git a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.cs b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.cs index 41bb11d6ff6..267bffb5482 100644 --- a/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.cs +++ b/src/Razor/src/Microsoft.VisualStudio.LanguageServices.Razor/WorkspaceProjectStateChangeDetector.cs @@ -57,8 +57,6 @@ public WorkspaceProjectStateChangeDetector( _workQueue = new AsyncBatchingWorkQueue<(Project?, IProjectSnapshot)>( s_delay, ProcessBatchAsync, - Comparer.Instance, - preferMostRecentItems: true, _disposeTokenSource.Token); _projectManager.Changed += ProjectManager_Changed; @@ -82,7 +80,7 @@ public void Dispose() private async ValueTask ProcessBatchAsync(ImmutableArray<(Project? Project, IProjectSnapshot ProjectSnapshot)> items, CancellationToken token) { - foreach (var (project, projectSnapshot) in items) + foreach (var (project, projectSnapshot) in items.GetMostRecentUniqueItems(Comparer.Instance)) { if (token.IsCancellationRequested) { diff --git a/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Utilities/AsyncBatchingWorkQueueTest.cs b/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Utilities/AsyncBatchingWorkQueueTest.cs index 6b3e6b2bc0f..4bb9c5d5f73 100644 --- a/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Utilities/AsyncBatchingWorkQueueTest.cs +++ b/src/Razor/test/Microsoft.CodeAnalysis.Razor.Workspaces.Test/Utilities/AsyncBatchingWorkQueueTest.cs @@ -70,7 +70,6 @@ public async Task DedupesItems() return default; }, equalityComparer: EqualityComparer.Default, - preferMostRecentItems: false, DisposalToken); for (var i = 0; i < 1000; i++) From 929658029634c72b92f29596e6216f726598ce68 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Mon, 25 Mar 2024 14:06:26 -0700 Subject: [PATCH 13/17] Clean up ImmutableArrayExtensions --- .../ImmutableArrayExtensions.cs | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs index 700fd18c341..c1f0d0e9056 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs @@ -16,7 +16,7 @@ internal static class ImmutableArrayExtensions /// public static ImmutableArray NullToEmpty(this ImmutableArray array) { - return array.IsDefault ? ImmutableArray.Empty : array; + return array.IsDefault ? [] : array; } public static void SetCapacityIfLarger(this ImmutableArray.Builder builder, int newCapacity) @@ -46,7 +46,7 @@ public static ImmutableArray DrainToImmutable(this ImmutableArray.Build #else if (builder.Count == 0) { - return ImmutableArray.Empty; + return []; } if (builder.Count == builder.Capacity) @@ -64,11 +64,11 @@ public static ImmutableArray SelectAsArray(this ImmutableAr { return source switch { - [] => ImmutableArray.Empty, - [var item] => ImmutableArray.Create(selector(item)), - [var item1, var item2] => ImmutableArray.Create(selector(item1), selector(item2)), - [var item1, var item2, var item3] => ImmutableArray.Create(selector(item1), selector(item2), selector(item3)), - [var item1, var item2, var item3, var item4] => ImmutableArray.Create(selector(item1), selector(item2), selector(item3), selector(item4)), + [] => [], + [var item] => [selector(item)], + [var item1, var item2] => [selector(item1), selector(item2)], + [var item1, var item2, var item3] => [selector(item1), selector(item2), selector(item3)], + [var item1, var item2, var item3, var item4] => [selector(item1), selector(item2), selector(item3), selector(item4)], var items => BuildResult(items, selector) }; @@ -89,7 +89,7 @@ public static ImmutableArray SelectManyAsArray(this I { if (source is null || source.Count == 0) { - return ImmutableArray.Empty; + return []; } using var builder = new PooledArrayBuilder(capacity: source.Count); @@ -105,7 +105,7 @@ public static ImmutableArray WhereAsArray(this ImmutableArray source, F { if (source is []) { - return ImmutableArray.Empty; + return []; } using var builder = new PooledArrayBuilder(); @@ -127,6 +127,11 @@ public static ImmutableArray WhereAsArray(this ImmutableArray source, F /// public static ImmutableArray GetMostRecentUniqueItems(this ImmutableArray source, IEqualityComparer comparer) { + if (source.IsEmpty) + { + return []; + } + #if !NETSTANDARD2_0 var uniqueItems = new HashSet(capacity: source.Length, comparer); #else From de084f057ac5308ddf8d1317710667d95ecb4fe8 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Mon, 25 Mar 2024 14:29:53 -0700 Subject: [PATCH 14/17] Improve test assertion --- .../ImmutableArrayExtensionsTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/ImmutableArrayExtensionsTests.cs b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/ImmutableArrayExtensionsTests.cs index 28109500f72..8528d3bc222 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/ImmutableArrayExtensionsTests.cs +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/ImmutableArrayExtensionsTests.cs @@ -28,7 +28,7 @@ public void GetMostRecentUniqueItems() Assert.Collection(mostRecent, s => Assert.Equal("HeLlO", s), - s => Assert.Equal(", ", s), + s => Assert.Same(items[4], s), // make sure it's the most recent ", " s => Assert.Equal("WoRlD", s)); } } From 357afee67cdda2dbc4880248e8604901463f8ab2 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Tue, 26 Mar 2024 06:36:42 -0700 Subject: [PATCH 15/17] Fix indentation --- .../ImmutableArrayExtensions.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs index c1f0d0e9056..22bee7ef6cc 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs @@ -64,11 +64,11 @@ public static ImmutableArray SelectAsArray(this ImmutableAr { return source switch { - [] => [], - [var item] => [selector(item)], - [var item1, var item2] => [selector(item1), selector(item2)], - [var item1, var item2, var item3] => [selector(item1), selector(item2), selector(item3)], - [var item1, var item2, var item3, var item4] => [selector(item1), selector(item2), selector(item3), selector(item4)], + [] => [], + [var item] => [selector(item)], + [var item1, var item2] => [selector(item1), selector(item2)], + [var item1, var item2, var item3] => [selector(item1), selector(item2), selector(item3)], + [var item1, var item2, var item3, var item4] => [selector(item1), selector(item2), selector(item3), selector(item4)], var items => BuildResult(items, selector) }; From 36676816abf33a137ded8cec7bea40d287964cd7 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Tue, 26 Mar 2024 06:39:24 -0700 Subject: [PATCH 16/17] Remove extra word from comment --- .../ImmutableArrayExtensions.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs index 22bee7ef6cc..54ab5702f27 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared/ImmutableArrayExtensions.cs @@ -140,7 +140,7 @@ public static ImmutableArray GetMostRecentUniqueItems(this ImmutableArray< using var stack = new PooledArrayBuilder(capacity: source.Length); - // Walk the next batch in reverse and to identify unique items. + // Walk the next batch in reverse to identify unique items. // We push them on a stack so that we can pop them in order later for (var i = source.Length - 1; i >= 0; i--) { From ab4f568c8e378d6a9674d3a6fe2d5b297a62e09b Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Tue, 26 Mar 2024 06:46:13 -0700 Subject: [PATCH 17/17] Improve test assertion --- .../ImmutableArrayExtensionsTests.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/ImmutableArrayExtensionsTests.cs b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/ImmutableArrayExtensionsTests.cs index 8528d3bc222..c8a436095c4 100644 --- a/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/ImmutableArrayExtensionsTests.cs +++ b/src/Shared/Microsoft.AspNetCore.Razor.Utilities.Shared.Test/ImmutableArrayExtensionsTests.cs @@ -17,8 +17,8 @@ public void GetMostRecentUniqueItems() "Hello", "HELLO", "HeLlO", - ", ", - ", ", + new string([',', ' ']), + new string([',', ' ']), "World", "WORLD", "WoRlD" @@ -28,7 +28,12 @@ public void GetMostRecentUniqueItems() Assert.Collection(mostRecent, s => Assert.Equal("HeLlO", s), - s => Assert.Same(items[4], s), // make sure it's the most recent ", " + s => + { + // make sure it's the most recent ", " + Assert.NotSame(items[3], s); + Assert.Same(items[4], s); + }, s => Assert.Equal("WoRlD", s)); } }