From ea8db6906f5abb2c791b07a54c288e13ed61cd12 Mon Sep 17 00:00:00 2001 From: Dustin Campbell Date: Thu, 21 Mar 2024 12:03:08 -0700 Subject: [PATCH] Delete old BatchingWorkQueue --- .../BatchingWorkQueue.cs | 284 ------------------ .../BatchingWorkQueueTest.cs | 256 ---------------- 2 files changed, 540 deletions(-) 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/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); - } -}