Move RazorWorkspaceListener to AsyncBatchingWorkQueue (#10480)

In Linux + VS Code when creating a new project we're seeing work get dropped from the queue that would result in up to date information being written to disk. Looking at the logs + the bin file it's noticeable that the correct project information isn't there, and that results in Razor behaving poorly by not having the correct files in the correct projects, and often times lacking the accurate amount of taghelpers.

Switching to AsyncBatchingWorkQueue seems to improve reliability here. I also added more trace logging to help identify when information is being written, when files are moving, etc. A try/catch was added to make sure we log in case of an exception as well.

There are still issues on Linux, but this solves part of them.

Author: ryzngard

src/Razor/src/Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace/PublicAPI.Unshipped.txt

@@ -3,5 +3,4 @@ Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace.RazorWorkspaceListener
 Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace.RazorWorkspaceListener.Dispose() -> void
 Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace.RazorWorkspaceListener.EnsureInitialized(Microsoft.CodeAnalysis.Workspace! workspace, string! projectInfoFileName) -> void
 Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace.RazorWorkspaceListener.NotifyDynamicFile(Microsoft.CodeAnalysis.ProjectId! projectId) -> void
-Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace.RazorWorkspaceListener.RazorWorkspaceListener(Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void
-virtual Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace.RazorWorkspaceListener.SerializeProjectAsync(Microsoft.CodeAnalysis.ProjectId! projectId, System.Threading.CancellationToken ct) -> System.Threading.Tasks.Task!
+Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace.RazorWorkspaceListener.RazorWorkspaceListener(Microsoft.Extensions.Logging.ILoggerFactory! loggerFactory) -> void

src/Razor/src/Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace/RazorProjectInfoSerializer.cs

@@ -15,6 +15,7 @@
 using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Razor;
+using Microsoft.Extensions.Logging;
 
 namespace Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace;
 
@@ -29,17 +30,19 @@ static RazorProjectInfoSerializer()
             : StringComparison.OrdinalIgnoreCase;
     }
 
-    public static async Task SerializeAsync(Project project, string configurationFileName, CancellationToken cancellationToken)
+    public static async Task SerializeAsync(Project project, string configurationFileName, ILogger? logger, CancellationToken cancellationToken)
     {
         var projectPath = Path.GetDirectoryName(project.FilePath);
         if (projectPath is null)
         {
+            logger?.LogTrace("projectPath is null, skipping writing info for {projectId}", project.Id);
             return;
         }
 
         var intermediateOutputPath = Path.GetDirectoryName(project.CompilationOutputInfo.AssemblyPath);
         if (intermediateOutputPath is null)
         {
+            logger?.LogTrace("intermediatePath is null, skipping writing info for {projectId}", project.Id);
             return;
         }
 
@@ -49,13 +52,14 @@ public static async Task SerializeAsync(Project project, string configurationFil
         // Not a razor project
         if (documents.Length == 0)
         {
+            logger?.LogTrace("No razor documents for {projectId}", project.Id);
             return;
         }
 
         var csharpLanguageVersion = (project.ParseOptions as CSharpParseOptions)?.LanguageVersion ?? LanguageVersion.Default;
 
         var options = project.AnalyzerOptions.AnalyzerConfigOptionsProvider;
-        var configuration = ComputeRazorConfigurationOptions(options, out var defaultNamespace);
+        var configuration = ComputeRazorConfigurationOptions(options, logger, out var defaultNamespace);
 
         var fileSystem = RazorProjectFileSystem.Create(projectPath);
 
@@ -93,10 +97,10 @@ public static async Task SerializeAsync(Project project, string configurationFil
             projectWorkspaceState: projectWorkspaceState,
             documents: documents);
 
-        WriteToFile(configurationFilePath, projectInfo);
+        WriteToFile(configurationFilePath, projectInfo, logger);
     }
 
-    private static RazorConfiguration ComputeRazorConfigurationOptions(AnalyzerConfigOptionsProvider options, out string defaultNamespace)
+    private static RazorConfiguration ComputeRazorConfigurationOptions(AnalyzerConfigOptionsProvider options, ILogger? logger, out string defaultNamespace)
     {
         // See RazorSourceGenerator.RazorProviders.cs
 
@@ -111,6 +115,7 @@ private static RazorConfiguration ComputeRazorConfigurationOptions(AnalyzerConfi
         if (!globalOptions.TryGetValue("build_property.RazorLangVersion", out var razorLanguageVersionString) ||
             !RazorLanguageVersion.TryParse(razorLanguageVersionString, out var razorLanguageVersion))
         {
+            logger?.LogTrace("Using default of latest language version");
             razorLanguageVersion = RazorLanguageVersion.Latest;
         }
 
@@ -121,7 +126,7 @@ private static RazorConfiguration ComputeRazorConfigurationOptions(AnalyzerConfi
         return razorConfiguration;
     }
 
-    private static void WriteToFile(string configurationFilePath, RazorProjectInfo projectInfo)
+    private static void WriteToFile(string configurationFilePath, RazorProjectInfo projectInfo, ILogger? logger)
     {
         // We need to avoid having an incomplete file at any point, but our
         // project configuration is large enough that it will be written as multiple operations.
@@ -131,6 +136,7 @@ private static void WriteToFile(string configurationFilePath, RazorProjectInfo p
         if (tempFileInfo.Exists)
         {
             // This could be caused by failures during serialization or early process termination.
+            logger?.LogTrace("deleting existing file {filePath}", tempFilePath);
             tempFileInfo.Delete();
         }
 
@@ -144,9 +150,11 @@ private static void WriteToFile(string configurationFilePath, RazorProjectInfo p
         var fileInfo = new FileInfo(configurationFilePath);
         if (fileInfo.Exists)
         {
+            logger?.LogTrace("deleting existing file {filePath}", configurationFilePath);
             fileInfo.Delete();
         }
 
+        logger?.LogTrace("Moving {tmpPath} to {newPath}", tempFilePath, configurationFilePath);
         File.Move(tempFileInfo.FullName, configurationFilePath);
     }
 

src/Razor/src/Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace/RazorWorkspaceListener.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT license. See License.txt in the project root for license information.
 
 using System.Collections.Immutable;
+using Microsoft.AspNetCore.Razor.Utilities;
 using Microsoft.CodeAnalysis;
 using Microsoft.Extensions.Logging;
 
@@ -15,11 +16,33 @@ public class RazorWorkspaceListener : IDisposable
 
     private string? _projectInfoFileName;
     private Workspace? _workspace;
-    private ImmutableDictionary<ProjectId, TaskDelayScheduler> _workQueues = ImmutableDictionary<ProjectId, TaskDelayScheduler>.Empty;
+
+    // Use an immutable dictionary for ImmutableInterlocked operations. The value isn't checked, just
+    // the existance of the key so work is only done for projects with dynamic files.
+    private ImmutableDictionary<ProjectId, bool> _projectsWithDynamicFile = ImmutableDictionary<ProjectId, bool>.Empty;
+    private readonly CancellationTokenSource _disposeTokenSource = new();
+    private readonly AsyncBatchingWorkQueue<ProjectId> _workQueue;
 
     public RazorWorkspaceListener(ILoggerFactory loggerFactory)
     {
         _logger = loggerFactory.CreateLogger(nameof(RazorWorkspaceListener));
+        _workQueue = new(TimeSpan.FromMilliseconds(500), UpdateCurrentProjectsAsync, EqualityComparer<ProjectId>.Default, _disposeTokenSource.Token);
+    }
+
+    public void Dispose()
+    {
+        if (_workspace is not null)
+        {
+            _workspace.WorkspaceChanged -= Workspace_WorkspaceChanged;
+        }
+
+        if (_disposeTokenSource.IsCancellationRequested)
+        {
+            return;
+        }
+
+        _disposeTokenSource.Cancel();
+        _disposeTokenSource.Dispose();
     }
 
     public void EnsureInitialized(Workspace workspace, string projectInfoFileName)
@@ -45,14 +68,11 @@ public void NotifyDynamicFile(ProjectId projectId)
             return;
         }
 
-        // We expect this to be called multiple times per project so a no-op update operation seems like a better choice
-        // than constructing a new TaskDelayScheduler each time, and using the TryAdd method, which doesn't support a
-        // valueFactory argument.
-        var scheduler = ImmutableInterlocked.AddOrUpdate(ref _workQueues, projectId, static _ => new TaskDelayScheduler(s_debounceTime, CancellationToken.None), static (_, val) => val);
+        ImmutableInterlocked.GetOrAdd(ref _projectsWithDynamicFile, projectId, static (_) => true);
 
         // Schedule a task, in case adding a dynamic file is the last thing that happens
         _logger.LogTrace("{projectId} scheduling task due to dynamic file", projectId);
-        scheduler.ScheduleAsyncTask(ct => SerializeProjectAsync(projectId, ct), CancellationToken.None);
+        _workQueue.AddWork(projectId);
     }
 
     private void Workspace_WorkspaceChanged(object? sender, WorkspaceChangeEventArgs e)
@@ -61,11 +81,6 @@ private void Workspace_WorkspaceChanged(object? sender, WorkspaceChangeEventArgs
         {
             case WorkspaceChangeKind.SolutionChanged:
             case WorkspaceChangeKind.SolutionReloaded:
-                foreach (var project in e.OldSolution.Projects)
-                {
-                    RemoveProject(project);
-                }
-
                 foreach (var project in e.NewSolution.Projects)
                 {
                     EnqueueUpdate(project);
@@ -81,15 +96,14 @@ private void Workspace_WorkspaceChanged(object? sender, WorkspaceChangeEventArgs
 
                 break;
 
-            case WorkspaceChangeKind.ProjectRemoved:
-                RemoveProject(e.OldSolution.GetProject(e.ProjectId));
-                break;
-
             case WorkspaceChangeKind.ProjectReloaded:
-                RemoveProject(e.OldSolution.GetProject(e.ProjectId));
                 EnqueueUpdate(e.NewSolution.GetProject(e.ProjectId));
                 break;
 
+            case WorkspaceChangeKind.ProjectRemoved:
+                RemoveProject(e.ProjectId.AssumeNotNull());
+                break;
+
             case WorkspaceChangeKind.ProjectAdded:
             case WorkspaceChangeKind.ProjectChanged:
             case WorkspaceChangeKind.DocumentAdded:
@@ -112,30 +126,24 @@ private void Workspace_WorkspaceChanged(object? sender, WorkspaceChangeEventArgs
                 }
 
                 break;
+
             case WorkspaceChangeKind.SolutionCleared:
             case WorkspaceChangeKind.SolutionRemoved:
                 foreach (var project in e.OldSolution.Projects)
                 {
-                    RemoveProject(project);
+                    RemoveProject(project.Id);
                 }
 
                 break;
+
             default:
                 break;
         }
     }
 
-    private void RemoveProject(Project? project)
+    private void RemoveProject(ProjectId projectId)
     {
-        if (project is null)
-        {
-            return;
-        }
-
-        if (ImmutableInterlocked.TryRemove(ref _workQueues, project.Id, out var scheduler))
-        {
-            scheduler.Dispose();
-        }
+        ImmutableInterlocked.TryRemove(ref _projectsWithDynamicFile, projectId, out var _);
     }
 
     private void EnqueueUpdate(Project? project)
@@ -149,44 +157,42 @@ project is not
             return;
         }
 
-        var projectId = project.Id;
-        if (_workQueues.TryGetValue(projectId, out var scheduler))
+        // Don't queue work for projects that don't have a dynamic file
+        if (!_projectsWithDynamicFile.TryGetValue(project.Id, out var _))
         {
-            _logger.LogTrace("{projectId} scheduling task due to workspace event", projectId);
-
-            scheduler.ScheduleAsyncTask(ct => SerializeProjectAsync(projectId, ct), CancellationToken.None);
+            return;
         }
+
+        var projectId = project.Id;
+        _workQueue.AddWork(projectId);
     }
 
-    // Protected for testing
-    protected virtual Task SerializeProjectAsync(ProjectId projectId, CancellationToken ct)
+    private async ValueTask UpdateCurrentProjectsAsync(ImmutableArray<ProjectId> projectIds, CancellationToken cancellationToken)
     {
-        if (_projectInfoFileName is null || _workspace is null)
-        {
-            return Task.CompletedTask;
-        }
+        var solution = _workspace.AssumeNotNull().CurrentSolution;
 
-        var project = _workspace.CurrentSolution.GetProject(projectId);
-        if (project is null)
+        foreach (var projectId in projectIds)
         {
-            return Task.CompletedTask;
-        }
+            if (_disposeTokenSource.IsCancellationRequested)
+            {
+                return;
+            }
 
-        _logger.LogTrace("{projectId} writing json file", projectId);
-        return RazorProjectInfoSerializer.SerializeAsync(project, _projectInfoFileName, ct);
-    }
+            var project = solution.GetProject(projectId);
+            if (project is null)
+            {
+                _logger?.LogTrace("Project {projectId} is not in workspace", projectId);
+                continue;
+            }
 
-    public void Dispose()
-    {
-        if (_workspace is not null)
-        {
-            _workspace.WorkspaceChanged -= Workspace_WorkspaceChanged;
+            await SerializeProjectAsync(project, solution, cancellationToken).ConfigureAwait(false);
         }
+    }
 
-        var queues = Interlocked.Exchange(ref _workQueues, ImmutableDictionary<ProjectId, TaskDelayScheduler>.Empty);
-        foreach (var (_, value) in queues)
-        {
-            value.Dispose();
-        }
+    // private protected for testing
+    private protected virtual Task SerializeProjectAsync(Project project, Solution solution, CancellationToken cancellationToken)
+    {
+        _logger?.LogTrace("Serializing information for {projectId}", project.Id);
+        return RazorProjectInfoSerializer.SerializeAsync(project, _projectInfoFileName.AssumeNotNull(), _logger, cancellationToken);
     }
 }

src/Razor/src/Microsoft.AspNetCore.Razor.ExternalAccess.RoslynWorkspace/TaskDelayScheduler.cs

@@ -11,11 +11,11 @@
 using Microsoft.AspNetCore.Razor.Language;
 using Microsoft.AspNetCore.Razor.LanguageServer.Hosting;
 using Microsoft.AspNetCore.Razor.PooledObjects;
+using Microsoft.AspNetCore.Razor.Utilities;
 using Microsoft.CodeAnalysis.Razor;
 using Microsoft.CodeAnalysis.Razor.Logging;
 using Microsoft.CodeAnalysis.Razor.ProjectSystem;
 using Microsoft.CodeAnalysis.Razor.Protocol;
-using Microsoft.CodeAnalysis.Razor.Utilities;
 using Microsoft.CodeAnalysis.Razor.Workspaces;
 using Microsoft.VisualStudio.LanguageServer.Protocol;
 using Microsoft.VisualStudio.Threading;

src/Razor/src/Microsoft.AspNetCore.Razor.LanguageServer/Diagnostics/RazorDiagnosticsPublisher.cs

@@ -6,9 +6,9 @@
 using System.Collections.Immutable;
 using System.Threading;
 using System.Threading.Tasks;
+using Microsoft.AspNetCore.Razor.Utilities;
 using Microsoft.CodeAnalysis.Razor.Logging;
 using Microsoft.CodeAnalysis.Razor.ProjectSystem;
-using Microsoft.CodeAnalysis.Razor.Utilities;
 using Microsoft.CodeAnalysis.Razor.Workspaces;
 
 namespace Microsoft.AspNetCore.Razor.LanguageServer;