From bc33a80715cb6b2e2fe9ec062323368a37e94ce1 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Mar 2024 10:38:51 -0800 Subject: [PATCH 1/3] Update semantic token refresh queue. --- .../CompilationAvailableEventSource.cs | 95 ------------------- .../IRemoteCompilationAvailableService.cs | 14 --- .../SemanticTokensRefreshQueue.cs | 92 +++++------------- .../Remote/Core/ServiceDescriptors.cs | 1 - 4 files changed, 22 insertions(+), 180 deletions(-) delete mode 100644 src/Features/Core/Portable/Shared/Utilities/CompilationAvailableEventSource.cs delete mode 100644 src/Features/Core/Portable/Workspace/IRemoteCompilationAvailableService.cs diff --git a/src/Features/Core/Portable/Shared/Utilities/CompilationAvailableEventSource.cs b/src/Features/Core/Portable/Shared/Utilities/CompilationAvailableEventSource.cs deleted file mode 100644 index c780d82092b4f..0000000000000 --- a/src/Features/Core/Portable/Shared/Utilities/CompilationAvailableEventSource.cs +++ /dev/null @@ -1,95 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System; -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Host; -using Microsoft.CodeAnalysis.Remote; -using Microsoft.CodeAnalysis.Shared.TestHooks; -using Microsoft.CodeAnalysis.Tagging; -using Roslyn.Utilities; - -namespace Microsoft.CodeAnalysis.Shared.Utilities; - -/// -/// Helper type that can be used to ask for a to be produced in our OOP server for a -/// particular , asking for a callback to be executed when that has happened. Each time this -/// is asked for for a particular project, any existing outstanding work to produce a for -/// a prior will be cancelled. -/// -internal class CompilationAvailableEventSource( - IAsynchronousOperationListener asyncListener) : IDisposable -{ - private readonly IAsynchronousOperationListener _asyncListener = asyncListener; - - /// - /// Cancellation tokens controlling background computation of the compilation. - /// - private readonly ReferenceCountedDisposable _cancellationSeries = new(new CancellationSeries()); - - public void Dispose() - => _cancellationSeries.Dispose(); - - /// - /// Request that the compilation for be made available in our OOP server, calling back on - /// once that happens. Subsequence calls to this method will cancel - /// any outstanding requests in flight. - /// - public void EnsureCompilationAvailability(Project project, Action onCompilationAvailable) - { - if (project == null) - return; - - if (!project.SupportsCompilation) - return; - - using var cancellationSeries = _cancellationSeries.TryAddReference(); - if (cancellationSeries is null) - { - // Already in the process of disposing this instance - return; - } - - // Cancel any existing tasks that are computing the compilation and spawn a new one to compute - // it and notify any listening clients. - var cancellationToken = cancellationSeries.Target.CreateNext(); - - var token = _asyncListener.BeginAsyncOperation(nameof(EnsureCompilationAvailability)); - var task = Task.Run(async () => - { - // Support cancellation without throwing. - // - // We choose a long delay here so that we can avoid this work as long as the user is continually making - // changes to their code. During that time, features that use this are already kicking off fast work - // with frozen-partial semantics and we'd like that to not have to contend with more expensive work - // kicked off in OOP to compute full compilations. - await _asyncListener.Delay(DelayTimeSpan.NonFocus, cancellationToken).NoThrowAwaitableInternal(captureContext: false); - if (cancellationToken.IsCancellationRequested) - return; - - var client = await RemoteHostClient.TryGetClientAsync(project, cancellationToken).ConfigureAwait(false); - if (client != null) - { - var result = await client.TryInvokeAsync( - project, - (service, solutionInfo, cancellationToken) => service.ComputeCompilationAsync(solutionInfo, project.Id, cancellationToken), - cancellationToken).ConfigureAwait(false); - - if (!result) - return; - } - else - { - // if we can't get the client, just compute the compilation locally and fire the event once we have it. - await CompilationAvailableHelpers.ComputeCompilationInCurrentProcessAsync(project, cancellationToken).ConfigureAwait(false); - } - - // now that we know we have an full compilation, let the caller know so it can do whatever it needs in - // response. - onCompilationAvailable(); - }, cancellationToken); - task.CompletesAsyncOperation(token); - } -} diff --git a/src/Features/Core/Portable/Workspace/IRemoteCompilationAvailableService.cs b/src/Features/Core/Portable/Workspace/IRemoteCompilationAvailableService.cs deleted file mode 100644 index 3fa952751ba54..0000000000000 --- a/src/Features/Core/Portable/Workspace/IRemoteCompilationAvailableService.cs +++ /dev/null @@ -1,14 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Remote; - -namespace Microsoft.CodeAnalysis.Host; - -internal interface IRemoteCompilationAvailableService -{ - ValueTask ComputeCompilationAsync(Checksum solutionChecksum, ProjectId projectId, CancellationToken cancellationToken); -} diff --git a/src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensRefreshQueue.cs b/src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensRefreshQueue.cs index b142f1100e336..4628b4a80a6eb 100644 --- a/src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensRefreshQueue.cs +++ b/src/Features/LanguageServer/Protocol/Handler/SemanticTokens/SemanticTokensRefreshQueue.cs @@ -4,24 +4,22 @@ using System; using System.Collections.Generic; -using System.Collections.Immutable; using System.IO; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.Collections; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Shared.TestHooks; -using Microsoft.CodeAnalysis.Shared.Utilities; using Roslyn.LanguageServer.Protocol; using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.LanguageServer.Handler.SemanticTokens; /// -/// Batches requests to refresh the semantic tokens to optomize user experience. +/// Batches requests to refresh the semantic tokens to optimize user experience. /// -/// This implements to avoid race conditions -/// related to creating the queue on the first request. +/// This implements to avoid race conditions related to creating the queue on the +/// first request. internal class SemanticTokensRefreshQueue : IOnInitialized, ILspService, @@ -32,11 +30,6 @@ internal class SemanticTokensRefreshQueue : /// private readonly object _gate = new(); - /// - /// Mapping from project id to the workqueue for producing the corresponding compilation for it on the OOP server. - /// - private readonly Dictionary _projectIdToEventSource = []; - /// /// Mapping from project id to the project-cone-checksum for it we were at when the project for it had its /// compilation produced on the oop server. @@ -48,12 +41,12 @@ internal class SemanticTokensRefreshQueue : private readonly ICapabilitiesProvider _capabilitiesProvider; private readonly IAsynchronousOperationListener _asyncListener; - private readonly CancellationTokenSource _disposalTokenSource; + private readonly CancellationTokenSource _disposalTokenSource = new(); /// /// Debouncing queue so that we don't attempt to issue a semantic tokens refresh notification too often. - /// - /// Null when the client does not support sending refresh notifications. + /// + /// when the client does not support sending refresh notifications. /// private AsyncBatchingWorkQueue? _semanticTokenRefreshQueue; @@ -69,8 +62,6 @@ public SemanticTokensRefreshQueue( _asyncListener = asynchronousOperationListenerProvider.GetListener(FeatureAttribute.Classification); _lspWorkspaceRegistrationService = lspWorkspaceRegistrationService; - _disposalTokenSource = new(); - _lspWorkspaceManager = lspWorkspaceManager; _notificationManager = notificationManager; _capabilitiesProvider = capabilitiesProvider; @@ -82,14 +73,12 @@ public Task OnInitializedAsync(ClientCapabilities clientCapabilities, RequestCon && clientCapabilities.Workspace?.SemanticTokens?.RefreshSupport is true && _capabilitiesProvider.GetCapabilities(clientCapabilities).SemanticTokensOptions is not null) { - // Only send a refresh notification to the client every 2s (if needed) in order to avoid - // sending too many notifications at once. This ensures we batch up workspace notifications, - // but also means we send soon enough after a compilation-computation to not make the user wait - // an enormous amount of time. + // Only send a refresh notification to the client every 2s (if needed) in order to avoid sending too many + // notifications at once. This ensures we batch up workspace notifications, but also means we send soon + // enough after a compilation-computation to not make the user wait an enormous amount of time. _semanticTokenRefreshQueue = new AsyncBatchingWorkQueue( delay: TimeSpan.FromMilliseconds(2000), - processBatchAsync: (documentUris, cancellationToken) - => FilterLspTrackedDocumentsAsync(_lspWorkspaceManager, _notificationManager, documentUris, cancellationToken), + processBatchAsync: FilterLspTrackedDocumentsAsync, equalityComparer: EqualityComparer.Default, asyncListener: _asyncListener, _disposalTokenSource.Token); @@ -104,41 +93,37 @@ public async Task TryEnqueueRefreshComputationAsync(Project project, Cancellatio { if (_semanticTokenRefreshQueue is not null) { - // Determine the checksum for this project cone. Note: this should be fast in practice because this is - // the same project-cone-checksum we used to even call into OOP above when we computed semantic tokens. + // Determine the checksum for this project cone. Note: this should be fast in practice because this is the + // same project-cone-checksum we used to even call into OOP above when we computed semantic tokens. var projectChecksum = await project.Solution.CompilationState.GetChecksumAsync(project.Id, cancellationToken).ConfigureAwait(false); lock (_gate) { // If this checksum is the same as the last computed result, no need to continue, we would not produce a // different compilation. - if (ChecksumIsUnchanged_NoLock(project, projectChecksum)) + if (_projectIdToLastComputedChecksum.TryGetValue(project.Id, out var lastChecksum) && lastChecksum == projectChecksum) return; - if (!_projectIdToEventSource.TryGetValue(project.Id, out var eventSource)) - { - eventSource = new CompilationAvailableEventSource(_asyncListener); - _projectIdToEventSource.Add(project.Id, eventSource); - } + // keep track of this checksum. That way we don't get into a loop where we send a refresh notification, + // then we get called back into, causing us to compute the compilation, causing us to send the refresh + // notification, etc. etc. + _projectIdToLastComputedChecksum[project.Id] = projectChecksum; - eventSource.EnsureCompilationAvailability(project, () => OnCompilationAvailable(project, projectChecksum)); } + + EnqueueSemanticTokenRefreshNotification(documentUri: null); } } - private static ValueTask FilterLspTrackedDocumentsAsync( - LspWorkspaceManager lspWorkspaceManager, - IClientLanguageServerManager notificationManager, + private ValueTask FilterLspTrackedDocumentsAsync( ImmutableSegmentedList documentUris, CancellationToken cancellationToken) { - var trackedDocuments = lspWorkspaceManager.GetTrackedLspText(); + var trackedDocuments = _lspWorkspaceManager.GetTrackedLspText(); foreach (var documentUri in documentUris) { if (documentUri is null || !trackedDocuments.ContainsKey(documentUri)) - { - return notificationManager.SendRequestAsync(Methods.WorkspaceSemanticTokensRefreshName, cancellationToken); - } + return _notificationManager.SendRequestAsync(Methods.WorkspaceSemanticTokensRefreshName, cancellationToken); } // LSP is already tracking all changed documents so we don't need to send a refresh request. @@ -198,46 +183,13 @@ private void EnqueueSemanticTokenRefreshNotification(Uri? documentUri) _semanticTokenRefreshQueue.AddWork(documentUri); } - private void OnCompilationAvailable(Project project, Checksum projectChecksum) - { - lock (_gate) - { - // Paranoia: It's technically possible (though unlikely) for two calls to compute the compilation for - // the same project to come back and call into this. This is because the - // CompilationAvailableEventSource uses cooperative cancellation to cancel the in-flight request before - // issuing the new one. There is no requirement though that the inflight request actually stop (or run - // to completion) prior to the next request running and completing. In practice this should not happen - // as cancellation is checked fairly regularly. However, if it does, check and do not bother to issue a - // refresh in this case. - if (ChecksumIsUnchanged_NoLock(project, projectChecksum)) - return; - - // keep track of this checksum. That way we don't get into a loop where we send a refresh notification, - // then we get called back into, causing us to compute the compilation, causing us to send the refresh - // notification, etc. etc. - _projectIdToLastComputedChecksum[project.Id] = projectChecksum; - } - - EnqueueSemanticTokenRefreshNotification(documentUri: null); - } - - private bool ChecksumIsUnchanged_NoLock(Project project, Checksum projectChecksum) - => _projectIdToLastComputedChecksum.TryGetValue(project.Id, out var lastChecksum) && lastChecksum == projectChecksum; - public void Dispose() { - ImmutableArray eventSources; lock (_gate) { - eventSources = _projectIdToEventSource.Values.ToImmutableArray(); - _projectIdToEventSource.Clear(); - _lspWorkspaceRegistrationService.LspSolutionChanged -= OnLspSolutionChanged; } - foreach (var eventSource in eventSources) - eventSource.Dispose(); - _disposalTokenSource.Cancel(); _disposalTokenSource.Dispose(); } diff --git a/src/Workspaces/Remote/Core/ServiceDescriptors.cs b/src/Workspaces/Remote/Core/ServiceDescriptors.cs index 171eddb9dc002..c375fee080ccb 100644 --- a/src/Workspaces/Remote/Core/ServiceDescriptors.cs +++ b/src/Workspaces/Remote/Core/ServiceDescriptors.cs @@ -80,7 +80,6 @@ internal sealed class ServiceDescriptors (typeof(IRemoteInheritanceMarginService), null), (typeof(IRemoteUnusedReferenceAnalysisService), null), (typeof(IRemoteProcessTelemetryService), null), - (typeof(IRemoteCompilationAvailableService), null), (typeof(IRemoteLegacySolutionEventsAggregationService), null), (typeof(IRemoteStackTraceExplorerService), null), (typeof(IRemoteUnitTestingSearchService), null), From 62521e8d449a8a460dcd860e94aa93a8f8fcd4a2 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Mar 2024 11:33:47 -0800 Subject: [PATCH 2/3] Remove type --- ...RemoteTaggerCompilationAvailableService.cs | 39 ------------------- 1 file changed, 39 deletions(-) delete mode 100644 src/Workspaces/Remote/ServiceHub/Services/Tagging/RemoteTaggerCompilationAvailableService.cs diff --git a/src/Workspaces/Remote/ServiceHub/Services/Tagging/RemoteTaggerCompilationAvailableService.cs b/src/Workspaces/Remote/ServiceHub/Services/Tagging/RemoteTaggerCompilationAvailableService.cs deleted file mode 100644 index 62135c02710dd..0000000000000 --- a/src/Workspaces/Remote/ServiceHub/Services/Tagging/RemoteTaggerCompilationAvailableService.cs +++ /dev/null @@ -1,39 +0,0 @@ -// Licensed to the .NET Foundation under one or more agreements. -// The .NET Foundation licenses this file to you under the MIT license. -// See the LICENSE file in the project root for more information. - -using System.Threading; -using System.Threading.Tasks; -using Microsoft.CodeAnalysis.Host; -using Microsoft.CodeAnalysis.Shared.Extensions; -using Microsoft.CodeAnalysis.Tagging; - -namespace Microsoft.CodeAnalysis.Remote -{ - internal sealed class RemoteCompilationAvailableService : BrokeredServiceBase, IRemoteCompilationAvailableService - { - internal sealed class Factory : FactoryBase - { - protected override IRemoteCompilationAvailableService CreateService(in ServiceConstructionArguments arguments) - => new RemoteCompilationAvailableService(arguments); - } - - public RemoteCompilationAvailableService(in ServiceConstructionArguments arguments) - : base(arguments) - { - } - - public ValueTask ComputeCompilationAsync( - Checksum solutionChecksum, - ProjectId projectId, - CancellationToken cancellationToken) - { - return RunServiceAsync(solutionChecksum, async solution => - { - var project = solution.GetRequiredProject(projectId); - - await CompilationAvailableHelpers.ComputeCompilationInCurrentProcessAsync(project, cancellationToken).ConfigureAwait(false); - }, cancellationToken); - } - } -} From 734de55b4af386f076cbe28e105ec7c7d5c9a9b4 Mon Sep 17 00:00:00 2001 From: Cyrus Najmabadi Date: Tue, 5 Mar 2024 11:44:01 -0800 Subject: [PATCH 3/3] Remove --- .../CoreTestUtilities/Remote/InProcRemostHostClient.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Workspaces/CoreTestUtilities/Remote/InProcRemostHostClient.cs b/src/Workspaces/CoreTestUtilities/Remote/InProcRemostHostClient.cs index 39c69bc4c0702..d6438a07d9c5a 100644 --- a/src/Workspaces/CoreTestUtilities/Remote/InProcRemostHostClient.cs +++ b/src/Workspaces/CoreTestUtilities/Remote/InProcRemostHostClient.cs @@ -208,7 +208,6 @@ public InProcRemoteServices(SolutionServices workspaceServices, TraceListener? t RegisterRemoteBrokeredService(new RemoteValueTrackingService.Factory()); RegisterRemoteBrokeredService(new RemoteInheritanceMarginService.Factory()); RegisterRemoteBrokeredService(new RemoteUnusedReferenceAnalysisService.Factory()); - RegisterRemoteBrokeredService(new RemoteCompilationAvailableService.Factory()); RegisterRemoteBrokeredService(new RemoteLegacySolutionEventsAggregationService.Factory()); RegisterRemoteBrokeredService(new RemoteStackTraceExplorerService.Factory()); RegisterRemoteBrokeredService(new RemoteUnitTestingSearchService.Factory());