From d1c5b7cc5d96c26b471c6a6d5ffb8b9aa340a500 Mon Sep 17 00:00:00 2001 From: Jimmy Byrd Date: Mon, 16 Feb 2026 23:08:04 -0500 Subject: [PATCH 01/10] Add shared progress reporter for concurrent typecheck operations --- .../LspServers/AdaptiveServerState.fs | 92 +++++----- .../LspServers/FSharpLspClient.fs | 162 ++++++++++++++++++ .../LspServers/FSharpLspClient.fsi | 19 ++ 3 files changed, 220 insertions(+), 53 deletions(-) diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index c0537f977..e3200f9a2 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -219,6 +219,14 @@ type AdaptiveState let progressLookup = new ServerProgressLookup() do disposables.Add progressLookup + let typecheckProgressReporter = + new SharedTypecheckProgressReporter( + "Typechecking", + fun () -> progressLookup.CreateProgressReport(lspClient, cancellable = true) + ) + + do disposables.Add typecheckProgressReporter + let serilogLoggerFactory = new SerilogLoggerFactory(Serilog.Log.Logger) do disposables.Add serilogLoggerFactory @@ -1950,11 +1958,8 @@ type AdaptiveState ) - use progressReport = - progressLookup.CreateProgressReport(lspClient, cancellable = true) - - let simpleName = Path.GetFileName(UMX.untag file.Source.FileName) - do! progressReport.Begin($"Typechecking {simpleName}", message = $"{file.Source.FileName}") + let fileNameStr = UMX.untag file.Source.FileName + use! _fileProgress = typecheckProgressReporter.Begin(fileNameStr) let! result = match compilerOptions with @@ -2725,29 +2730,23 @@ type AdaptiveState |> List.collect (fun (proj, snap) -> snap.SourceFilesTagged |> List.map (fun sourceFile -> proj, sourceFile)) |> List.toArray - let mutable checksCompleted = 0 + let innerChecks = + Array.concat [| dependentFiles; dependentProjectsAndSourceFiles |] + |> Array.filter (fun (_, file) -> + let file = UMX.untag file - use progressReporter = - progressLookup.CreateProgressReport(lspClient, cancellable = true) + file.Contains "AssemblyInfo.fs" |> not + && file.Contains "AssemblyAttributes.fs" |> not) - let percentage numerator denominator = - if denominator = 0 then - 0u - else - ((float numerator) / (float denominator)) * 100.0 |> uint32 + let batchFileList = innerChecks |> Array.map (fun (_, file) -> UMX.untag file) - let checksToPerform = - let innerChecks = - Array.concat [| dependentFiles; dependentProjectsAndSourceFiles |] - |> Array.filter (fun (_, file) -> - let file = UMX.untag file + // Start batch first so the report (and its CancellationToken) exists before per-file work begins + use! _batchProgress = typecheckProgressReporter.BeginBatch(batchFileList) + let! progressCt = typecheckProgressReporter.GetCancellationToken() - file.Contains "AssemblyInfo.fs" |> not - && file.Contains "AssemblyAttributes.fs" |> not) - - let checksToPerformLength = innerChecks.Length - let rootToken = sourceFilePath |> getOpenFileTokenOrDefault + let rootToken = sourceFilePath |> getOpenFileTokenOrDefault + let checksToPerform = innerChecks |> Array.map (fun (proj, file) -> async { @@ -2755,46 +2754,33 @@ type AdaptiveState use joinedToken = if file = sourceFilePath then // dont reset the token for the incoming file as it would cancel the whole operation - CancellationTokenSource.CreateLinkedTokenSource(rootToken, progressReporter.CancellationToken) + CancellationTokenSource.CreateLinkedTokenSource(rootToken, progressCt) else // only cancel other files // If we have multiple saves from separate root files we want only one to be running let fileToken = resetCancellationToken file None // and join with the root token as well since we want to cancel the whole operation if the root files changes - CancellationTokenSource.CreateLinkedTokenSource( - rootToken, - fileToken, - progressReporter.CancellationToken - ) - - try - let! _ = - bypassAdaptiveTypeCheck (file) (proj) (AVal.force proj.FSharpProjectCompilerOptions) - |> Async.withCancellation joinedToken.Token + CancellationTokenSource.CreateLinkedTokenSource(rootToken, fileToken, progressCt) - () - with :? OperationCanceledException -> - // if a file shows up multiple times in the list such as Microsoft.NET.Test.Sdk.Program.fs we may cancel it but we don't want to stop the whole operation for it - () + // Track per-file progress so the shared reporter updates message and batch counter + let! fileProgress = + typecheckProgressReporter.Begin (UMX.untag file) CancellationToken.None + |> Async.AwaitTask - let checksCompleted = Interlocked.Increment(&checksCompleted) + try + try + let! _ = + bypassAdaptiveTypeCheck (file) (proj) (AVal.force proj.FSharpProjectCompilerOptions) + |> Async.withCancellation joinedToken.Token - do! - progressReporter.Report( - message = $"{checksCompleted}/{checksToPerformLength} remaining", - percentage = percentage checksCompleted checksToPerformLength - ) + () + with :? OperationCanceledException -> + // if a file shows up multiple times in the list such as Microsoft.NET.Test.Sdk.Program.fs we may cancel it but we don't want to stop the whole operation for it + () + finally + fileProgress.DisposeAsync().AsTask().GetAwaiter().GetResult() }) - - - do! - progressReporter.Begin( - "Typechecking Dependent F# files", - message = $"0/{checksToPerform.Length} remaining", - percentage = percentage 0 checksToPerform.Length - ) - do! checksToPerform |> Async.parallel75 |> Async.Ignore } diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fs b/src/FsAutoComplete/LspServers/FSharpLspClient.fs index d015bc88a..18fd9c6e3 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fs +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fs @@ -187,6 +187,168 @@ type ServerProgressReport(lspClient: FSharpLspClient, ?token: ProgressToken, ?ca member x.Dispose() = (x :> IAsyncDisposable).DisposeAsync() |> ignore +/// +/// A shared progress reporter that consolidates multiple concurrent typecheck operations +/// into a single LSP progress notification. Instead of creating a new Begin/End cycle per file, +/// this maintains one notification that updates its message with the current file being checked. +/// +type SharedTypecheckProgressReporter + (title: string, createReport: unit -> ServerProgressReport) = + + let locker = new SemaphoreSlim(1, 1) + /// Display-only set of short file names currently being checked + let mutable activeFiles = Set.empty + let mutable progressReport: ServerProgressReport option = None + let mutable batchTotal = 0 + let mutable batchCompleted = 0 + /// Tracks which files belong to active batches (by full normalized path) for accurate counting + let mutable batchFiles = Set.empty + /// Number of currently active batch scopes (supports overlapping batches) + let mutable activeBatchScopes = 0 + + let normalizePath (path: string) = IO.Path.GetFullPath(path) + + let buildMessage () = + let filesPart = + match Set.count activeFiles with + | 0 -> None + | 1 -> activeFiles |> Set.toList |> List.head |> Some + | n -> + let fileList = activeFiles |> Set.toList |> List.truncate 3 |> String.concat ", " + + (if n > 3 then $"{fileList} (+{n - 3} more)" else fileList) |> Some + + let batchPart = + if batchTotal > 0 then + Some $"{batchCompleted}/{batchTotal} completed" + else + None + + match filesPart, batchPart with + | Some files, Some batch -> Some $"{files} ({batch})" + | Some files, None -> Some files + | None, Some batch -> Some batch + | None, None -> None + + let calcPercentage () = + if batchTotal > 0 then + ((float batchCompleted) / (float batchTotal)) * 100.0 |> uint32 |> Some + else + None + + member private x.StartFile(fileName: string) = + cancellableTask { + use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) + let simpleName = IO.Path.GetFileName fileName + activeFiles <- Set.add simpleName activeFiles + + match progressReport with + | None -> + let report = createReport () + progressReport <- Some report + let message = buildMessage () + do! report.Begin(title, ?message = message, ?percentage = calcPercentage ()) + | Some report -> + let message = buildMessage () + do! report.Report(?message = message, ?percentage = calcPercentage ()) + } + + member private x.EndFile(fileName: string) = + cancellableTask { + use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) + let simpleName = IO.Path.GetFileName fileName + activeFiles <- Set.remove simpleName activeFiles + + let normalizedName = normalizePath fileName + + if batchTotal > 0 && Set.contains normalizedName batchFiles then + batchCompleted <- min (batchCompleted + 1) batchTotal + batchFiles <- Set.remove normalizedName batchFiles + + match progressReport with + | Some report when Set.isEmpty activeFiles && activeBatchScopes <= 0 -> + do! report.End() + progressReport <- None + | Some report -> + let message = buildMessage () + do! report.Report(?message = message, ?percentage = calcPercentage ()) + | None -> () + } + + /// Begin tracking a file being typechecked. Returns an IAsyncDisposable that ends tracking on dispose. + member x.Begin(fileName: string) : CancellableTask = + cancellableTask { + do! x.StartFile(fileName) + + return + { new IAsyncDisposable with + member _.DisposeAsync() = x.EndFile (fileName) CancellationToken.None |> ValueTask } + } + + member private x.StartBatch(files: string array) = + cancellableTask { + use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) + activeBatchScopes <- activeBatchScopes + 1 + let normalizedFiles = files |> Array.map normalizePath |> Array.distinct + batchTotal <- batchTotal + normalizedFiles.Length + batchFiles <- Set.union batchFiles (normalizedFiles |> Set.ofArray) + + // Eagerly create the report so the CancellationToken is available for linking + match progressReport with + | None -> + let report = createReport () + progressReport <- Some report + do! report.Begin(title, ?percentage = calcPercentage ()) + | Some report -> do! report.Report(?percentage = calcPercentage ()) + } + + member private x.EndBatch() = + cancellableTask { + use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) + activeBatchScopes <- activeBatchScopes - 1 + + if activeBatchScopes <= 0 then + activeBatchScopes <- 0 + batchTotal <- 0 + batchCompleted <- 0 + batchFiles <- Set.empty + + // End the report if nothing else is active + if Set.isEmpty activeFiles && activeBatchScopes <= 0 then + match progressReport with + | Some report -> + do! report.End() + progressReport <- None + | None -> () + } + + /// Set up a batch of files to be typechecked. Returns an IAsyncDisposable that clears the batch on dispose. + member x.BeginBatch(files: string array) : CancellableTask = + cancellableTask { + do! x.StartBatch(files) + + return + { new IAsyncDisposable with + member _.DisposeAsync() = x.EndBatch () CancellationToken.None |> ValueTask } + } + + /// Gets the cancellation token from the current progress report, or CancellationToken.None if no report is active. + member x.GetCancellationToken() : Task = + task { + use! __ = locker.LockAsync() + + return + match progressReport with + | Some report -> report.CancellationToken + | None -> CancellationToken.None + } + + interface IDisposable with + member x.Dispose() = + progressReport |> Option.iter (fun r -> (r :> IDisposable).Dispose()) + locker.Dispose() + + open System.Diagnostics.Tracing open System.Collections.Concurrent open System.Diagnostics diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi index da3f4ee6e..ff3994d0c 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi @@ -7,6 +7,7 @@ open Ionide.LanguageServerProtocol.JsonRpc open FsAutoComplete.LspHelpers open System open System.Threading +open System.Threading.Tasks open IcedTasks type FSharpLspClient = @@ -79,6 +80,24 @@ type ServerProgressReport = interface IAsyncDisposable interface IDisposable +/// +/// A shared progress reporter that consolidates multiple concurrent typecheck operations +/// into a single LSP progress notification. Instead of creating a new Begin/End cycle per file, +/// this maintains one notification that updates its message with the current file being checked. +/// +type SharedTypecheckProgressReporter = + new: title: string * createReport: (unit -> ServerProgressReport) -> SharedTypecheckProgressReporter + + /// Begin tracking a file being typechecked. Returns an IAsyncDisposable that ends tracking on dispose. + member Begin: fileName: string -> CancellableTask + + /// Set up a batch of files to be typechecked. Returns an IAsyncDisposable that clears the batch on dispose. + member BeginBatch: files: string array -> CancellableTask + + /// Gets the cancellation token from the current progress report, or CancellationToken.None if no report is active. + member GetCancellationToken: unit -> Task + interface IDisposable + /// listener for the the events generated from the fsc ActivitySource type ProgressListener = new: lspClient: FSharpLspClient * traceNamespace: string array -> ProgressListener From d8af2f9b6cd607cc88d332c61b00cacff243838f Mon Sep 17 00:00:00 2001 From: Jimmy Byrd Date: Mon, 16 Feb 2026 23:32:42 -0500 Subject: [PATCH 02/10] Add parallel25 function for 25% processor concurrency and integrate shared progress reporter --- src/FsAutoComplete.Core/Utils.fs | 6 ++ src/FsAutoComplete.Core/Utils.fsi | 5 ++ .../LspServers/AdaptiveServerState.fs | 78 +++++++++---------- .../LspServers/FSharpLspClient.fs | 78 ++++++++++++------- .../LspServers/FSharpLspClient.fsi | 6 +- 5 files changed, 101 insertions(+), 72 deletions(-) diff --git a/src/FsAutoComplete.Core/Utils.fs b/src/FsAutoComplete.Core/Utils.fs index bab105fa5..6785e916a 100644 --- a/src/FsAutoComplete.Core/Utils.fs +++ b/src/FsAutoComplete.Core/Utils.fs @@ -238,6 +238,12 @@ module Async = Async.Parallel(computations, int maxConcurrency) + let parallel25 computations = + let maxConcurrency = + Math.Max(1.0, Math.Floor((float System.Environment.ProcessorCount) * 0.25)) + + Async.Parallel(computations, int maxConcurrency) + [] module Array = /// Async implementation of Array.map. diff --git a/src/FsAutoComplete.Core/Utils.fsi b/src/FsAutoComplete.Core/Utils.fsi index fe18becb8..7ef79b228 100644 --- a/src/FsAutoComplete.Core/Utils.fsi +++ b/src/FsAutoComplete.Core/Utils.fsi @@ -112,6 +112,11 @@ module Async = /// A sequence of distinct computations to be parallelized. val parallel75: computations: seq> -> Async<'a array> + + /// Creates an asynchronous computation that executes all the given asynchronous computations, using 25% of the Environment.ProcessorCount + /// A sequence of distinct computations to be parallelized. + val parallel25: computations: seq> -> Async<'a array> + [] module Array = /// Async implementation of Array.map. diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index e3200f9a2..4667eee4d 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -219,14 +219,26 @@ type AdaptiveState let progressLookup = new ServerProgressLookup() do disposables.Add progressLookup + let config = cval FSharpConfig.Default + let typecheckProgressReporter = new SharedTypecheckProgressReporter( "Typechecking", - fun () -> progressLookup.CreateProgressReport(lspClient, cancellable = true) + (fun () -> progressLookup.CreateProgressReport(lspClient, cancellable = true)), + isEnabled = (fun () -> (AVal.force config).Notifications.BackgroundServiceProgress) ) do disposables.Add typecheckProgressReporter + let analyzerProgressReporter = + new SharedTypecheckProgressReporter( + "Analyzing", + (fun () -> progressLookup.CreateProgressReport(lspClient, cancellable = true)), + isEnabled = (fun () -> (AVal.force config).Notifications.BackgroundServiceProgress) + ) + + do disposables.Add analyzerProgressReporter + let serilogLoggerFactory = new SerilogLoggerFactory(Serilog.Log.Logger) do disposables.Add serilogLoggerFactory @@ -234,8 +246,6 @@ type AdaptiveState let rootPath = cval None - let config = cval FSharpConfig.Default - let checker = config |> AVal.map (fun c -> @@ -544,14 +554,12 @@ type AdaptiveState try let! ct = Async.CancellationToken use! _l = analyzersLocker.LockAsync(ct) - use progress = progressLookup.CreateProgressReport(lspClient, cancellable = true) - - if config.Notifications.BackgroundServiceProgress then - do! progress.Begin($"Checking unused opens {fileName}...", message = filePathUntag) + use! _progress = analyzerProgressReporter.Begin($"Unused opens - {fileName}") + let! progressCt = analyzerProgressReporter.GetCancellationToken() let! unused = UnusedOpens.getUnusedOpens (tyRes.GetCheckResults, getSourceLine) - |> Async.withCancellation progress.CancellationToken + |> Async.withCancellation progressCt do! triggerNotificationAndWait @@ -566,16 +574,14 @@ type AdaptiveState try let! ct = Async.CancellationToken use! _l = analyzersLocker.LockAsync(ct) - use progress = progressLookup.CreateProgressReport(lspClient, cancellable = true) - - if config.Notifications.BackgroundServiceProgress then - do! progress.Begin($"Checking unused declarations {fileName}...", message = filePathUntag) + use! _progress = analyzerProgressReporter.Begin($"Unused declarations - {fileName}") + let! progressCt = analyzerProgressReporter.GetCancellationToken() let isScript = Utils.isAScript (filePathUntag) let! unused = UnusedDeclarations.getUnusedDeclarations (tyRes.GetCheckResults, isScript) - |> Async.withCancellation progress.CancellationToken + |> Async.withCancellation progressCt let unused = unused |> Seq.toArray @@ -589,14 +595,12 @@ type AdaptiveState try let! ct = Async.CancellationToken use! _l = analyzersLocker.LockAsync(ct) - use progress = progressLookup.CreateProgressReport(lspClient, cancellable = true) - - if config.Notifications.BackgroundServiceProgress then - do! progress.Begin($"Checking simplifying of names {fileName}...", message = filePathUntag) + use! _progress = analyzerProgressReporter.Begin($"Simplify names - {fileName}") + let! progressCt = analyzerProgressReporter.GetCancellationToken() let! simplified = SimplifyNames.getSimplifiableNames (tyRes.GetCheckResults, getSourceLine) - |> Async.withCancellation progress.CancellationToken + |> Async.withCancellation progressCt let simplified = Array.ofSeq simplified do! triggerNotificationAndWait (NotificationEvent.SimplifyNames(filePath, simplified, file.Version)) ct @@ -609,10 +613,7 @@ type AdaptiveState try let! ct = Async.CancellationToken use! _l = analyzersLocker.LockAsync(ct) - use progress = progressLookup.CreateProgressReport(lspClient) - - if config.Notifications.BackgroundServiceProgress then - do! progress.Begin($"Checking for unnecessary parentheses {fileName}...", message = filePathUntag) + use! _progress = analyzerProgressReporter.Begin($"Unnecessary parens - {fileName}") let unnecessaryParentheses = (System.Collections.Generic.HashSet(comparer = Range.comparer), tyRes.GetAST) @@ -686,10 +687,8 @@ type AdaptiveState let file = volatileFile.FileName try - use progress = new ServerProgressReport(lspClient) - - if config.Notifications.BackgroundServiceProgress then - do! progress.Begin("Running analyzers...", message = UMX.untag file) + let fileName = Path.GetFileName(UMX.untag file) + use! _progress = analyzerProgressReporter.Begin($"External analyzers - {fileName}") Loggers.analyzers.info ( Log.setMessage "begin analysis of {file}" @@ -2749,7 +2748,7 @@ type AdaptiveState let checksToPerform = innerChecks |> Array.map (fun (proj, file) -> - async { + asyncEx { use joinedToken = if file = sourceFilePath then @@ -2763,25 +2762,20 @@ type AdaptiveState CancellationTokenSource.CreateLinkedTokenSource(rootToken, fileToken, progressCt) // Track per-file progress so the shared reporter updates message and batch counter - let! fileProgress = - typecheckProgressReporter.Begin (UMX.untag file) CancellationToken.None - |> Async.AwaitTask + use! _fileProgress = typecheckProgressReporter.Begin(UMX.untag file) try - try - let! _ = - bypassAdaptiveTypeCheck (file) (proj) (AVal.force proj.FSharpProjectCompilerOptions) - |> Async.withCancellation joinedToken.Token - - () - with :? OperationCanceledException -> - // if a file shows up multiple times in the list such as Microsoft.NET.Test.Sdk.Program.fs we may cancel it but we don't want to stop the whole operation for it - () - finally - fileProgress.DisposeAsync().AsTask().GetAwaiter().GetResult() + let! _ = + bypassAdaptiveTypeCheck (file) (proj) (AVal.force proj.FSharpProjectCompilerOptions) + |> Async.withCancellation joinedToken.Token + + () + with :? OperationCanceledException -> + // if a file shows up multiple times in the list such as Microsoft.NET.Test.Sdk.Program.fs we may cancel it but we don't want to stop the whole operation for it + () }) - do! checksToPerform |> Async.parallel75 |> Async.Ignore + do! checksToPerform |> Async.parallel25 |> Async.Ignore } diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fs b/src/FsAutoComplete/LspServers/FSharpLspClient.fs index 18fd9c6e3..a5259d527 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fs +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fs @@ -193,9 +193,11 @@ type ServerProgressReport(lspClient: FSharpLspClient, ?token: ProgressToken, ?ca /// this maintains one notification that updates its message with the current file being checked. /// type SharedTypecheckProgressReporter - (title: string, createReport: unit -> ServerProgressReport) = + (title: string, createReport: unit -> ServerProgressReport, ?isEnabled: unit -> bool) + = let locker = new SemaphoreSlim(1, 1) + let isEnabled = defaultArg isEnabled (fun () -> true) /// Display-only set of short file names currently being checked let mutable activeFiles = Set.empty let mutable progressReport: ServerProgressReport option = None @@ -238,19 +240,24 @@ type SharedTypecheckProgressReporter member private x.StartFile(fileName: string) = cancellableTask { - use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) - let simpleName = IO.Path.GetFileName fileName - activeFiles <- Set.add simpleName activeFiles + if isEnabled () then + use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) + let simpleName = IO.Path.GetFileName fileName + activeFiles <- Set.add simpleName activeFiles - match progressReport with - | None -> - let report = createReport () - progressReport <- Some report - let message = buildMessage () - do! report.Begin(title, ?message = message, ?percentage = calcPercentage ()) - | Some report -> - let message = buildMessage () - do! report.Report(?message = message, ?percentage = calcPercentage ()) + match progressReport with + | None -> + let report = createReport () + progressReport <- Some report + let message = buildMessage () + do! report.Begin(title, ?message = message, ?percentage = calcPercentage ()) + | Some report -> + let message = buildMessage () + do! report.Report(?message = message, ?percentage = calcPercentage ()) + + return true + else + return false } member private x.EndFile(fileName: string) = @@ -278,28 +285,37 @@ type SharedTypecheckProgressReporter /// Begin tracking a file being typechecked. Returns an IAsyncDisposable that ends tracking on dispose. member x.Begin(fileName: string) : CancellableTask = cancellableTask { - do! x.StartFile(fileName) + let! started = x.StartFile(fileName) return { new IAsyncDisposable with - member _.DisposeAsync() = x.EndFile (fileName) CancellationToken.None |> ValueTask } + member _.DisposeAsync() = + if started then + x.EndFile (fileName) CancellationToken.None |> ValueTask + else + ValueTask() } } member private x.StartBatch(files: string array) = cancellableTask { - use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) - activeBatchScopes <- activeBatchScopes + 1 - let normalizedFiles = files |> Array.map normalizePath |> Array.distinct - batchTotal <- batchTotal + normalizedFiles.Length - batchFiles <- Set.union batchFiles (normalizedFiles |> Set.ofArray) + if isEnabled () then + use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) + activeBatchScopes <- activeBatchScopes + 1 + let normalizedFiles = files |> Array.map normalizePath |> Array.distinct + batchTotal <- batchTotal + normalizedFiles.Length + batchFiles <- Set.union batchFiles (normalizedFiles |> Set.ofArray) + + // Eagerly create the report so the CancellationToken is available for linking + match progressReport with + | None -> + let report = createReport () + progressReport <- Some report + do! report.Begin(title, ?percentage = calcPercentage ()) + | Some report -> do! report.Report(?percentage = calcPercentage ()) - // Eagerly create the report so the CancellationToken is available for linking - match progressReport with - | None -> - let report = createReport () - progressReport <- Some report - do! report.Begin(title, ?percentage = calcPercentage ()) - | Some report -> do! report.Report(?percentage = calcPercentage ()) + return true + else + return false } member private x.EndBatch() = @@ -325,11 +341,15 @@ type SharedTypecheckProgressReporter /// Set up a batch of files to be typechecked. Returns an IAsyncDisposable that clears the batch on dispose. member x.BeginBatch(files: string array) : CancellableTask = cancellableTask { - do! x.StartBatch(files) + let! started = x.StartBatch(files) return { new IAsyncDisposable with - member _.DisposeAsync() = x.EndBatch () CancellationToken.None |> ValueTask } + member _.DisposeAsync() = + if started then + x.EndBatch () CancellationToken.None |> ValueTask + else + ValueTask() } } /// Gets the cancellation token from the current progress report, or CancellationToken.None if no report is active. diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi index ff3994d0c..f440f5169 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi @@ -86,7 +86,11 @@ type ServerProgressReport = /// this maintains one notification that updates its message with the current file being checked. /// type SharedTypecheckProgressReporter = - new: title: string * createReport: (unit -> ServerProgressReport) -> SharedTypecheckProgressReporter + new: + title: string * + createReport: (unit -> ServerProgressReport) * + ?isEnabled: (unit -> bool) -> + SharedTypecheckProgressReporter /// Begin tracking a file being typechecked. Returns an IAsyncDisposable that ends tracking on dispose. member Begin: fileName: string -> CancellableTask From 28805bc16a0e0391b97f28c228df673f45f90157 Mon Sep 17 00:00:00 2001 From: Jimmy Byrd Date: Mon, 16 Feb 2026 23:51:05 -0500 Subject: [PATCH 03/10] Add synchronous batch progress reporting for project loading --- .../LspServers/AdaptiveServerState.fs | 14 ++++++++++---- src/FsAutoComplete/LspServers/FSharpLspClient.fs | 16 ++++++++++++++++ .../LspServers/FSharpLspClient.fsi | 5 +++++ 3 files changed, 31 insertions(+), 4 deletions(-) diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index 4667eee4d..3ac31b691 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -239,6 +239,11 @@ type AdaptiveState do disposables.Add analyzerProgressReporter + let projectLoadingProgressReporter = + new SharedTypecheckProgressReporter("Loading Projects", fun () -> progressLookup.CreateProgressReport(lspClient)) + + do disposables.Add projectLoadingProgressReporter + let serilogLoggerFactory = new SerilogLoggerFactory(Serilog.Log.Logger) do disposables.Add serilogLoggerFactory @@ -1119,13 +1124,12 @@ type AdaptiveState triggerNotification not CancellationToken.None) - use progressReport = new ServerProgressReport(lspClient) + let projectFiles = projects |> Seq.map (fst >> UMX.untag) |> Seq.toArray - progressReport.Begin ($"Loading {projects.Count} Projects") (CancellationToken.None) - |> ignore> + use _progressReport = projectLoadingProgressReporter.BeginBatchSync(projectFiles) let projectOptions = - loader.LoadProjects(projects |> Seq.map (fst >> UMX.untag) |> Seq.toList, [], binlogConfig) + loader.LoadProjects(projectFiles |> Array.toList, [], binlogConfig) |> Seq.toList for p in projectOptions do @@ -1959,6 +1963,7 @@ type AdaptiveState let fileNameStr = UMX.untag file.Source.FileName use! _fileProgress = typecheckProgressReporter.Begin(fileNameStr) + let! progressCt = typecheckProgressReporter.GetCancellationToken() let! result = match compilerOptions with @@ -1972,6 +1977,7 @@ type AdaptiveState opts, shouldCache = shouldCache ) + |> Async.withCancellation progressCt let! ct = Async.CancellationToken diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fs b/src/FsAutoComplete/LspServers/FSharpLspClient.fs index a5259d527..e23673eea 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fs +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fs @@ -363,6 +363,22 @@ type SharedTypecheckProgressReporter | None -> CancellationToken.None } + /// Set up a batch of files synchronously (for contexts where CancellableTask is not available). + /// Sets up batch tracking and adds all files to activeFiles so their names appear in the message. + /// Returns an IDisposable that ends all file tracking and the batch on dispose. + member x.BeginBatchSync(files: string array) : IDisposable = + x.StartBatch (files) CancellationToken.None |> ignore> + + for file in files do + x.StartFile (file) CancellationToken.None |> ignore> + + { new IDisposable with + member _.Dispose() = + for file in files do + x.EndFile (file) CancellationToken.None |> ignore> + + x.EndBatch () CancellationToken.None |> ignore> } + interface IDisposable with member x.Dispose() = progressReport |> Option.iter (fun r -> (r :> IDisposable).Dispose()) diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi index f440f5169..3abac1a05 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi @@ -100,6 +100,11 @@ type SharedTypecheckProgressReporter = /// Gets the cancellation token from the current progress report, or CancellationToken.None if no report is active. member GetCancellationToken: unit -> Task + + /// Set up a batch of files synchronously (for contexts where CancellableTask is not available). + /// Sets up batch tracking and adds all files to activeFiles so their names appear in the message. + /// Returns an IDisposable that ends all file tracking and the batch on dispose. + member BeginBatchSync: files: string array -> IDisposable interface IDisposable /// listener for the the events generated from the fsc ActivitySource From 687d836ee8dacd07eadd64ef10f3850109c1c6f3 Mon Sep 17 00:00:00 2001 From: Jimmy Byrd Date: Tue, 17 Feb 2026 00:00:51 -0500 Subject: [PATCH 04/10] Fix indentation for Async.withCancellation in AdaptiveState --- src/FsAutoComplete/LspServers/AdaptiveServerState.fs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index 3ac31b691..a2cca39a3 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -1977,7 +1977,7 @@ type AdaptiveState opts, shouldCache = shouldCache ) - |> Async.withCancellation progressCt + |> Async.withCancellation progressCt let! ct = Async.CancellationToken From acff5b8211c2beed8ebcf0421c072b10ecc3d03f Mon Sep 17 00:00:00 2001 From: Jimmy Byrd Date: Tue, 17 Feb 2026 10:20:19 -0500 Subject: [PATCH 05/10] Refactor parallel computation functions and enhance progress reporting in typechecking --- src/FsAutoComplete.Core/Utils.fs | 15 +- src/FsAutoComplete.Core/Utils.fsi | 3 + .../LspServers/AdaptiveServerState.fs | 4 +- .../LspServers/FSharpLspClient.fs | 87 ++- .../LspServers/FSharpLspClient.fsi | 4 +- test/FsAutoComplete.Tests.Lsp/Program.fs | 2 + .../SharedTypecheckProgressReporterTests.fs | 681 ++++++++++++++++++ 7 files changed, 739 insertions(+), 57 deletions(-) create mode 100644 test/FsAutoComplete.Tests.Lsp/SharedTypecheckProgressReporterTests.fs diff --git a/src/FsAutoComplete.Core/Utils.fs b/src/FsAutoComplete.Core/Utils.fs index 6785e916a..688b756c0 100644 --- a/src/FsAutoComplete.Core/Utils.fs +++ b/src/FsAutoComplete.Core/Utils.fs @@ -230,20 +230,17 @@ module Async = // Start the workflow using a provided cancellation token Async.StartWithContinuations(work, cont, econt, ccont, cancellationToken = cancellationToken)) - /// Creates an asynchronous computation that executes all the given asynchronous computations, using 75% of the Environment.ProcessorCount - /// A sequence of distinct computations to be parallelized. - let parallel75 computations = - let maxConcurrency = - Math.Max(1.0, Math.Floor((float System.Environment.ProcessorCount) * 0.75)) - - Async.Parallel(computations, int maxConcurrency) - let parallel25 computations = + let private parallelOfPercentage percentage computations = let maxConcurrency = - Math.Max(1.0, Math.Floor((float System.Environment.ProcessorCount) * 0.25)) + Math.Max(1.0, Math.Floor((float System.Environment.ProcessorCount) * percentage)) Async.Parallel(computations, int maxConcurrency) + let parallel75 computations = parallelOfPercentage 0.75 computations + let parallel50 computations = parallelOfPercentage 0.50 computations + let parallel25 computations = parallelOfPercentage 0.25 computations + [] module Array = /// Async implementation of Array.map. diff --git a/src/FsAutoComplete.Core/Utils.fsi b/src/FsAutoComplete.Core/Utils.fsi index 7ef79b228..f8a85e0e2 100644 --- a/src/FsAutoComplete.Core/Utils.fsi +++ b/src/FsAutoComplete.Core/Utils.fsi @@ -112,6 +112,9 @@ module Async = /// A sequence of distinct computations to be parallelized. val parallel75: computations: seq> -> Async<'a array> + /// Creates an asynchronous computation that executes all the given asynchronous computations, using 50% of the Environment.ProcessorCount + /// A sequence of distinct computations to be parallelized. + val parallel50: computations: seq> -> Async<'a array> /// Creates an asynchronous computation that executes all the given asynchronous computations, using 25% of the Environment.ProcessorCount /// A sequence of distinct computations to be parallelized. diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index a2cca39a3..162128250 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -694,6 +694,7 @@ type AdaptiveState try let fileName = Path.GetFileName(UMX.untag file) use! _progress = analyzerProgressReporter.Begin($"External analyzers - {fileName}") + let! progressCt = analyzerProgressReporter.GetCancellationToken() Loggers.analyzers.info ( Log.setMessage "begin analysis of {file}" @@ -743,6 +744,7 @@ type AdaptiveState analyzerOptions, analyzerPredicate ) + |> Async.withCancellation progressCt let! ct = Async.CancellationToken do! triggerNotificationAndWait (NotificationEvent.AnalyzerMessage(res, file, volatileFile.Version)) ct @@ -1977,7 +1979,7 @@ type AdaptiveState opts, shouldCache = shouldCache ) - |> Async.withCancellation progressCt + |> Async.withCancellation progressCt let! ct = Async.CancellationToken diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fs b/src/FsAutoComplete/LspServers/FSharpLspClient.fs index e23673eea..9fe2d37ed 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fs +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fs @@ -193,8 +193,7 @@ type ServerProgressReport(lspClient: FSharpLspClient, ?token: ProgressToken, ?ca /// this maintains one notification that updates its message with the current file being checked. /// type SharedTypecheckProgressReporter - (title: string, createReport: unit -> ServerProgressReport, ?isEnabled: unit -> bool) - = + (title: string, createReport: unit -> ServerProgressReport, ?isEnabled: unit -> bool) = let locker = new SemaphoreSlim(1, 1) let isEnabled = defaultArg isEnabled (fun () -> true) @@ -240,24 +239,19 @@ type SharedTypecheckProgressReporter member private x.StartFile(fileName: string) = cancellableTask { - if isEnabled () then - use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) - let simpleName = IO.Path.GetFileName fileName - activeFiles <- Set.add simpleName activeFiles - - match progressReport with - | None -> - let report = createReport () - progressReport <- Some report - let message = buildMessage () - do! report.Begin(title, ?message = message, ?percentage = calcPercentage ()) - | Some report -> - let message = buildMessage () - do! report.Report(?message = message, ?percentage = calcPercentage ()) + use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) + let simpleName = IO.Path.GetFileName fileName + activeFiles <- Set.add simpleName activeFiles - return true - else - return false + match progressReport with + | None -> + let report = createReport () + progressReport <- Some report + let message = buildMessage () + do! report.Begin(title, ?message = message, ?percentage = calcPercentage ()) + | Some report -> + let message = buildMessage () + do! report.Report(?message = message, ?percentage = calcPercentage ()) } member private x.EndFile(fileName: string) = @@ -285,7 +279,10 @@ type SharedTypecheckProgressReporter /// Begin tracking a file being typechecked. Returns an IAsyncDisposable that ends tracking on dispose. member x.Begin(fileName: string) : CancellableTask = cancellableTask { - let! started = x.StartFile(fileName) + let started = isEnabled () + + if started then + do! x.StartFile(fileName) return { new IAsyncDisposable with @@ -298,24 +295,19 @@ type SharedTypecheckProgressReporter member private x.StartBatch(files: string array) = cancellableTask { - if isEnabled () then - use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) - activeBatchScopes <- activeBatchScopes + 1 - let normalizedFiles = files |> Array.map normalizePath |> Array.distinct - batchTotal <- batchTotal + normalizedFiles.Length - batchFiles <- Set.union batchFiles (normalizedFiles |> Set.ofArray) - - // Eagerly create the report so the CancellationToken is available for linking - match progressReport with - | None -> - let report = createReport () - progressReport <- Some report - do! report.Begin(title, ?percentage = calcPercentage ()) - | Some report -> do! report.Report(?percentage = calcPercentage ()) + use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) + activeBatchScopes <- activeBatchScopes + 1 + let normalizedFiles = files |> Array.map normalizePath |> Array.distinct + batchTotal <- batchTotal + normalizedFiles.Length + batchFiles <- Set.union batchFiles (normalizedFiles |> Set.ofArray) - return true - else - return false + // Eagerly create the report so the CancellationToken is available for linking + match progressReport with + | None -> + let report = createReport () + progressReport <- Some report + do! report.Begin(title, ?percentage = calcPercentage ()) + | Some report -> do! report.Report(?percentage = calcPercentage ()) } member private x.EndBatch() = @@ -341,7 +333,10 @@ type SharedTypecheckProgressReporter /// Set up a batch of files to be typechecked. Returns an IAsyncDisposable that clears the batch on dispose. member x.BeginBatch(files: string array) : CancellableTask = cancellableTask { - let! started = x.StartBatch(files) + let started = isEnabled () + + if started then + do! x.StartBatch(files) return { new IAsyncDisposable with @@ -367,17 +362,21 @@ type SharedTypecheckProgressReporter /// Sets up batch tracking and adds all files to activeFiles so their names appear in the message. /// Returns an IDisposable that ends all file tracking and the batch on dispose. member x.BeginBatchSync(files: string array) : IDisposable = - x.StartBatch (files) CancellationToken.None |> ignore> + let started = isEnabled () + + if started then + x.StartBatch (files) CancellationToken.None |> ignore> - for file in files do - x.StartFile (file) CancellationToken.None |> ignore> + for file in files do + x.StartFile (file) CancellationToken.None |> ignore> { new IDisposable with member _.Dispose() = - for file in files do - x.EndFile (file) CancellationToken.None |> ignore> + if started then + for file in files do + x.EndFile (file) CancellationToken.None |> ignore> - x.EndBatch () CancellationToken.None |> ignore> } + x.EndBatch () CancellationToken.None |> ignore> } interface IDisposable with member x.Dispose() = diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi index 3abac1a05..f4fb9f41c 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi @@ -87,9 +87,7 @@ type ServerProgressReport = /// type SharedTypecheckProgressReporter = new: - title: string * - createReport: (unit -> ServerProgressReport) * - ?isEnabled: (unit -> bool) -> + title: string * createReport: (unit -> ServerProgressReport) * ?isEnabled: (unit -> bool) -> SharedTypecheckProgressReporter /// Begin tracking a file being typechecked. Returns an IAsyncDisposable that ends tracking on dispose. diff --git a/test/FsAutoComplete.Tests.Lsp/Program.fs b/test/FsAutoComplete.Tests.Lsp/Program.fs index 7d49a8e83..b836a529b 100644 --- a/test/FsAutoComplete.Tests.Lsp/Program.fs +++ b/test/FsAutoComplete.Tests.Lsp/Program.fs @@ -13,6 +13,7 @@ open FsAutoComplete.Tests.ExtensionsTests open FsAutoComplete.Tests.InteractiveDirectivesTests open FsAutoComplete.Tests.Lsp.CoreUtilsTests open FsAutoComplete.Tests.Lsp.DecompilerTests +open FsAutoComplete.Tests.SharedTypecheckProgressReporterTests open FsAutoComplete.Tests.CallHierarchy open Ionide.ProjInfo open System.Threading @@ -374,6 +375,7 @@ let generalTests = testCase "test host uses target runtime" (fun _ -> Expect.equal Environment.Version.Major expectedRuntimeMajor "Test host runtime must match the target framework") testList (nameof (Utils)) [ Utils.Tests.Utils.tests; Utils.Tests.TextEdit.tests ] + SharedTypecheckProgressReporterTests.tests InlayHintTests.explicitTypeInfoTests sourceTextFactory FindReferences.tryFixupRangeTests sourceTextFactory UtilsTests.allTests diff --git a/test/FsAutoComplete.Tests.Lsp/SharedTypecheckProgressReporterTests.fs b/test/FsAutoComplete.Tests.Lsp/SharedTypecheckProgressReporterTests.fs new file mode 100644 index 000000000..2c6fded00 --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/SharedTypecheckProgressReporterTests.fs @@ -0,0 +1,681 @@ +module FsAutoComplete.Tests.SharedTypecheckProgressReporterTests + +open Expecto +open System +open System.Threading +open System.Threading.Tasks +open System.Collections.Concurrent +open FsAutoComplete.Lsp +open Ionide.LanguageServerProtocol +open Ionide.LanguageServerProtocol.Server +open Ionide.LanguageServerProtocol.Types +open Ionide.LanguageServerProtocol.JsonRpc + +/// Captured progress call from the mock client +type ProgressCall = + | Begin of title: string * message: string option * percentage: uint32 option + | Report of message: string option * percentage: uint32 option + | End of message: string option + +/// Creates a mock FSharpLspClient that captures all progress notifications +let private createMockClient () = + let progressCalls = ConcurrentQueue() + + let notificationSender: ClientNotificationSender = + fun methodName payload -> + match methodName with + | "$/progress" -> + let p = payload :?> ProgressParams + // Parse the progress value to determine the call type + let json = p.Value.ToString() + + if json.Contains("\"kind\":\"begin\"") || json.Contains("\"kind\": \"begin\"") then + // Extract title from JSON + let titleStart = json.IndexOf("\"title\"") + "\"title\"".Length + let colonAfterTitle = json.IndexOf(":", titleStart) + 1 + let titleQuoteStart = json.IndexOf("\"", colonAfterTitle) + 1 + let titleQuoteEnd = json.IndexOf("\"", titleQuoteStart) + let title = json.Substring(titleQuoteStart, titleQuoteEnd - titleQuoteStart) + + let message = + if json.Contains("\"message\"") then + let msgStart = json.IndexOf("\"message\"") + "\"message\"".Length + let colonAfterMsg = json.IndexOf(":", msgStart) + 1 + let msgQuoteStart = json.IndexOf("\"", colonAfterMsg) + 1 + let msgQuoteEnd = json.IndexOf("\"", msgQuoteStart) + Some(json.Substring(msgQuoteStart, msgQuoteEnd - msgQuoteStart)) + else + None + + let percentage = + if json.Contains("\"percentage\"") then + let pctStart = json.IndexOf("\"percentage\"") + "\"percentage\"".Length + let colonAfterPct = json.IndexOf(":", pctStart) + 1 + // Find the number value after the colon + let numStr = + json.Substring(colonAfterPct).TrimStart() + |> fun s -> + let endIdx = s.IndexOfAny([| ','; '}'; ' ' |]) + + if endIdx > 0 then s.Substring(0, endIdx) else s + + match UInt32.TryParse(numStr) with + | true, v -> Some v + | _ -> None + else + None + + progressCalls.Enqueue(Begin(title, message, percentage)) + elif json.Contains("\"kind\":\"report\"") || json.Contains("\"kind\": \"report\"") then + let message = + if json.Contains("\"message\"") then + let msgStart = json.IndexOf("\"message\"") + "\"message\"".Length + let colonAfterMsg = json.IndexOf(":", msgStart) + 1 + let msgQuoteStart = json.IndexOf("\"", colonAfterMsg) + 1 + let msgQuoteEnd = json.IndexOf("\"", msgQuoteStart) + Some(json.Substring(msgQuoteStart, msgQuoteEnd - msgQuoteStart)) + else + None + + let percentage = + if json.Contains("\"percentage\"") then + let pctStart = json.IndexOf("\"percentage\"") + "\"percentage\"".Length + let colonAfterPct = json.IndexOf(":", pctStart) + 1 + + let numStr = + json.Substring(colonAfterPct).TrimStart() + |> fun s -> + let endIdx = s.IndexOfAny([| ','; '}'; ' ' |]) + + if endIdx > 0 then s.Substring(0, endIdx) else s + + match UInt32.TryParse(numStr) with + | true, v -> Some v + | _ -> None + else + None + + progressCalls.Enqueue(Report(message, percentage)) + elif json.Contains("\"kind\":\"end\"") || json.Contains("\"kind\": \"end\"") then + let message = + if json.Contains("\"message\"") then + let msgStart = json.IndexOf("\"message\"") + "\"message\"".Length + let colonAfterMsg = json.IndexOf(":", msgStart) + 1 + let msgQuoteStart = json.IndexOf("\"", colonAfterMsg) + 1 + let msgQuoteEnd = json.IndexOf("\"", msgQuoteStart) + Some(json.Substring(msgQuoteStart, msgQuoteEnd - msgQuoteStart)) + else + None + + progressCalls.Enqueue(End message) + | _ -> () + + AsyncLspResult.success () + + let requestSender = + { new ClientRequestSender with + member _.Send _name _payload = AsyncLspResult.success Unchecked.defaultof<_> } + + let client = new FSharpLspClient(notificationSender, requestSender) + + client.ClientCapabilities <- + Some + { Workspace = None + TextDocument = None + Experimental = None + Window = + Some + { WorkDoneProgress = Some true + ShowMessage = None + ShowDocument = None } + General = None + NotebookDocument = None } + + client, progressCalls + +/// Helper: create a reporter with a mock client, returning (reporter, capturedCalls, reportCount) +let private createReporter (title: string) = + let client, calls = createMockClient () + let reportCount = ref 0 + + let reporter = + new SharedTypecheckProgressReporter( + title, + fun () -> + Interlocked.Increment(reportCount) |> ignore + new ServerProgressReport(client) + ) + + reporter, calls, reportCount + +/// Wait briefly for fire-and-forget tasks to settle +let private settle () = Task.Delay(50) |> Async.AwaitTask + + +let singleFileTests = + testList + "Single File" + [ testCaseAsync "Begin creates report and End disposes it" + <| async { + let reporter, calls, reportCount = createReporter "Typechecking" + + use reporter = reporter + + let! disp = reporter.Begin ("C:/src/Foo.fs") CancellationToken.None |> Async.AwaitTask + + do! settle () + + Expect.equal reportCount.Value 1 "Should create one report" + Expect.isGreaterThanOrEqual (calls.Count) 1 "Should have at least a Begin call" + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.isFalse (ct = CancellationToken.None) "CancellationToken should be active during file tracking" + + do! disp.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.isTrue (ct = CancellationToken.None) "CancellationToken should be None after all files end" + } + + testCaseAsync "Multiple sequential files reuse the same report cycle" + <| async { + let reporter, _calls, reportCount = createReporter "Typechecking" + + use reporter = reporter + + // File 1 + let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + do! settle () + do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + Expect.equal reportCount.Value 1 "First file creates one report" + + // File 2 — new report cycle + let! disp2 = reporter.Begin ("C:/src/B.fs") CancellationToken.None |> Async.AwaitTask + do! settle () + do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + Expect.equal reportCount.Value 2 "Second file after first ended creates new report" + } + + testCaseAsync "Concurrent files keep report alive until all end" + <| async { + let reporter, _calls, reportCount = createReporter "Typechecking" + + use reporter = reporter + + let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + let! disp2 = reporter.Begin ("C:/src/B.fs") CancellationToken.None |> Async.AwaitTask + do! settle () + + Expect.equal reportCount.Value 1 "Concurrent files share one report" + + // End first file — report should still be active + do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.isFalse (ct = CancellationToken.None) "Report should still be active with one file remaining" + + // End second file — report should end + do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.isTrue (ct = CancellationToken.None) "Report should end after all files complete" + } ] + + +let batchTests = + testList + "Batch" + [ testCaseAsync "BeginBatch creates report eagerly for CancellationToken access" + <| async { + let reporter, _calls, reportCount = createReporter "Typechecking" + + use reporter = reporter + + let! batchDisp = + reporter.BeginBatch ([| "C:/src/A.fs"; "C:/src/B.fs" |]) CancellationToken.None + |> Async.AwaitTask + + do! settle () + + Expect.equal reportCount.Value 1 "BeginBatch should eagerly create a report" + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.isFalse (ct = CancellationToken.None) "CancellationToken should be available after BeginBatch" + + do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + } + + testCaseAsync "Batch tracks completion percentage for batch files only" + <| async { + let reporter, calls, _reportCount = createReporter "Typechecking" + + use reporter = reporter + + let! batchDisp = + reporter.BeginBatch ([| "C:/src/A.fs"; "C:/src/B.fs"; "C:/src/C.fs" |]) CancellationToken.None + |> Async.AwaitTask + + do! settle () + + // Process batch file A + let! dispA = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + do! settle () + do! dispA.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + // After A completes: 1/3 = 33% + let lastReport = + calls.ToArray() + |> Array.tryFindBack (fun c -> + match c with + | Report(_, Some _) -> true + | _ -> false) + + match lastReport with + | Some(Report(_, Some pct)) -> Expect.equal pct 33u "After 1/3 batch files complete, percentage should be 33%" + | _ -> failtest "Expected a Report call with percentage after first batch file completes" + + // Process non-batch file — should NOT affect batch percentage + let! dispX = reporter.Begin ("C:/src/X.fs") CancellationToken.None |> Async.AwaitTask + do! settle () + do! dispX.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + // Process batch file B + let! dispB = reporter.Begin ("C:/src/B.fs") CancellationToken.None |> Async.AwaitTask + do! settle () + do! dispB.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + // After B completes: 2/3 = 66% + let lastReport2 = + calls.ToArray() + |> Array.tryFindBack (fun c -> + match c with + | Report(_, Some pct) when pct >= 60u -> true + | _ -> false) + + match lastReport2 with + | Some(Report(_, Some pct)) -> Expect.equal pct 66u "After 2/3 batch files complete, percentage should be 66%" + | _ -> failtest "Expected a Report call with ~66% after second batch file completes" + + do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + } + + testCaseAsync "Batch with active file scopes keeps report alive" + <| async { + let reporter, _calls, _reportCount = createReporter "Typechecking" + + use reporter = reporter + + let! batchDisp = + reporter.BeginBatch ([| "C:/src/A.fs" |]) CancellationToken.None + |> Async.AwaitTask + + let! fileDisp = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + + do! settle () + + // End batch scope but file is still active + do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.isFalse (ct = CancellationToken.None) "Report should remain active while file scope is open" + + // End file scope + do! fileDisp.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.isTrue (ct = CancellationToken.None) "Report should end after file and batch both complete" + } + + testCaseAsync "EndBatch resets batch counters" + <| async { + let reporter, calls, _reportCount = createReporter "Typechecking" + + use reporter = reporter + + // First batch + let! batchDisp1 = + reporter.BeginBatch ([| "C:/src/A.fs"; "C:/src/B.fs" |]) CancellationToken.None + |> Async.AwaitTask + + let! dispA = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + do! settle () + do! dispA.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + do! batchDisp1.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + // Second batch — counters should have been reset + let! batchDisp2 = + reporter.BeginBatch ([| "C:/src/C.fs"; "C:/src/D.fs" |]) CancellationToken.None + |> Async.AwaitTask + + do! settle () + + // The Begin for second batch should show 0% (0/2) + let callsArr = calls.ToArray() + + let hasZeroPercentage = + callsArr + |> Array.exists (fun c -> + match c with + | Begin(_, _, Some 0u) -> true + | Report(_, Some 0u) -> true + | _ -> false) + + Expect.isTrue hasZeroPercentage "Second batch should start at 0% after first batch ended" + + do! batchDisp2.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + } ] + + +let batchSyncTests = + testList + "BeginBatchSync" + [ testCaseAsync "BeginBatchSync adds files to active tracking and sets up batch" + <| async { + let reporter, calls, reportCount = createReporter "Loading Projects" + use reporter = reporter + + let disp = + reporter.BeginBatchSync([| "C:/src/Proj1.fsproj"; "C:/src/Proj2.fsproj"; "C:/src/Proj3.fsproj" |]) + + // Give fire-and-forget tasks time to complete + do! settle () + do! settle () + + Expect.equal reportCount.Value 1 "Should create one report" + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.isFalse (ct = CancellationToken.None) "Report should be active after BeginBatchSync" + + // Check that a Begin call was made with the title + let callsArr = calls.ToArray() + + let hasBegin = + callsArr + |> Array.exists (fun c -> + match c with + | Begin("Loading Projects", _, _) -> true + | _ -> false) + + Expect.isTrue hasBegin "Should have a Begin call with the title" + + // Check that file names appear in report messages + let hasFileNames = + callsArr + |> Array.exists (fun c -> + match c with + | Report(Some msg, _) -> + msg.Contains("Proj1.fsproj") + || msg.Contains("Proj2.fsproj") + || msg.Contains("Proj3.fsproj") + | _ -> false) + + Expect.isTrue hasFileNames "Report messages should contain project file names" + + disp.Dispose() + do! settle () + do! settle () + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.isTrue (ct = CancellationToken.None) "Report should end after Dispose" + } + + testCaseAsync "BeginBatchSync Dispose ends all file tracking and batch" + <| async { + let reporter, calls, _reportCount = createReporter "Loading Projects" + use reporter = reporter + + let disp = reporter.BeginBatchSync([| "C:/src/A.fsproj"; "C:/src/B.fsproj" |]) + do! settle () + do! settle () + + disp.Dispose() + do! settle () + do! settle () + + // After dispose, an End call should have been made + let callsArr = calls.ToArray() + + let hasEnd = + callsArr + |> Array.exists (fun c -> + match c with + | End _ -> true + | _ -> false) + + Expect.isTrue hasEnd "Should have an End call after Dispose" + } ] + + +let messageFormattingTests = + testList + "Message Formatting" + [ testCaseAsync "Single file shows just the filename" + <| async { + let reporter, calls, _reportCount = createReporter "Typechecking" + use reporter = reporter + + let! disp = reporter.Begin ("C:/src/Foo.fs") CancellationToken.None |> Async.AwaitTask + + do! settle () + + let callsArr = calls.ToArray() + + let beginMsg = + callsArr + |> Array.tryPick (fun c -> + match c with + | Begin(_, msg, _) -> msg + | _ -> None) + + Expect.equal beginMsg (Some "Foo.fs") "Single file message should be just the filename" + + do! disp.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + } + + testCaseAsync "Two concurrent files shows both filenames" + <| async { + let reporter, calls, _reportCount = createReporter "Typechecking" + use reporter = reporter + + let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + do! settle () + let! disp2 = reporter.Begin ("C:/src/B.fs") CancellationToken.None |> Async.AwaitTask + do! settle () + + let callsArr = calls.ToArray() + + let hasMultipleFiles = + callsArr + |> Array.exists (fun c -> + match c with + | Report(Some msg, _) -> msg.Contains("A.fs") && msg.Contains("B.fs") + | _ -> false) + + Expect.isTrue hasMultipleFiles "Message should contain both file names" + + do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask + do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + } + + testCaseAsync "More than 3 concurrent files truncates with (+N more)" + <| async { + let reporter, calls, _reportCount = createReporter "Typechecking" + use reporter = reporter + + let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + let! disp2 = reporter.Begin ("C:/src/B.fs") CancellationToken.None |> Async.AwaitTask + let! disp3 = reporter.Begin ("C:/src/C.fs") CancellationToken.None |> Async.AwaitTask + let! disp4 = reporter.Begin ("C:/src/D.fs") CancellationToken.None |> Async.AwaitTask + do! settle () + + let callsArr = calls.ToArray() + + let hasTruncation = + callsArr + |> Array.exists (fun c -> + match c with + | Report(Some msg, _) -> msg.Contains("(+1 more)") + | _ -> false) + + Expect.isTrue hasTruncation "4 files should show (+1 more) truncation" + + do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask + do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask + do! disp3.DisposeAsync().AsTask() |> Async.AwaitTask + do! disp4.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + } + + testCaseAsync "Batch file with active file shows combined message" + <| async { + let reporter, calls, _reportCount = createReporter "Typechecking" + use reporter = reporter + + let! batchDisp = + reporter.BeginBatch ([| "C:/src/A.fs"; "C:/src/B.fs" |]) CancellationToken.None + |> Async.AwaitTask + + let! fileDisp = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + + do! settle () + + let callsArr = calls.ToArray() + + // Should have a message with both file name and batch progress + let hasCombined = + callsArr + |> Array.exists (fun c -> + match c with + | Report(Some msg, _) -> msg.Contains("A.fs") && msg.Contains("0/2 completed") + | _ -> false) + + Expect.isTrue hasCombined "Message should contain filename and batch progress" + + do! fileDisp.DisposeAsync().AsTask() |> Async.AwaitTask + do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + } ] + + +let cancellationTokenTests = + testList + "CancellationToken" + [ testCaseAsync "GetCancellationToken returns None when no report is active" + <| async { + let reporter, _calls, _reportCount = createReporter "Typechecking" + use reporter = reporter + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.isTrue (ct = CancellationToken.None) "Should return None when no report is active" + } + + testCaseAsync "GetCancellationToken returns token from active report" + <| async { + let reporter, _calls, _reportCount = createReporter "Typechecking" + use reporter = reporter + + let! disp = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + do! settle () + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.isFalse (ct = CancellationToken.None) "Should return active token" + Expect.isFalse ct.IsCancellationRequested "Token should not be cancelled" + + do! disp.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + } + + testCaseAsync "GetCancellationToken returns token from batch-created report" + <| async { + let reporter, _calls, _reportCount = createReporter "Typechecking" + use reporter = reporter + + let! batchDisp = + reporter.BeginBatch ([| "C:/src/A.fs" |]) CancellationToken.None + |> Async.AwaitTask + + do! settle () + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.isFalse (ct = CancellationToken.None) "BeginBatch should eagerly create report with token" + + do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + } ] + + +let disabledReporterTests = + testList + "Disabled reporter" + [ testCaseAsync "Does not create progress reports" + <| async { + let client, calls = createMockClient () + let enabled = ref false + let reportCount = ref 0 + + use reporter = + new SharedTypecheckProgressReporter( + "Typechecking", + (fun () -> + Interlocked.Increment(reportCount) |> ignore + new ServerProgressReport(client)), + isEnabled = (fun () -> enabled.Value) + ) + + let! fileDisp = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + + let! batchDisp = + reporter.BeginBatch ([| "C:/src/A.fs" |]) CancellationToken.None + |> Async.AwaitTask + + use _batchSyncDisp = reporter.BeginBatchSync([| "C:/src/A.fs" |]) + + do! fileDisp.DisposeAsync().AsTask() |> Async.AwaitTask + do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + Expect.equal reportCount.Value 0 "The disabled reporter must not create a report" + Expect.equal calls.Count 0 "The disabled reporter must not send progress" + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.equal ct CancellationToken.None "The disabled reporter must not create a cancellation token" + + enabled.Value <- true + + let! enabledDisp = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + do! settle () + + Expect.equal reportCount.Value 1 "The enabled reporter must create a report" + + do! enabledDisp.DisposeAsync().AsTask() |> Async.AwaitTask + } ] + + +let tests = + testList + "SharedTypecheckProgressReporter" + [ singleFileTests + batchTests + batchSyncTests + messageFormattingTests + cancellationTokenTests + disabledReporterTests ] From d1f49be9e1d60bcfef9ecbdc3f168d24d77a13c5 Mon Sep 17 00:00:00 2001 From: Jimmy Byrd Date: Mon, 31 Aug 2026 06:51:08 -0600 Subject: [PATCH 06/10] Fix progress reporting cancellation and tracking --- .../LspServers/AdaptiveServerState.fs | 58 +++++---- .../LspServers/FSharpLspClient.fs | 59 +++++---- .../SharedTypecheckProgressReporterTests.fs | 118 +++++++++++++++++- 3 files changed, 185 insertions(+), 50 deletions(-) diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index 162128250..90c733fb8 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -240,7 +240,11 @@ type AdaptiveState do disposables.Add analyzerProgressReporter let projectLoadingProgressReporter = - new SharedTypecheckProgressReporter("Loading Projects", fun () -> progressLookup.CreateProgressReport(lspClient)) + new SharedTypecheckProgressReporter( + "Loading Projects", + (fun () -> progressLookup.CreateProgressReport(lspClient)), + isEnabled = (fun () -> (AVal.force config).Notifications.BackgroundServiceProgress) + ) do disposables.Add projectLoadingProgressReporter @@ -619,10 +623,13 @@ type AdaptiveState let! ct = Async.CancellationToken use! _l = analyzersLocker.LockAsync(ct) use! _progress = analyzerProgressReporter.Begin($"Unnecessary parens - {fileName}") + let! progressCt = analyzerProgressReporter.GetCancellationToken() let unnecessaryParentheses = (System.Collections.Generic.HashSet(comparer = Range.comparer), tyRes.GetAST) ||> ParsedInput.fold (fun ranges path node -> + progressCt.ThrowIfCancellationRequested() + match node with | SyntaxNode.SynExpr(SynExpr.Paren(expr = inner; rightParenRange = Some _; range = range)) when not (SynExpr.shouldBeParenthesizedInContext getSourceLine path inner) @@ -638,6 +645,8 @@ type AdaptiveState | _ -> ranges) + progressCt.ThrowIfCancellationRequested() + do! triggerNotificationAndWait (NotificationEvent.UnnecessaryParentheses(filePath, Array.ofSeq unnecessaryParentheses, file.Version)) @@ -1968,18 +1977,18 @@ type AdaptiveState let! progressCt = typecheckProgressReporter.GetCancellationToken() let! result = - match compilerOptions with - | CompilerProjectOption.TransparentCompiler snap -> - checker.ParseAndCheckFileInProject(file.Source.FileName, snap, shouldCache = shouldCache) - | CompilerProjectOption.BackgroundCompiler opts -> - checker.ParseAndCheckFileInProject( - file.Source.FileName, - file.Version, - file.Source, - opts, - shouldCache = shouldCache - ) - |> Async.withCancellation progressCt + (match compilerOptions with + | CompilerProjectOption.TransparentCompiler snap -> + checker.ParseAndCheckFileInProject(file.Source.FileName, snap, shouldCache = shouldCache) + | CompilerProjectOption.BackgroundCompiler opts -> + checker.ParseAndCheckFileInProject( + file.Source.FileName, + file.Version, + file.Source, + opts, + shouldCache = shouldCache + )) + |> Async.withCancellation progressCt let! ct = Async.CancellationToken @@ -2747,7 +2756,6 @@ type AdaptiveState let batchFileList = innerChecks |> Array.map (fun (_, file) -> UMX.untag file) - // Start batch first so the report (and its CancellationToken) exists before per-file work begins use! _batchProgress = typecheckProgressReporter.BeginBatch(batchFileList) let! progressCt = typecheckProgressReporter.GetCancellationToken() @@ -2757,19 +2765,15 @@ type AdaptiveState innerChecks |> Array.map (fun (proj, file) -> asyncEx { - - use joinedToken = + let fileToken = if file = sourceFilePath then - // dont reset the token for the incoming file as it would cancel the whole operation - CancellationTokenSource.CreateLinkedTokenSource(rootToken, progressCt) + CancellationToken.None else - // only cancel other files - // If we have multiple saves from separate root files we want only one to be running - let fileToken = resetCancellationToken file None - // and join with the root token as well since we want to cancel the whole operation if the root files changes - CancellationTokenSource.CreateLinkedTokenSource(rootToken, fileToken, progressCt) + resetCancellationToken file None + + use joinedToken = + CancellationTokenSource.CreateLinkedTokenSource(rootToken, fileToken, progressCt) - // Track per-file progress so the shared reporter updates message and batch counter use! _fileProgress = typecheckProgressReporter.Begin(UMX.untag file) try @@ -2778,8 +2782,10 @@ type AdaptiveState |> Async.withCancellation joinedToken.Token () - with :? OperationCanceledException -> - // if a file shows up multiple times in the list such as Microsoft.NET.Test.Sdk.Program.fs we may cancel it but we don't want to stop the whole operation for it + with :? OperationCanceledException when + fileToken.IsCancellationRequested + && not rootToken.IsCancellationRequested + && not progressCt.IsCancellationRequested -> () }) diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fs b/src/FsAutoComplete/LspServers/FSharpLspClient.fs index 9fe2d37ed..de405ae29 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fs +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fs @@ -197,25 +197,39 @@ type SharedTypecheckProgressReporter let locker = new SemaphoreSlim(1, 1) let isEnabled = defaultArg isEnabled (fun () -> true) - /// Display-only set of short file names currently being checked - let mutable activeFiles = Set.empty + let mutable activeFiles = Map.empty let mutable progressReport: ServerProgressReport option = None let mutable batchTotal = 0 let mutable batchCompleted = 0 - /// Tracks which files belong to active batches (by full normalized path) for accurate counting - let mutable batchFiles = Set.empty - /// Number of currently active batch scopes (supports overlapping batches) + let mutable batchFiles = Map.empty let mutable activeBatchScopes = 0 let normalizePath (path: string) = IO.Path.GetFullPath(path) + let incrementCount key counts = + counts + |> Map.change key (function + | Some count -> Some(count + 1) + | None -> Some 1) + + let decrementCount key counts = + counts + |> Map.change key (function + | Some count when count > 1 -> Some(count - 1) + | Some _ -> None + | None -> None) + let buildMessage () = + let activeFileNames = + activeFiles |> Map.toList |> List.map (fst >> IO.Path.GetFileName) + let filesPart = - match Set.count activeFiles with - | 0 -> None - | 1 -> activeFiles |> Set.toList |> List.head |> Some - | n -> - let fileList = activeFiles |> Set.toList |> List.truncate 3 |> String.concat ", " + match activeFileNames with + | [] -> None + | [ file ] -> Some file + | files -> + let n = List.length files + let fileList = files |> List.truncate 3 |> String.concat ", " (if n > 3 then $"{fileList} (+{n - 3} more)" else fileList) |> Some @@ -240,8 +254,8 @@ type SharedTypecheckProgressReporter member private x.StartFile(fileName: string) = cancellableTask { use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) - let simpleName = IO.Path.GetFileName fileName - activeFiles <- Set.add simpleName activeFiles + let normalizedName = normalizePath fileName + activeFiles <- incrementCount normalizedName activeFiles match progressReport with | None -> @@ -257,17 +271,15 @@ type SharedTypecheckProgressReporter member private x.EndFile(fileName: string) = cancellableTask { use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) - let simpleName = IO.Path.GetFileName fileName - activeFiles <- Set.remove simpleName activeFiles - let normalizedName = normalizePath fileName + activeFiles <- decrementCount normalizedName activeFiles - if batchTotal > 0 && Set.contains normalizedName batchFiles then + if batchTotal > 0 && Map.containsKey normalizedName batchFiles then batchCompleted <- min (batchCompleted + 1) batchTotal - batchFiles <- Set.remove normalizedName batchFiles + batchFiles <- decrementCount normalizedName batchFiles match progressReport with - | Some report when Set.isEmpty activeFiles && activeBatchScopes <= 0 -> + | Some report when Map.isEmpty activeFiles && activeBatchScopes <= 0 -> do! report.End() progressReport <- None | Some report -> @@ -297,9 +309,12 @@ type SharedTypecheckProgressReporter cancellableTask { use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) activeBatchScopes <- activeBatchScopes + 1 - let normalizedFiles = files |> Array.map normalizePath |> Array.distinct + let normalizedFiles = files |> Array.map normalizePath batchTotal <- batchTotal + normalizedFiles.Length - batchFiles <- Set.union batchFiles (normalizedFiles |> Set.ofArray) + + batchFiles <- + (batchFiles, normalizedFiles) + ||> Array.fold (fun counts file -> incrementCount file counts) // Eagerly create the report so the CancellationToken is available for linking match progressReport with @@ -319,10 +334,10 @@ type SharedTypecheckProgressReporter activeBatchScopes <- 0 batchTotal <- 0 batchCompleted <- 0 - batchFiles <- Set.empty + batchFiles <- Map.empty // End the report if nothing else is active - if Set.isEmpty activeFiles && activeBatchScopes <= 0 then + if Map.isEmpty activeFiles && activeBatchScopes <= 0 then match progressReport with | Some report -> do! report.End() diff --git a/test/FsAutoComplete.Tests.Lsp/SharedTypecheckProgressReporterTests.fs b/test/FsAutoComplete.Tests.Lsp/SharedTypecheckProgressReporterTests.fs index 2c6fded00..fbc2d9b33 100644 --- a/test/FsAutoComplete.Tests.Lsp/SharedTypecheckProgressReporterTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/SharedTypecheckProgressReporterTests.fs @@ -151,6 +151,13 @@ let private createReporter (title: string) = /// Wait briefly for fire-and-forget tasks to settle let private settle () = Task.Delay(50) |> Async.AwaitTask +let private lastPercentage (calls: ConcurrentQueue) = + calls.ToArray() + |> Array.rev + |> Array.tryPick (function + | Report(_, Some percentage) -> Some percentage + | _ -> None) + let singleFileTests = testList @@ -178,7 +185,7 @@ let singleFileTests = Expect.isTrue (ct = CancellationToken.None) "CancellationToken should be None after all files end" } - testCaseAsync "Multiple sequential files reuse the same report cycle" + testCaseAsync "Sequential files create separate report cycles" <| async { let reporter, _calls, reportCount = createReporter "Typechecking" @@ -226,6 +233,48 @@ let singleFileTests = let! ct = reporter.GetCancellationToken() |> Async.AwaitTask Expect.isTrue (ct = CancellationToken.None) "Report should end after all files complete" + } + + testCaseAsync "Concurrent scopes for the same path keep the report active" + <| async { + let reporter, _calls, _reportCount = createReporter "Typechecking" + use reporter = reporter + + let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + let! disp2 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + + do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.notEqual ct CancellationToken.None "The second scope must keep the report active" + + do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.equal ct CancellationToken.None "The report must end after both scopes end" + } + + testCaseAsync "Files with the same basename keep independent scopes" + <| async { + let reporter, _calls, _reportCount = createReporter "Typechecking" + use reporter = reporter + + let! disp1 = reporter.Begin ("C:/src/A/Foo.fs") CancellationToken.None |> Async.AwaitTask + let! disp2 = reporter.Begin ("C:/src/B/Foo.fs") CancellationToken.None |> Async.AwaitTask + + do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.notEqual ct CancellationToken.None "The second path must keep the report active" + + do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let! ct = reporter.GetCancellationToken() |> Async.AwaitTask + Expect.equal ct CancellationToken.None "The report must end after both paths complete" } ] @@ -311,6 +360,69 @@ let batchTests = do! settle () } + testCaseAsync "Batch counts duplicate paths independently" + <| async { + let reporter, calls, _reportCount = createReporter "Typechecking" + use reporter = reporter + + let! batchDisp = + reporter.BeginBatch ([| "C:/src/A.fs"; "C:/src/A.fs" |]) CancellationToken.None + |> Async.AwaitTask + + let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + let! disp2 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + + do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let firstPercentage = lastPercentage calls + + Expect.equal firstPercentage (Some 50u) "The first duplicate completion must report 50 percent" + + do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let secondPercentage = lastPercentage calls + + Expect.equal secondPercentage (Some 100u) "The second duplicate completion must report 100 percent" + + do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask + } + + testCaseAsync "Overlapping batches count the same path independently" + <| async { + let reporter, calls, _reportCount = createReporter "Typechecking" + use reporter = reporter + + let! batchDisp1 = + reporter.BeginBatch ([| "C:/src/A.fs" |]) CancellationToken.None + |> Async.AwaitTask + + let! batchDisp2 = + reporter.BeginBatch ([| "C:/src/A.fs" |]) CancellationToken.None + |> Async.AwaitTask + + let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + let! disp2 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask + + do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let firstPercentage = lastPercentage calls + + Expect.equal firstPercentage (Some 50u) "The first overlapping completion must report 50 percent" + + do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask + do! settle () + + let secondPercentage = lastPercentage calls + + Expect.equal secondPercentage (Some 100u) "The second overlapping completion must report 100 percent" + + do! batchDisp2.DisposeAsync().AsTask() |> Async.AwaitTask + do! batchDisp1.DisposeAsync().AsTask() |> Async.AwaitTask + } + testCaseAsync "Batch with active file scopes keeps report alive" <| async { let reporter, _calls, _reportCount = createReporter "Typechecking" @@ -359,6 +471,8 @@ let batchTests = do! settle () // Second batch — counters should have been reset + let callCountBeforeSecondBatch = calls.Count + let! batchDisp2 = reporter.BeginBatch ([| "C:/src/C.fs"; "C:/src/D.fs" |]) CancellationToken.None |> Async.AwaitTask @@ -366,7 +480,7 @@ let batchTests = do! settle () // The Begin for second batch should show 0% (0/2) - let callsArr = calls.ToArray() + let callsArr = calls.ToArray() |> Array.skip callCountBeforeSecondBatch let hasZeroPercentage = callsArr From d454d462200472aa4d0bb9a492f570160b9e648c Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Wed, 28 Oct 2026 15:07:18 +0100 Subject: [PATCH 07/10] Address review feedback on shared progress reporting Rename SharedTypecheckProgressReporter to SharedProgressReporter and rework it around scopes: - Begin(label) and BeginBatch(count) return scopes that carry the cancellation token of their report, so GetCancellationToken is gone and a disabled reporter takes no lock. - A single agent sends all progress messages to the client in order. Callers only change the state under a lock and never wait for the workDoneProgress/create round-trip. Bursts are coalesced and nothing is left unawaited. - When the user cancels the report, the next operation gets a fresh report and token. Operations that a cancelled batch starts later get the cancelled token, so the batch stops. - A batch counts its own operations instead of file paths. A batch that ends early stops counting its operations that never ran. Batch operations have no label: the typecheck they run shows the file name once it holds the typecheck lock, so queued files do not show. - Labels are shown as is instead of passing through Path.GetFullPath. The typecheck itself does not use the progress token, as on main: a cancelled check of an open file would stay in the adaptive cache until the file changes. The dependent-file batch links the batch token, so cancelling the report stops that batch. Keep the behaviour of main where the review asked for it: - Typecheck progress is always shown again. Only analyzer progress follows FSharp.notifications.backgroundServiceProgress. - Project loading shows "Loading N Projects" as before. - Dependent files are checked with parallel75 again, and parallel50, parallel25 and the lost doc comment are reverted. A cancelled dependent-file check is logged at debug level instead of failing the save handler. Analyzers log a cancel at debug level, not error. Port the reporter tests to the new API, parse the progress payload as a JToken and assert only on the calls a step adds. --- src/FsAutoComplete.Core/Utils.fs | 11 +- src/FsAutoComplete.Core/Utils.fsi | 8 - .../LspServers/AdaptiveServerState.fs | 131 ++- .../LspServers/FSharpLspClient.fs | 443 ++++++---- .../LspServers/FSharpLspClient.fsi | 52 +- test/FsAutoComplete.Tests.Lsp/Program.fs | 4 +- .../SharedProgressReporterTests.fs | 448 ++++++++++ .../SharedTypecheckProgressReporterTests.fs | 795 ------------------ 8 files changed, 823 insertions(+), 1069 deletions(-) create mode 100644 test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs delete mode 100644 test/FsAutoComplete.Tests.Lsp/SharedTypecheckProgressReporterTests.fs diff --git a/src/FsAutoComplete.Core/Utils.fs b/src/FsAutoComplete.Core/Utils.fs index 688b756c0..bab105fa5 100644 --- a/src/FsAutoComplete.Core/Utils.fs +++ b/src/FsAutoComplete.Core/Utils.fs @@ -230,17 +230,14 @@ module Async = // Start the workflow using a provided cancellation token Async.StartWithContinuations(work, cont, econt, ccont, cancellationToken = cancellationToken)) - - let private parallelOfPercentage percentage computations = + /// Creates an asynchronous computation that executes all the given asynchronous computations, using 75% of the Environment.ProcessorCount + /// A sequence of distinct computations to be parallelized. + let parallel75 computations = let maxConcurrency = - Math.Max(1.0, Math.Floor((float System.Environment.ProcessorCount) * percentage)) + Math.Max(1.0, Math.Floor((float System.Environment.ProcessorCount) * 0.75)) Async.Parallel(computations, int maxConcurrency) - let parallel75 computations = parallelOfPercentage 0.75 computations - let parallel50 computations = parallelOfPercentage 0.50 computations - let parallel25 computations = parallelOfPercentage 0.25 computations - [] module Array = /// Async implementation of Array.map. diff --git a/src/FsAutoComplete.Core/Utils.fsi b/src/FsAutoComplete.Core/Utils.fsi index f8a85e0e2..fe18becb8 100644 --- a/src/FsAutoComplete.Core/Utils.fsi +++ b/src/FsAutoComplete.Core/Utils.fsi @@ -112,14 +112,6 @@ module Async = /// A sequence of distinct computations to be parallelized. val parallel75: computations: seq> -> Async<'a array> - /// Creates an asynchronous computation that executes all the given asynchronous computations, using 50% of the Environment.ProcessorCount - /// A sequence of distinct computations to be parallelized. - val parallel50: computations: seq> -> Async<'a array> - - /// Creates an asynchronous computation that executes all the given asynchronous computations, using 25% of the Environment.ProcessorCount - /// A sequence of distinct computations to be parallelized. - val parallel25: computations: seq> -> Async<'a array> - [] module Array = /// Async implementation of Array.map. diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index 90c733fb8..7c98fd83a 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -222,16 +222,15 @@ type AdaptiveState let config = cval FSharpConfig.Default let typecheckProgressReporter = - new SharedTypecheckProgressReporter( + new SharedProgressReporter( "Typechecking", - (fun () -> progressLookup.CreateProgressReport(lspClient, cancellable = true)), - isEnabled = (fun () -> (AVal.force config).Notifications.BackgroundServiceProgress) + (fun () -> progressLookup.CreateProgressReport(lspClient, cancellable = true)) ) do disposables.Add typecheckProgressReporter let analyzerProgressReporter = - new SharedTypecheckProgressReporter( + new SharedProgressReporter( "Analyzing", (fun () -> progressLookup.CreateProgressReport(lspClient, cancellable = true)), isEnabled = (fun () -> (AVal.force config).Notifications.BackgroundServiceProgress) @@ -239,15 +238,6 @@ type AdaptiveState do disposables.Add analyzerProgressReporter - let projectLoadingProgressReporter = - new SharedTypecheckProgressReporter( - "Loading Projects", - (fun () -> progressLookup.CreateProgressReport(lspClient)), - isEnabled = (fun () -> (AVal.force config).Notifications.BackgroundServiceProgress) - ) - - do disposables.Add projectLoadingProgressReporter - let serilogLoggerFactory = new SerilogLoggerFactory(Serilog.Log.Logger) do disposables.Add serilogLoggerFactory @@ -563,19 +553,19 @@ type AdaptiveState try let! ct = Async.CancellationToken use! _l = analyzersLocker.LockAsync(ct) - use! _progress = analyzerProgressReporter.Begin($"Unused opens - {fileName}") - let! progressCt = analyzerProgressReporter.GetCancellationToken() + use progress = analyzerProgressReporter.Begin($"Unused opens - {fileName}") let! unused = UnusedOpens.getUnusedOpens (tyRes.GetCheckResults, getSourceLine) - |> Async.withCancellation progressCt + |> Async.withCancellation progress.CancellationToken do! triggerNotificationAndWait (NotificationEvent.UnusedOpens(filePath, (unused |> List.toArray), file.Version)) ct - with e -> - logger.error (Log.setMessage "checkUnusedOpens failed" >> Log.addExn e) + with + | :? OperationCanceledException -> logger.debug (Log.setMessage "checkUnusedOpens cancelled") + | e -> logger.error (Log.setMessage "checkUnusedOpens failed" >> Log.addExn e) } let checkUnusedDeclarations = @@ -583,20 +573,20 @@ type AdaptiveState try let! ct = Async.CancellationToken use! _l = analyzersLocker.LockAsync(ct) - use! _progress = analyzerProgressReporter.Begin($"Unused declarations - {fileName}") - let! progressCt = analyzerProgressReporter.GetCancellationToken() + use progress = analyzerProgressReporter.Begin($"Unused declarations - {fileName}") let isScript = Utils.isAScript (filePathUntag) let! unused = UnusedDeclarations.getUnusedDeclarations (tyRes.GetCheckResults, isScript) - |> Async.withCancellation progressCt + |> Async.withCancellation progress.CancellationToken let unused = unused |> Seq.toArray do! triggerNotificationAndWait (NotificationEvent.UnusedDeclarations(filePath, unused, file.Version)) ct - with e -> - logger.error (Log.setMessage "checkUnusedDeclarations failed" >> Log.addExn e) + with + | :? OperationCanceledException -> logger.debug (Log.setMessage "checkUnusedDeclarations cancelled") + | e -> logger.error (Log.setMessage "checkUnusedDeclarations failed" >> Log.addExn e) } let checkSimplifiedNames = @@ -604,17 +594,17 @@ type AdaptiveState try let! ct = Async.CancellationToken use! _l = analyzersLocker.LockAsync(ct) - use! _progress = analyzerProgressReporter.Begin($"Simplify names - {fileName}") - let! progressCt = analyzerProgressReporter.GetCancellationToken() + use progress = analyzerProgressReporter.Begin($"Simplify names - {fileName}") let! simplified = SimplifyNames.getSimplifiableNames (tyRes.GetCheckResults, getSourceLine) - |> Async.withCancellation progressCt + |> Async.withCancellation progress.CancellationToken let simplified = Array.ofSeq simplified do! triggerNotificationAndWait (NotificationEvent.SimplifyNames(filePath, simplified, file.Version)) ct - with e -> - logger.error (Log.setMessage "checkSimplifiedNames failed" >> Log.addExn e) + with + | :? OperationCanceledException -> logger.debug (Log.setMessage "checkSimplifiedNames cancelled") + | e -> logger.error (Log.setMessage "checkSimplifiedNames failed" >> Log.addExn e) } let checkUnnecessaryParentheses = @@ -622,13 +612,12 @@ type AdaptiveState try let! ct = Async.CancellationToken use! _l = analyzersLocker.LockAsync(ct) - use! _progress = analyzerProgressReporter.Begin($"Unnecessary parens - {fileName}") - let! progressCt = analyzerProgressReporter.GetCancellationToken() + use progress = analyzerProgressReporter.Begin($"Unnecessary parens - {fileName}") let unnecessaryParentheses = (System.Collections.Generic.HashSet(comparer = Range.comparer), tyRes.GetAST) ||> ParsedInput.fold (fun ranges path node -> - progressCt.ThrowIfCancellationRequested() + progress.CancellationToken.ThrowIfCancellationRequested() match node with | SyntaxNode.SynExpr(SynExpr.Paren(expr = inner; rightParenRange = Some _; range = range)) when @@ -645,14 +634,15 @@ type AdaptiveState | _ -> ranges) - progressCt.ThrowIfCancellationRequested() + progress.CancellationToken.ThrowIfCancellationRequested() do! triggerNotificationAndWait (NotificationEvent.UnnecessaryParentheses(filePath, Array.ofSeq unnecessaryParentheses, file.Version)) ct - with e -> - logger.error (Log.setMessage "checkUnnecessaryParentheses failed" >> Log.addExn e) + with + | :? OperationCanceledException -> logger.debug (Log.setMessage "checkUnnecessaryParentheses cancelled") + | e -> logger.error (Log.setMessage "checkUnnecessaryParentheses failed" >> Log.addExn e) } let inline isNotExcluded (exclusions: Regex array) = @@ -702,8 +692,7 @@ type AdaptiveState try let fileName = Path.GetFileName(UMX.untag file) - use! _progress = analyzerProgressReporter.Begin($"External analyzers - {fileName}") - let! progressCt = analyzerProgressReporter.GetCancellationToken() + use progress = analyzerProgressReporter.Begin($"External analyzers - {fileName}") Loggers.analyzers.info ( Log.setMessage "begin analysis of {file}" @@ -753,7 +742,7 @@ type AdaptiveState analyzerOptions, analyzerPredicate ) - |> Async.withCancellation progressCt + |> Async.withCancellation progress.CancellationToken let! ct = Async.CancellationToken do! triggerNotificationAndWait (NotificationEvent.AnalyzerMessage(res, file, volatileFile.Version)) ct @@ -764,8 +753,9 @@ type AdaptiveState Loggers.analyzers.info (Log.setMessageI $"missing components of {file:file} to run analyzers, skipped them") () - with ex -> - Loggers.analyzers.error (Log.setMessageI $"Run failed for {file:file}" >> Log.addExn ex) + with + | :? OperationCanceledException -> Loggers.analyzers.debug (Log.setMessageI $"Run cancelled for {file:file}") + | ex -> Loggers.analyzers.error (Log.setMessageI $"Run failed for {file:file}" >> Log.addExn ex) } @@ -1135,12 +1125,13 @@ type AdaptiveState triggerNotification not CancellationToken.None) - let projectFiles = projects |> Seq.map (fst >> UMX.untag) |> Seq.toArray + use progressReport = new ServerProgressReport(lspClient) - use _progressReport = projectLoadingProgressReporter.BeginBatchSync(projectFiles) + progressReport.Begin ($"Loading {projects.Count} Projects") (CancellationToken.None) + |> ignore> let projectOptions = - loader.LoadProjects(projectFiles |> Array.toList, [], binlogConfig) + loader.LoadProjects(projects |> Seq.map (fst >> UMX.untag) |> Seq.toList, [], binlogConfig) |> Seq.toList for p in projectOptions do @@ -1972,23 +1963,23 @@ type AdaptiveState ) - let fileNameStr = UMX.untag file.Source.FileName - use! _fileProgress = typecheckProgressReporter.Begin(fileNameStr) - let! progressCt = typecheckProgressReporter.GetCancellationToken() + // The token of this scope is not used: a cancelled check of an open file would stay cached until the file changes. + // The dependent-file check links the token of its batch instead. + use _progress = + typecheckProgressReporter.Begin(Path.GetFileName(UMX.untag file.Source.FileName)) let! result = - (match compilerOptions with - | CompilerProjectOption.TransparentCompiler snap -> - checker.ParseAndCheckFileInProject(file.Source.FileName, snap, shouldCache = shouldCache) - | CompilerProjectOption.BackgroundCompiler opts -> - checker.ParseAndCheckFileInProject( - file.Source.FileName, - file.Version, - file.Source, - opts, - shouldCache = shouldCache - )) - |> Async.withCancellation progressCt + match compilerOptions with + | CompilerProjectOption.TransparentCompiler snap -> + checker.ParseAndCheckFileInProject(file.Source.FileName, snap, shouldCache = shouldCache) + | CompilerProjectOption.BackgroundCompiler opts -> + checker.ParseAndCheckFileInProject( + file.Source.FileName, + file.Version, + file.Source, + opts, + shouldCache = shouldCache + ) let! ct = Async.CancellationToken @@ -2754,11 +2745,7 @@ type AdaptiveState file.Contains "AssemblyInfo.fs" |> not && file.Contains "AssemblyAttributes.fs" |> not) - let batchFileList = innerChecks |> Array.map (fun (_, file) -> UMX.untag file) - - use! _batchProgress = typecheckProgressReporter.BeginBatch(batchFileList) - let! progressCt = typecheckProgressReporter.GetCancellationToken() - + use progress = typecheckProgressReporter.BeginBatch(innerChecks.Length) let rootToken = sourceFilePath |> getOpenFileTokenOrDefault let checksToPerform = @@ -2767,14 +2754,18 @@ type AdaptiveState asyncEx { let fileToken = if file = sourceFilePath then + // dont reset the token for the incoming file as it would cancel the whole operation CancellationToken.None else + // only cancel other files + // If we have multiple saves from separate root files we want only one to be running resetCancellationToken file None + // join with the root token as well since we want to cancel the whole operation if the root files changes use joinedToken = - CancellationTokenSource.CreateLinkedTokenSource(rootToken, fileToken, progressCt) + CancellationTokenSource.CreateLinkedTokenSource(rootToken, fileToken, progress.CancellationToken) - use! _fileProgress = typecheckProgressReporter.Begin(UMX.untag file) + use _fileProgress = progress.Begin() try let! _ = @@ -2785,11 +2776,19 @@ type AdaptiveState with :? OperationCanceledException when fileToken.IsCancellationRequested && not rootToken.IsCancellationRequested - && not progressCt.IsCancellationRequested -> + && not progress.CancellationToken.IsCancellationRequested -> + // if a file shows up multiple times in the list such as Microsoft.NET.Test.Sdk.Program.fs we may cancel it but we don't want to stop the whole operation for it () }) - do! checksToPerform |> Async.parallel25 |> Async.Ignore + try + do! checksToPerform |> Async.parallel75 |> Async.Ignore + with :? OperationCanceledException -> + // The root file changed again or the user cancelled the progress report. + logger.debug ( + Log.setMessage "Typechecking the dependent files of {file} was cancelled" + >> Log.addContextDestructured "file" sourceFilePath + ) } diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fs b/src/FsAutoComplete/LspServers/FSharpLspClient.fs index de405ae29..1e4373f19 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fs +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fs @@ -187,216 +187,311 @@ type ServerProgressReport(lspClient: FSharpLspClient, ?token: ProgressToken, ?ca member x.Dispose() = (x :> IAsyncDisposable).DisposeAsync() |> ignore +open FsAutoComplete.Logging + +/// One operation tracked by a . Dispose it when the operation ends. +[] +type SharedProgressScope(cancellationToken: CancellationToken, onDispose: unit -> unit) = + let mutable disposed = 0 + + member _.CancellationToken = cancellationToken + + interface IDisposable with + member _.Dispose() = + if Interlocked.Exchange(&disposed, 1) = 0 then + onDispose () + +/// A batch of operations tracked by a . Dispose it when the batch ends. +[] +type SharedProgressBatch + (cancellationToken: CancellationToken, beginOperation: unit -> SharedProgressScope, onDispose: unit -> unit) = + let mutable disposed = 0 + + member _.CancellationToken = cancellationToken + + member _.Begin() = beginOperation () + + interface IDisposable with + member _.Dispose() = + if Interlocked.Exchange(&disposed, 1) = 0 then + onDispose () + +/// One progress report of a SharedProgressReporter, from its creation until its last operation ends. +/// Only access it while holding the lock of the reporter. +type SharedProgressGeneration(report: ServerProgressReport) = + member val Report = report + + /// The running operations and their labels, in the order they started. + member val Operations = Map.empty with get, set + member val Batches = 0 with get, set + member val BatchTotal = 0 with get, set + member val BatchCompleted = 0 with get, set + member x.IsIdle = Map.isEmpty x.Operations && x.Batches = 0 + +type SharedProgressRequest = + | SyncProgress + | FlushProgress of AsyncReplyChannel + /// -/// A shared progress reporter that consolidates multiple concurrent typecheck operations -/// into a single LSP progress notification. Instead of creating a new Begin/End cycle per file, -/// this maintains one notification that updates its message with the current file being checked. +/// Shows concurrent operations, such as typechecks, as one LSP progress report. +/// The report starts with the first operation, lists the running operations in its message, +/// and ends when the last operation ends. /// -type SharedTypecheckProgressReporter - (title: string, createReport: unit -> ServerProgressReport, ?isEnabled: unit -> bool) = +[] +type SharedProgressReporter(title: string, createReport: unit -> ServerProgressReport, ?isEnabled: unit -> bool) = - let locker = new SemaphoreSlim(1, 1) let isEnabled = defaultArg isEnabled (fun () -> true) - let mutable activeFiles = Map.empty - let mutable progressReport: ServerProgressReport option = None - let mutable batchTotal = 0 - let mutable batchCompleted = 0 - let mutable batchFiles = Map.empty - let mutable activeBatchScopes = 0 - - let normalizePath (path: string) = IO.Path.GetFullPath(path) - - let incrementCount key counts = - counts - |> Map.change key (function - | Some count -> Some(count + 1) - | None -> Some 1) - - let decrementCount key counts = - counts - |> Map.change key (function - | Some count when count > 1 -> Some(count - 1) - | Some _ -> None - | None -> None) - - let buildMessage () = - let activeFileNames = - activeFiles |> Map.toList |> List.map (fst >> IO.Path.GetFileName) - - let filesPart = - match activeFileNames with + let logger = LogProvider.getLoggerByName "SharedProgressReporter" + let syncRoot = obj () + let mutable current: SharedProgressGeneration option = None + let mutable lastOperationId = 0L + let mutable syncPending = 0 + let mutable disposed = false + + let buildMessage (generation: SharedProgressGeneration) = + let labels = + generation.Operations + |> Map.values + |> Seq.choose id + |> Seq.distinct + |> Seq.toList + + let labelsPart = + match labels with | [] -> None - | [ file ] -> Some file - | files -> - let n = List.length files - let fileList = files |> List.truncate 3 |> String.concat ", " + | [ label ] -> Some label + | labels -> + let shown = labels |> List.truncate 3 |> String.concat ", " + let hidden = labels.Length - 3 - (if n > 3 then $"{fileList} (+{n - 3} more)" else fileList) |> Some + Some(if hidden > 0 then $"{shown} (+{hidden} more)" else shown) let batchPart = - if batchTotal > 0 then - Some $"{batchCompleted}/{batchTotal} completed" + if generation.BatchTotal > 0 then + Some $"{generation.BatchCompleted}/{generation.BatchTotal} completed" else None - match filesPart, batchPart with - | Some files, Some batch -> Some $"{files} ({batch})" - | Some files, None -> Some files + match labelsPart, batchPart with + | Some labels, Some batch -> Some $"{labels} ({batch})" + | Some labels, None -> Some labels | None, Some batch -> Some batch | None, None -> None - let calcPercentage () = - if batchTotal > 0 then - ((float batchCompleted) / (float batchTotal)) * 100.0 |> uint32 |> Some + let calcPercentage (generation: SharedProgressGeneration) = + if generation.BatchTotal > 0 then + min 100 (generation.BatchCompleted * 100 / generation.BatchTotal) + |> uint32 + |> Some else None - member private x.StartFile(fileName: string) = - cancellableTask { - use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) - let normalizedName = normalizePath fileName - activeFiles <- incrementCount normalizedName activeFiles - - match progressReport with - | None -> - let report = createReport () - progressReport <- Some report - let message = buildMessage () - do! report.Begin(title, ?message = message, ?percentage = calcPercentage ()) - | Some report -> - let message = buildMessage () - do! report.Report(?message = message, ?percentage = calcPercentage ()) - } - - member private x.EndFile(fileName: string) = - cancellableTask { - use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) - let normalizedName = normalizePath fileName - activeFiles <- decrementCount normalizedName activeFiles - - if batchTotal > 0 && Map.containsKey normalizedName batchFiles then - batchCompleted <- min (batchCompleted + 1) batchTotal - batchFiles <- decrementCount normalizedName batchFiles - - match progressReport with - | Some report when Map.isEmpty activeFiles && activeBatchScopes <= 0 -> - do! report.End() - progressReport <- None - | Some report -> - let message = buildMessage () - do! report.Report(?message = message, ?percentage = calcPercentage ()) - | None -> () + let send (kind: string) (work: CancellationToken -> Task) = + task { + try + do! work CancellationToken.None + with e -> + logger.warn ( + Log.setMessage "Could not send {kind} of {title} progress" + >> Log.addContext "kind" kind + >> Log.addContext "title" title + >> Log.addExn e + ) } - /// Begin tracking a file being typechecked. Returns an IAsyncDisposable that ends tracking on dispose. - member x.Begin(fileName: string) : CancellableTask = - cancellableTask { - let started = isEnabled () - - if started then - do! x.StartFile(fileName) + /// Brings the client from the shown progress state to the wanted one. + /// A report that started and ended between two syncs is never sent. + let sync + (shown: (ServerProgressReport * string option * uint32 option) option) + (wanted: (ServerProgressReport * string option * uint32 option) option) + = + task { + match shown, wanted with + | Some(shownReport, shownMessage, shownPercentage), Some(report, message, percentage) when + obj.ReferenceEquals(shownReport, report) + -> + if shownMessage <> message || shownPercentage <> percentage then + do! send "report" (report.Report(?message = message, ?percentage = percentage)) + | _ -> + match shown with + | Some(shownReport, _, _) -> + do! send "end" (shownReport.End()) + (shownReport :> IDisposable).Dispose() + | None -> () - return - { new IAsyncDisposable with - member _.DisposeAsync() = - if started then - x.EndFile (fileName) CancellationToken.None |> ValueTask - else - ValueTask() } - } + match wanted with + | Some(report, message, percentage) -> + do! send "begin" (report.Begin(title, ?message = message, ?percentage = percentage)) + | None -> () - member private x.StartBatch(files: string array) = - cancellableTask { - use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) - activeBatchScopes <- activeBatchScopes + 1 - let normalizedFiles = files |> Array.map normalizePath - batchTotal <- batchTotal + normalizedFiles.Length - - batchFiles <- - (batchFiles, normalizedFiles) - ||> Array.fold (fun counts file -> incrementCount file counts) - - // Eagerly create the report so the CancellationToken is available for linking - match progressReport with - | None -> - let report = createReport () - progressReport <- Some report - do! report.Begin(title, ?percentage = calcPercentage ()) - | Some report -> do! report.Report(?percentage = calcPercentage ()) + return wanted } - member private x.EndBatch() = - cancellableTask { - use! __ = fun (ct: CancellationToken) -> locker.LockAsync(ct) - activeBatchScopes <- activeBatchScopes - 1 - - if activeBatchScopes <= 0 then - activeBatchScopes <- 0 - batchTotal <- 0 - batchCompleted <- 0 - batchFiles <- Map.empty - - // End the report if nothing else is active - if Map.isEmpty activeFiles && activeBatchScopes <= 0 then - match progressReport with - | Some report -> - do! report.End() - progressReport <- None - | None -> () - } + // The callers only change the state; this agent alone talks to the client, in order, + // so no caller waits for a client round-trip. + let sender = + MailboxProcessor.Start(fun inbox -> + let rec loop shown = + async { + let! request = inbox.Receive() + + let! shown = + async { + try + let wanted = + lock syncRoot (fun () -> + syncPending <- 0 + + current + |> Option.map (fun generation -> + generation.Report, buildMessage generation, calcPercentage generation)) + + return! sync shown wanted |> Async.AwaitTask + with e -> + logger.warn ( + Log.setMessage "Could not sync {title} progress" + >> Log.addContext "title" title + >> Log.addExn e + ) + + return shown + } + + match request with + | FlushProgress reply -> reply.Reply() + | SyncProgress -> () + + return! loop shown + } + + loop None) + + let requestSync () = + if Interlocked.Exchange(&syncPending, 1) = 0 then + sender.Post SyncProgress + + /// Call while holding syncRoot. + let currentGeneration () = + match current with + | Some generation when not generation.Report.CancellationToken.IsCancellationRequested -> generation + | _ -> + // The user cancelled the current report: new operations get a fresh report and token. + let generation = SharedProgressGeneration(createReport ()) + current <- Some generation + generation + + /// Call while holding syncRoot. + let retireIfIdle (generation: SharedProgressGeneration) = + match current with + | Some c when obj.ReferenceEquals(c, generation) && generation.IsIdle -> current <- None + | _ -> () + + /// Call while holding syncRoot. onEnd is called while holding syncRoot. + let startOperation (generation: SharedProgressGeneration) (label: string option) (onEnd: unit -> unit) = + lastOperationId <- lastOperationId + 1L + let operationId = lastOperationId + generation.Operations <- Map.add operationId label generation.Operations + + new SharedProgressScope( + generation.Report.CancellationToken, + fun () -> + lock syncRoot (fun () -> + generation.Operations <- Map.remove operationId generation.Operations + onEnd () + retireIfIdle generation) + + requestSync () + ) - /// Set up a batch of files to be typechecked. Returns an IAsyncDisposable that clears the batch on dispose. - member x.BeginBatch(files: string array) : CancellableTask = - cancellableTask { - let started = isEnabled () + let disabledScope () = new SharedProgressScope(CancellationToken.None, ignore) - if started then - do! x.StartBatch(files) + let disabledBatch () = new SharedProgressBatch(CancellationToken.None, disabledScope, ignore) - return - { new IAsyncDisposable with - member _.DisposeAsync() = - if started then - x.EndBatch () CancellationToken.None |> ValueTask + /// Starts an operation. Its label shows in the message of the report until the scope is disposed. + member _.Begin(label: string) = + if not (isEnabled ()) then + disabledScope () + else + let scope = + lock syncRoot (fun () -> + if disposed then + disabledScope () + else + startOperation (currentGeneration ()) (Some label) ignore) + + requestSync () + scope + + /// Starts a batch of operations. The report shows how many of them completed. + member _.BeginBatch(count: int) = + if count <= 0 || not (isEnabled ()) then + disabledBatch () + else + let generation = + lock syncRoot (fun () -> + if disposed then + None + else + let generation = currentGeneration () + generation.Batches <- generation.Batches + 1 + generation.BatchTotal <- generation.BatchTotal + count + Some generation) + + match generation with + | None -> disabledBatch () + | Some generation -> + requestSync () + + let token = generation.Report.CancellationToken + // Only accessed while holding syncRoot. + let completed = ref 0 + let ended = ref false + + let completeOperation () = + if not ended.Value then + completed.Value <- completed.Value + 1 + generation.BatchCompleted <- generation.BatchCompleted + 1 + + let beginOperation () = + let scope = + lock syncRoot (fun () -> + if token.IsCancellationRequested then + // The user cancelled the batch, so its remaining operations do not show. + new SharedProgressScope(token, ignore) else - ValueTask() } - } + startOperation generation None completeOperation) - /// Gets the cancellation token from the current progress report, or CancellationToken.None if no report is active. - member x.GetCancellationToken() : Task = - task { - use! __ = locker.LockAsync() + requestSync () + scope - return - match progressReport with - | Some report -> report.CancellationToken - | None -> CancellationToken.None - } + let endBatch () = + lock syncRoot (fun () -> + ended.Value <- true + generation.Batches <- generation.Batches - 1 - /// Set up a batch of files synchronously (for contexts where CancellableTask is not available). - /// Sets up batch tracking and adds all files to activeFiles so their names appear in the message. - /// Returns an IDisposable that ends all file tracking and the batch on dispose. - member x.BeginBatchSync(files: string array) : IDisposable = - let started = isEnabled () + if generation.Batches = 0 then + generation.BatchTotal <- 0 + generation.BatchCompleted <- 0 + else + // Operations of this batch that never ran no longer count towards the batches still running. + generation.BatchTotal <- generation.BatchTotal - max 0 (count - completed.Value) - if started then - x.StartBatch (files) CancellationToken.None |> ignore> + retireIfIdle generation) - for file in files do - x.StartFile (file) CancellationToken.None |> ignore> + requestSync () - { new IDisposable with - member _.Dispose() = - if started then - for file in files do - x.EndFile (file) CancellationToken.None |> ignore> + new SharedProgressBatch(token, beginOperation, endBatch) - x.EndBatch () CancellationToken.None |> ignore> } + /// Completes when the client has been sent the current progress state. + member _.Flush() : Task = sender.PostAndAsyncReply FlushProgress |> Async.StartImmediateAsTask interface IDisposable with - member x.Dispose() = - progressReport |> Option.iter (fun r -> (r :> IDisposable).Dispose()) - locker.Dispose() + member _.Dispose() = + lock syncRoot (fun () -> + disposed <- true + current <- None) + + requestSync () open System.Diagnostics.Tracing diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi index f4fb9f41c..f16b96f9e 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fsi +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fsi @@ -80,29 +80,47 @@ type ServerProgressReport = interface IAsyncDisposable interface IDisposable +/// One operation tracked by a . Dispose it when the operation ends. +[] +type SharedProgressScope = + /// Cancelled when the user cancels the progress report that shows the operation. + member CancellationToken: CancellationToken + interface IDisposable + +/// A batch of operations tracked by a . Dispose it when the batch ends. +[] +type SharedProgressBatch = + /// Cancelled when the user cancels the progress report that shows the batch. + member CancellationToken: CancellationToken + + /// + /// Starts an operation of the batch. When its scope is disposed, it counts as completed. + /// It has no label: the work it runs shows its own label once it actually starts. + /// + member Begin: unit -> SharedProgressScope + interface IDisposable + /// -/// A shared progress reporter that consolidates multiple concurrent typecheck operations -/// into a single LSP progress notification. Instead of creating a new Begin/End cycle per file, -/// this maintains one notification that updates its message with the current file being checked. +/// Shows concurrent operations, such as typechecks, as one LSP progress report. +/// The report starts with the first operation, lists the running operations in its message, +/// and ends when the last operation ends. /// -type SharedTypecheckProgressReporter = +[] +type SharedProgressReporter = + /// The title of the progress report. + /// Creates the report that a new progress cycle uses. + /// When it returns false, operations are not shown and their token is never cancelled. new: - title: string * createReport: (unit -> ServerProgressReport) * ?isEnabled: (unit -> bool) -> - SharedTypecheckProgressReporter - - /// Begin tracking a file being typechecked. Returns an IAsyncDisposable that ends tracking on dispose. - member Begin: fileName: string -> CancellableTask + title: string * createReport: (unit -> ServerProgressReport) * ?isEnabled: (unit -> bool) -> SharedProgressReporter - /// Set up a batch of files to be typechecked. Returns an IAsyncDisposable that clears the batch on dispose. - member BeginBatch: files: string array -> CancellableTask + /// Starts an operation. Its label shows in the message of the report until the scope is disposed. + member Begin: label: string -> SharedProgressScope - /// Gets the cancellation token from the current progress report, or CancellationToken.None if no report is active. - member GetCancellationToken: unit -> Task + /// Starts a batch of operations. The report shows how many of them completed. + member BeginBatch: count: int -> SharedProgressBatch - /// Set up a batch of files synchronously (for contexts where CancellableTask is not available). - /// Sets up batch tracking and adds all files to activeFiles so their names appear in the message. - /// Returns an IDisposable that ends all file tracking and the batch on dispose. - member BeginBatchSync: files: string array -> IDisposable + /// Completes when the client has been sent the current progress state. + member Flush: unit -> Task interface IDisposable /// listener for the the events generated from the fsc ActivitySource diff --git a/test/FsAutoComplete.Tests.Lsp/Program.fs b/test/FsAutoComplete.Tests.Lsp/Program.fs index b836a529b..9b3cb17b6 100644 --- a/test/FsAutoComplete.Tests.Lsp/Program.fs +++ b/test/FsAutoComplete.Tests.Lsp/Program.fs @@ -13,7 +13,7 @@ open FsAutoComplete.Tests.ExtensionsTests open FsAutoComplete.Tests.InteractiveDirectivesTests open FsAutoComplete.Tests.Lsp.CoreUtilsTests open FsAutoComplete.Tests.Lsp.DecompilerTests -open FsAutoComplete.Tests.SharedTypecheckProgressReporterTests +open FsAutoComplete.Tests.SharedProgressReporterTests open FsAutoComplete.Tests.CallHierarchy open Ionide.ProjInfo open System.Threading @@ -375,7 +375,7 @@ let generalTests = testCase "test host uses target runtime" (fun _ -> Expect.equal Environment.Version.Major expectedRuntimeMajor "Test host runtime must match the target framework") testList (nameof (Utils)) [ Utils.Tests.Utils.tests; Utils.Tests.TextEdit.tests ] - SharedTypecheckProgressReporterTests.tests + SharedProgressReporterTests.tests InlayHintTests.explicitTypeInfoTests sourceTextFactory FindReferences.tryFixupRangeTests sourceTextFactory UtilsTests.allTests diff --git a/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs b/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs new file mode 100644 index 000000000..8211dc1a2 --- /dev/null +++ b/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs @@ -0,0 +1,448 @@ +module FsAutoComplete.Tests.SharedProgressReporterTests + +open Expecto +open System +open System.Collections.Concurrent +open FsAutoComplete.Lsp +open Ionide.LanguageServerProtocol +open Ionide.LanguageServerProtocol.Server +open Ionide.LanguageServerProtocol.Types +open Ionide.LanguageServerProtocol.JsonRpc +open Newtonsoft.Json.Linq + +/// Captured progress call from the mock client +type ProgressCall = + | Begin of title: string * message: string option * percentage: uint32 option + | Report of message: string option * percentage: uint32 option + | End + +/// Creates a mock FSharpLspClient that captures all progress notifications +let private createMockClient () = + let progressCalls = ConcurrentQueue() + + let notificationSender: ClientNotificationSender = + fun methodName payload -> + if methodName = "$/progress" then + let value = (payload :?> ProgressParams).Value + + let tryGet (name: string) = + match value.[name] with + | null -> None + | token -> Some token + + let message = tryGet "message" |> Option.map (fun t -> t.Value()) + let percentage = tryGet "percentage" |> Option.map (fun t -> t.Value()) + + match value.Value("kind") with + | "begin" -> progressCalls.Enqueue(Begin(value.Value("title"), message, percentage)) + | "report" -> progressCalls.Enqueue(Report(message, percentage)) + | "end" -> progressCalls.Enqueue End + | kind -> failwith $"Unexpected progress kind {kind}" + + AsyncLspResult.success () + + let requestSender = + { new ClientRequestSender with + member _.Send _name _payload = AsyncLspResult.success Unchecked.defaultof<_> + } + + let client = new FSharpLspClient(notificationSender, requestSender) + + client.ClientCapabilities <- + Some + { + Workspace = None + TextDocument = None + Experimental = None + Window = + Some + { + WorkDoneProgress = Some true + ShowMessage = None + ShowDocument = None + } + General = None + NotebookDocument = None + } + + client, progressCalls + +type private TestReporter = + { + Reporter: SharedProgressReporter + Calls: ConcurrentQueue + Reports: ConcurrentQueue + } + + /// Waits until the client has been sent the current state, then returns all calls so far. + member x.Flush() = + async { + do! x.Reporter.Flush() |> Async.AwaitTask + return x.Calls.ToArray() |> Array.toList + } + +let private createReporter (isEnabled: bool) = + let client, calls = createMockClient () + let reports = ConcurrentQueue() + + let reporter = + new SharedProgressReporter( + "Typechecking", + (fun () -> + let report = new ServerProgressReport(client, cancellableDefault = true) + reports.Enqueue report + report), + isEnabled = fun () -> isEnabled + ) + + { + Reporter = reporter + Calls = calls + Reports = reports + } + +let private dispose (d: IDisposable) = d.Dispose() + +/// The message and percentage the client shows after the given calls. +let private lastState calls = + match List.last calls with + | Begin(_, message, percentage) + | Report(message, percentage) -> Some(message, percentage) + | End -> None + +let operationTests = + testList + "Operations" + [ + testCaseAsync "One operation begins and ends a report" + <| async { + let t = createReporter true + let scope = t.Reporter.Begin "Foo.fs" + let! calls = t.Flush() + Expect.equal calls [ Begin("Typechecking", Some "Foo.fs", None) ] "The report should begin with the label" + + dispose scope + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the operation" + Expect.equal t.Reports.Count 1 "One report should be created" + } + + testCaseAsync "Operations that start after the previous one ended use a new report" + <| async { + let t = createReporter true + let first = t.Reporter.Begin "A.fs" + let! _ = t.Flush() + dispose first + let! _ = t.Flush() + + let second = t.Reporter.Begin "B.fs" + let! _ = t.Flush() + dispose second + let! calls = t.Flush() + + Expect.equal + calls + [ + Begin("Typechecking", Some "A.fs", None) + End + Begin("Typechecking", Some "B.fs", None) + End + ] + "Each operation should have its own report cycle" + + Expect.equal t.Reports.Count 2 "Two reports should be created" + } + + testCaseAsync "Concurrent operations share one report until the last ends" + <| async { + let t = createReporter true + let a = t.Reporter.Begin "A.fs" + let b = t.Reporter.Begin "B.fs" + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "A.fs, B.fs", None)) "Both labels should show" + + dispose a + let! calls = t.Flush() + Expect.equal (List.last calls) (Report(Some "B.fs", None)) "Only the running operation should show" + + dispose b + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the last operation" + Expect.equal t.Reports.Count 1 "One report should be shared" + } + + testCaseAsync "Operations with the same label keep the report open until both end" + <| async { + let t = createReporter true + let first = t.Reporter.Begin "Foo.fs" + let second = t.Reporter.Begin "Foo.fs" + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "Foo.fs", None)) "The label should show once" + + dispose first + let! calls = t.Flush() + Expect.isFalse (List.contains End calls) "The report should stay open for the second operation" + + dispose second + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the second operation" + } + + testCaseAsync "Disposing a scope twice ends the operation once" + <| async { + let t = createReporter true + let first = t.Reporter.Begin "Foo.fs" + let second = t.Reporter.Begin "Foo.fs" + dispose first + dispose first + let! calls = t.Flush() + Expect.isFalse (List.contains End calls) "The second operation still runs" + dispose second + } + + testCaseAsync "More than three operations show the first three and a count" + <| async { + let t = createReporter true + let scopes = [ "A.fs"; "B.fs"; "C.fs"; "D.fs"; "E.fs" ] |> List.map t.Reporter.Begin + let! calls = t.Flush() + + Expect.equal (lastState calls) (Some(Some "A.fs, B.fs, C.fs (+2 more)", None)) "The message should be truncated" + + scopes |> List.iter dispose + } + + testCaseAsync "Disposing the reporter ends the shown report" + <| async { + let t = createReporter true + let _scope = t.Reporter.Begin "Foo.fs" + let! _ = t.Flush() + dispose t.Reporter + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end" + + let _afterDispose = t.Reporter.Begin "Bar.fs" + let _batchAfterDispose = t.Reporter.BeginBatch 2 + let! callsAfter = t.Flush() + Expect.equal callsAfter calls "A disposed reporter should start no new report" + Expect.equal t.Reports.Count 1 "No report should be created after dispose" + } + ] + +let batchTests = + testList + "Batches" + [ + testCaseAsync "A batch shows how many of its operations completed" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 4 + let! calls = t.Flush() + Expect.equal calls [ Begin("Typechecking", Some "0/4 completed", Some 0u) ] "The batch should begin at 0%" + + let first = batch.Begin() + let! calls = t.Flush() + Expect.equal calls.Length 1 "A batch operation has no label of its own" + + let work = t.Reporter.Begin "A.fs" + let! calls = t.Flush() + + Expect.equal (List.last calls) (Report(Some "A.fs (0/4 completed)", Some 0u)) "The running work should show" + + dispose work + dispose first + let! calls = t.Flush() + Expect.equal (List.last calls) (Report(Some "1/4 completed", Some 25u)) "One of four operations completed" + + dispose batch + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the batch" + } + + testCaseAsync "Operations outside the batch do not count towards it" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 2 + let outside = t.Reporter.Begin "Outside.fs" + dispose outside + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "0/2 completed", Some 0u)) "Only batch operations should count" + + dispose batch + } + + testCaseAsync "Batch operations count separately" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 2 + let first = batch.Begin() + let second = batch.Begin() + dispose first + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "1/2 completed", Some 50u)) "One operation completed" + + dispose second + let! calls = t.Flush() + Expect.equal (List.last calls) (Report(Some "2/2 completed", Some 100u)) "Both operations completed" + dispose batch + } + + testCaseAsync "Overlapping batches add up" + <| async { + let t = createReporter true + let first = t.Reporter.BeginBatch 2 + let second = t.Reporter.BeginBatch 2 + first.Begin() |> dispose + second.Begin() |> dispose + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "2/4 completed", Some 50u)) "Both batches should count" + + dispose first + dispose second + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the last batch" + } + + testCaseAsync "A new batch starts from zero when the previous one ended" + <| async { + let t = createReporter true + // Keeps the report open between the two batches. + let keepOpen = t.Reporter.Begin "Open.fs" + let first = t.Reporter.BeginBatch 2 + first.Begin() |> dispose + dispose first + let! callsBefore = t.Flush() + + let second = t.Reporter.BeginBatch 3 + let! calls = t.Flush() + let newCalls = calls |> List.skip callsBefore.Length + + Expect.equal + newCalls + [ Report(Some "Open.fs (0/3 completed)", Some 0u) ] + "The second batch should not keep the counts of the first" + + dispose second + dispose keepOpen + } + + testCaseAsync "A batch that ends early stops counting its operations that did not complete" + <| async { + let t = createReporter true + let aborted = t.Reporter.BeginBatch 3 + let running = t.Reporter.BeginBatch 2 + aborted.Begin() |> dispose + let stillRunning = aborted.Begin() + dispose aborted + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "1/3 completed", Some 33u)) "Only the completed operation should stay" + + dispose stillRunning + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "1/3 completed", Some 33u)) "A late operation should not count" + + running.Begin() |> dispose + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "2/3 completed", Some 66u)) "The running batch should count" + dispose running + } + + testCaseAsync "An empty batch shows nothing" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 0 + let! calls = t.Flush() + Expect.isEmpty calls "No progress should be sent" + Expect.equal t.Reports.Count 0 "No report should be created" + dispose batch + } + ] + +let cancellationTests = + testList + "Cancellation" + [ + testCaseAsync "The scope token is cancelled when the user cancels the report" + <| async { + let t = createReporter true + let scope = t.Reporter.Begin "Foo.fs" + Expect.isFalse scope.CancellationToken.IsCancellationRequested "The token should not start cancelled" + t.Reports.ToArray().[0].Cancel() + Expect.isTrue scope.CancellationToken.IsCancellationRequested "The token should follow the report" + dispose scope + } + + testCaseAsync "An operation that starts after a cancel gets a new report and token" + <| async { + let t = createReporter true + let cancelled = t.Reporter.Begin "A.fs" + let! _ = t.Flush() + t.Reports.ToArray().[0].Cancel() + + let fresh = t.Reporter.Begin "B.fs" + Expect.isFalse fresh.CancellationToken.IsCancellationRequested "The new operation should not be cancelled" + Expect.equal t.Reports.Count 2 "A new report should be created" + + let! calls = t.Flush() + + Expect.equal + calls + [ + Begin("Typechecking", Some "A.fs", None) + End + Begin("Typechecking", Some "B.fs", None) + ] + "The cancelled report should end and the new one begin" + + dispose cancelled + let! calls = t.Flush() + Expect.equal (List.last calls) (Begin("Typechecking", Some "B.fs", None)) "The old operation is not shown" + + dispose fresh + let! calls = t.Flush() + Expect.equal (List.last calls) End "The new report should end" + } + + testCaseAsync "Batch operations that start after a cancel get the cancelled token" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 2 + let! _ = t.Flush() + t.Reports.ToArray().[0].Cancel() + + let operation = batch.Begin() + Expect.isTrue operation.CancellationToken.IsCancellationRequested "The batch was cancelled" + Expect.equal t.Reports.Count 1 "No new report should be created for the batch" + + dispose operation + dispose batch + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the batch" + } + ] + +let disabledTests = + testList + "Disabled" + [ + testCaseAsync "A disabled reporter sends nothing and never cancels" + <| async { + let t = createReporter false + let scope = t.Reporter.Begin "Foo.fs" + let batch = t.Reporter.BeginBatch 2 + let operation = batch.Begin() + + Expect.equal scope.CancellationToken Threading.CancellationToken.None "No token for an operation" + Expect.equal batch.CancellationToken Threading.CancellationToken.None "No token for a batch" + Expect.equal operation.CancellationToken Threading.CancellationToken.None "No token for a batch operation" + + dispose operation + dispose batch + dispose scope + let! calls = t.Flush() + Expect.isEmpty calls "No progress should be sent" + Expect.equal t.Reports.Count 0 "No report should be created" + } + ] + +let tests = + testList "SharedProgressReporter" [ operationTests; batchTests; cancellationTests; disabledTests ] diff --git a/test/FsAutoComplete.Tests.Lsp/SharedTypecheckProgressReporterTests.fs b/test/FsAutoComplete.Tests.Lsp/SharedTypecheckProgressReporterTests.fs deleted file mode 100644 index fbc2d9b33..000000000 --- a/test/FsAutoComplete.Tests.Lsp/SharedTypecheckProgressReporterTests.fs +++ /dev/null @@ -1,795 +0,0 @@ -module FsAutoComplete.Tests.SharedTypecheckProgressReporterTests - -open Expecto -open System -open System.Threading -open System.Threading.Tasks -open System.Collections.Concurrent -open FsAutoComplete.Lsp -open Ionide.LanguageServerProtocol -open Ionide.LanguageServerProtocol.Server -open Ionide.LanguageServerProtocol.Types -open Ionide.LanguageServerProtocol.JsonRpc - -/// Captured progress call from the mock client -type ProgressCall = - | Begin of title: string * message: string option * percentage: uint32 option - | Report of message: string option * percentage: uint32 option - | End of message: string option - -/// Creates a mock FSharpLspClient that captures all progress notifications -let private createMockClient () = - let progressCalls = ConcurrentQueue() - - let notificationSender: ClientNotificationSender = - fun methodName payload -> - match methodName with - | "$/progress" -> - let p = payload :?> ProgressParams - // Parse the progress value to determine the call type - let json = p.Value.ToString() - - if json.Contains("\"kind\":\"begin\"") || json.Contains("\"kind\": \"begin\"") then - // Extract title from JSON - let titleStart = json.IndexOf("\"title\"") + "\"title\"".Length - let colonAfterTitle = json.IndexOf(":", titleStart) + 1 - let titleQuoteStart = json.IndexOf("\"", colonAfterTitle) + 1 - let titleQuoteEnd = json.IndexOf("\"", titleQuoteStart) - let title = json.Substring(titleQuoteStart, titleQuoteEnd - titleQuoteStart) - - let message = - if json.Contains("\"message\"") then - let msgStart = json.IndexOf("\"message\"") + "\"message\"".Length - let colonAfterMsg = json.IndexOf(":", msgStart) + 1 - let msgQuoteStart = json.IndexOf("\"", colonAfterMsg) + 1 - let msgQuoteEnd = json.IndexOf("\"", msgQuoteStart) - Some(json.Substring(msgQuoteStart, msgQuoteEnd - msgQuoteStart)) - else - None - - let percentage = - if json.Contains("\"percentage\"") then - let pctStart = json.IndexOf("\"percentage\"") + "\"percentage\"".Length - let colonAfterPct = json.IndexOf(":", pctStart) + 1 - // Find the number value after the colon - let numStr = - json.Substring(colonAfterPct).TrimStart() - |> fun s -> - let endIdx = s.IndexOfAny([| ','; '}'; ' ' |]) - - if endIdx > 0 then s.Substring(0, endIdx) else s - - match UInt32.TryParse(numStr) with - | true, v -> Some v - | _ -> None - else - None - - progressCalls.Enqueue(Begin(title, message, percentage)) - elif json.Contains("\"kind\":\"report\"") || json.Contains("\"kind\": \"report\"") then - let message = - if json.Contains("\"message\"") then - let msgStart = json.IndexOf("\"message\"") + "\"message\"".Length - let colonAfterMsg = json.IndexOf(":", msgStart) + 1 - let msgQuoteStart = json.IndexOf("\"", colonAfterMsg) + 1 - let msgQuoteEnd = json.IndexOf("\"", msgQuoteStart) - Some(json.Substring(msgQuoteStart, msgQuoteEnd - msgQuoteStart)) - else - None - - let percentage = - if json.Contains("\"percentage\"") then - let pctStart = json.IndexOf("\"percentage\"") + "\"percentage\"".Length - let colonAfterPct = json.IndexOf(":", pctStart) + 1 - - let numStr = - json.Substring(colonAfterPct).TrimStart() - |> fun s -> - let endIdx = s.IndexOfAny([| ','; '}'; ' ' |]) - - if endIdx > 0 then s.Substring(0, endIdx) else s - - match UInt32.TryParse(numStr) with - | true, v -> Some v - | _ -> None - else - None - - progressCalls.Enqueue(Report(message, percentage)) - elif json.Contains("\"kind\":\"end\"") || json.Contains("\"kind\": \"end\"") then - let message = - if json.Contains("\"message\"") then - let msgStart = json.IndexOf("\"message\"") + "\"message\"".Length - let colonAfterMsg = json.IndexOf(":", msgStart) + 1 - let msgQuoteStart = json.IndexOf("\"", colonAfterMsg) + 1 - let msgQuoteEnd = json.IndexOf("\"", msgQuoteStart) - Some(json.Substring(msgQuoteStart, msgQuoteEnd - msgQuoteStart)) - else - None - - progressCalls.Enqueue(End message) - | _ -> () - - AsyncLspResult.success () - - let requestSender = - { new ClientRequestSender with - member _.Send _name _payload = AsyncLspResult.success Unchecked.defaultof<_> } - - let client = new FSharpLspClient(notificationSender, requestSender) - - client.ClientCapabilities <- - Some - { Workspace = None - TextDocument = None - Experimental = None - Window = - Some - { WorkDoneProgress = Some true - ShowMessage = None - ShowDocument = None } - General = None - NotebookDocument = None } - - client, progressCalls - -/// Helper: create a reporter with a mock client, returning (reporter, capturedCalls, reportCount) -let private createReporter (title: string) = - let client, calls = createMockClient () - let reportCount = ref 0 - - let reporter = - new SharedTypecheckProgressReporter( - title, - fun () -> - Interlocked.Increment(reportCount) |> ignore - new ServerProgressReport(client) - ) - - reporter, calls, reportCount - -/// Wait briefly for fire-and-forget tasks to settle -let private settle () = Task.Delay(50) |> Async.AwaitTask - -let private lastPercentage (calls: ConcurrentQueue) = - calls.ToArray() - |> Array.rev - |> Array.tryPick (function - | Report(_, Some percentage) -> Some percentage - | _ -> None) - - -let singleFileTests = - testList - "Single File" - [ testCaseAsync "Begin creates report and End disposes it" - <| async { - let reporter, calls, reportCount = createReporter "Typechecking" - - use reporter = reporter - - let! disp = reporter.Begin ("C:/src/Foo.fs") CancellationToken.None |> Async.AwaitTask - - do! settle () - - Expect.equal reportCount.Value 1 "Should create one report" - Expect.isGreaterThanOrEqual (calls.Count) 1 "Should have at least a Begin call" - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.isFalse (ct = CancellationToken.None) "CancellationToken should be active during file tracking" - - do! disp.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.isTrue (ct = CancellationToken.None) "CancellationToken should be None after all files end" - } - - testCaseAsync "Sequential files create separate report cycles" - <| async { - let reporter, _calls, reportCount = createReporter "Typechecking" - - use reporter = reporter - - // File 1 - let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - do! settle () - do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - Expect.equal reportCount.Value 1 "First file creates one report" - - // File 2 — new report cycle - let! disp2 = reporter.Begin ("C:/src/B.fs") CancellationToken.None |> Async.AwaitTask - do! settle () - do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - Expect.equal reportCount.Value 2 "Second file after first ended creates new report" - } - - testCaseAsync "Concurrent files keep report alive until all end" - <| async { - let reporter, _calls, reportCount = createReporter "Typechecking" - - use reporter = reporter - - let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - let! disp2 = reporter.Begin ("C:/src/B.fs") CancellationToken.None |> Async.AwaitTask - do! settle () - - Expect.equal reportCount.Value 1 "Concurrent files share one report" - - // End first file — report should still be active - do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.isFalse (ct = CancellationToken.None) "Report should still be active with one file remaining" - - // End second file — report should end - do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.isTrue (ct = CancellationToken.None) "Report should end after all files complete" - } - - testCaseAsync "Concurrent scopes for the same path keep the report active" - <| async { - let reporter, _calls, _reportCount = createReporter "Typechecking" - use reporter = reporter - - let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - let! disp2 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - - do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.notEqual ct CancellationToken.None "The second scope must keep the report active" - - do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.equal ct CancellationToken.None "The report must end after both scopes end" - } - - testCaseAsync "Files with the same basename keep independent scopes" - <| async { - let reporter, _calls, _reportCount = createReporter "Typechecking" - use reporter = reporter - - let! disp1 = reporter.Begin ("C:/src/A/Foo.fs") CancellationToken.None |> Async.AwaitTask - let! disp2 = reporter.Begin ("C:/src/B/Foo.fs") CancellationToken.None |> Async.AwaitTask - - do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.notEqual ct CancellationToken.None "The second path must keep the report active" - - do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.equal ct CancellationToken.None "The report must end after both paths complete" - } ] - - -let batchTests = - testList - "Batch" - [ testCaseAsync "BeginBatch creates report eagerly for CancellationToken access" - <| async { - let reporter, _calls, reportCount = createReporter "Typechecking" - - use reporter = reporter - - let! batchDisp = - reporter.BeginBatch ([| "C:/src/A.fs"; "C:/src/B.fs" |]) CancellationToken.None - |> Async.AwaitTask - - do! settle () - - Expect.equal reportCount.Value 1 "BeginBatch should eagerly create a report" - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.isFalse (ct = CancellationToken.None) "CancellationToken should be available after BeginBatch" - - do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - } - - testCaseAsync "Batch tracks completion percentage for batch files only" - <| async { - let reporter, calls, _reportCount = createReporter "Typechecking" - - use reporter = reporter - - let! batchDisp = - reporter.BeginBatch ([| "C:/src/A.fs"; "C:/src/B.fs"; "C:/src/C.fs" |]) CancellationToken.None - |> Async.AwaitTask - - do! settle () - - // Process batch file A - let! dispA = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - do! settle () - do! dispA.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - // After A completes: 1/3 = 33% - let lastReport = - calls.ToArray() - |> Array.tryFindBack (fun c -> - match c with - | Report(_, Some _) -> true - | _ -> false) - - match lastReport with - | Some(Report(_, Some pct)) -> Expect.equal pct 33u "After 1/3 batch files complete, percentage should be 33%" - | _ -> failtest "Expected a Report call with percentage after first batch file completes" - - // Process non-batch file — should NOT affect batch percentage - let! dispX = reporter.Begin ("C:/src/X.fs") CancellationToken.None |> Async.AwaitTask - do! settle () - do! dispX.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - // Process batch file B - let! dispB = reporter.Begin ("C:/src/B.fs") CancellationToken.None |> Async.AwaitTask - do! settle () - do! dispB.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - // After B completes: 2/3 = 66% - let lastReport2 = - calls.ToArray() - |> Array.tryFindBack (fun c -> - match c with - | Report(_, Some pct) when pct >= 60u -> true - | _ -> false) - - match lastReport2 with - | Some(Report(_, Some pct)) -> Expect.equal pct 66u "After 2/3 batch files complete, percentage should be 66%" - | _ -> failtest "Expected a Report call with ~66% after second batch file completes" - - do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - } - - testCaseAsync "Batch counts duplicate paths independently" - <| async { - let reporter, calls, _reportCount = createReporter "Typechecking" - use reporter = reporter - - let! batchDisp = - reporter.BeginBatch ([| "C:/src/A.fs"; "C:/src/A.fs" |]) CancellationToken.None - |> Async.AwaitTask - - let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - let! disp2 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - - do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let firstPercentage = lastPercentage calls - - Expect.equal firstPercentage (Some 50u) "The first duplicate completion must report 50 percent" - - do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let secondPercentage = lastPercentage calls - - Expect.equal secondPercentage (Some 100u) "The second duplicate completion must report 100 percent" - - do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask - } - - testCaseAsync "Overlapping batches count the same path independently" - <| async { - let reporter, calls, _reportCount = createReporter "Typechecking" - use reporter = reporter - - let! batchDisp1 = - reporter.BeginBatch ([| "C:/src/A.fs" |]) CancellationToken.None - |> Async.AwaitTask - - let! batchDisp2 = - reporter.BeginBatch ([| "C:/src/A.fs" |]) CancellationToken.None - |> Async.AwaitTask - - let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - let! disp2 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - - do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let firstPercentage = lastPercentage calls - - Expect.equal firstPercentage (Some 50u) "The first overlapping completion must report 50 percent" - - do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let secondPercentage = lastPercentage calls - - Expect.equal secondPercentage (Some 100u) "The second overlapping completion must report 100 percent" - - do! batchDisp2.DisposeAsync().AsTask() |> Async.AwaitTask - do! batchDisp1.DisposeAsync().AsTask() |> Async.AwaitTask - } - - testCaseAsync "Batch with active file scopes keeps report alive" - <| async { - let reporter, _calls, _reportCount = createReporter "Typechecking" - - use reporter = reporter - - let! batchDisp = - reporter.BeginBatch ([| "C:/src/A.fs" |]) CancellationToken.None - |> Async.AwaitTask - - let! fileDisp = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - - do! settle () - - // End batch scope but file is still active - do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.isFalse (ct = CancellationToken.None) "Report should remain active while file scope is open" - - // End file scope - do! fileDisp.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.isTrue (ct = CancellationToken.None) "Report should end after file and batch both complete" - } - - testCaseAsync "EndBatch resets batch counters" - <| async { - let reporter, calls, _reportCount = createReporter "Typechecking" - - use reporter = reporter - - // First batch - let! batchDisp1 = - reporter.BeginBatch ([| "C:/src/A.fs"; "C:/src/B.fs" |]) CancellationToken.None - |> Async.AwaitTask - - let! dispA = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - do! settle () - do! dispA.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - do! batchDisp1.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - // Second batch — counters should have been reset - let callCountBeforeSecondBatch = calls.Count - - let! batchDisp2 = - reporter.BeginBatch ([| "C:/src/C.fs"; "C:/src/D.fs" |]) CancellationToken.None - |> Async.AwaitTask - - do! settle () - - // The Begin for second batch should show 0% (0/2) - let callsArr = calls.ToArray() |> Array.skip callCountBeforeSecondBatch - - let hasZeroPercentage = - callsArr - |> Array.exists (fun c -> - match c with - | Begin(_, _, Some 0u) -> true - | Report(_, Some 0u) -> true - | _ -> false) - - Expect.isTrue hasZeroPercentage "Second batch should start at 0% after first batch ended" - - do! batchDisp2.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - } ] - - -let batchSyncTests = - testList - "BeginBatchSync" - [ testCaseAsync "BeginBatchSync adds files to active tracking and sets up batch" - <| async { - let reporter, calls, reportCount = createReporter "Loading Projects" - use reporter = reporter - - let disp = - reporter.BeginBatchSync([| "C:/src/Proj1.fsproj"; "C:/src/Proj2.fsproj"; "C:/src/Proj3.fsproj" |]) - - // Give fire-and-forget tasks time to complete - do! settle () - do! settle () - - Expect.equal reportCount.Value 1 "Should create one report" - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.isFalse (ct = CancellationToken.None) "Report should be active after BeginBatchSync" - - // Check that a Begin call was made with the title - let callsArr = calls.ToArray() - - let hasBegin = - callsArr - |> Array.exists (fun c -> - match c with - | Begin("Loading Projects", _, _) -> true - | _ -> false) - - Expect.isTrue hasBegin "Should have a Begin call with the title" - - // Check that file names appear in report messages - let hasFileNames = - callsArr - |> Array.exists (fun c -> - match c with - | Report(Some msg, _) -> - msg.Contains("Proj1.fsproj") - || msg.Contains("Proj2.fsproj") - || msg.Contains("Proj3.fsproj") - | _ -> false) - - Expect.isTrue hasFileNames "Report messages should contain project file names" - - disp.Dispose() - do! settle () - do! settle () - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.isTrue (ct = CancellationToken.None) "Report should end after Dispose" - } - - testCaseAsync "BeginBatchSync Dispose ends all file tracking and batch" - <| async { - let reporter, calls, _reportCount = createReporter "Loading Projects" - use reporter = reporter - - let disp = reporter.BeginBatchSync([| "C:/src/A.fsproj"; "C:/src/B.fsproj" |]) - do! settle () - do! settle () - - disp.Dispose() - do! settle () - do! settle () - - // After dispose, an End call should have been made - let callsArr = calls.ToArray() - - let hasEnd = - callsArr - |> Array.exists (fun c -> - match c with - | End _ -> true - | _ -> false) - - Expect.isTrue hasEnd "Should have an End call after Dispose" - } ] - - -let messageFormattingTests = - testList - "Message Formatting" - [ testCaseAsync "Single file shows just the filename" - <| async { - let reporter, calls, _reportCount = createReporter "Typechecking" - use reporter = reporter - - let! disp = reporter.Begin ("C:/src/Foo.fs") CancellationToken.None |> Async.AwaitTask - - do! settle () - - let callsArr = calls.ToArray() - - let beginMsg = - callsArr - |> Array.tryPick (fun c -> - match c with - | Begin(_, msg, _) -> msg - | _ -> None) - - Expect.equal beginMsg (Some "Foo.fs") "Single file message should be just the filename" - - do! disp.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - } - - testCaseAsync "Two concurrent files shows both filenames" - <| async { - let reporter, calls, _reportCount = createReporter "Typechecking" - use reporter = reporter - - let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - do! settle () - let! disp2 = reporter.Begin ("C:/src/B.fs") CancellationToken.None |> Async.AwaitTask - do! settle () - - let callsArr = calls.ToArray() - - let hasMultipleFiles = - callsArr - |> Array.exists (fun c -> - match c with - | Report(Some msg, _) -> msg.Contains("A.fs") && msg.Contains("B.fs") - | _ -> false) - - Expect.isTrue hasMultipleFiles "Message should contain both file names" - - do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask - do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - } - - testCaseAsync "More than 3 concurrent files truncates with (+N more)" - <| async { - let reporter, calls, _reportCount = createReporter "Typechecking" - use reporter = reporter - - let! disp1 = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - let! disp2 = reporter.Begin ("C:/src/B.fs") CancellationToken.None |> Async.AwaitTask - let! disp3 = reporter.Begin ("C:/src/C.fs") CancellationToken.None |> Async.AwaitTask - let! disp4 = reporter.Begin ("C:/src/D.fs") CancellationToken.None |> Async.AwaitTask - do! settle () - - let callsArr = calls.ToArray() - - let hasTruncation = - callsArr - |> Array.exists (fun c -> - match c with - | Report(Some msg, _) -> msg.Contains("(+1 more)") - | _ -> false) - - Expect.isTrue hasTruncation "4 files should show (+1 more) truncation" - - do! disp1.DisposeAsync().AsTask() |> Async.AwaitTask - do! disp2.DisposeAsync().AsTask() |> Async.AwaitTask - do! disp3.DisposeAsync().AsTask() |> Async.AwaitTask - do! disp4.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - } - - testCaseAsync "Batch file with active file shows combined message" - <| async { - let reporter, calls, _reportCount = createReporter "Typechecking" - use reporter = reporter - - let! batchDisp = - reporter.BeginBatch ([| "C:/src/A.fs"; "C:/src/B.fs" |]) CancellationToken.None - |> Async.AwaitTask - - let! fileDisp = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - - do! settle () - - let callsArr = calls.ToArray() - - // Should have a message with both file name and batch progress - let hasCombined = - callsArr - |> Array.exists (fun c -> - match c with - | Report(Some msg, _) -> msg.Contains("A.fs") && msg.Contains("0/2 completed") - | _ -> false) - - Expect.isTrue hasCombined "Message should contain filename and batch progress" - - do! fileDisp.DisposeAsync().AsTask() |> Async.AwaitTask - do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - } ] - - -let cancellationTokenTests = - testList - "CancellationToken" - [ testCaseAsync "GetCancellationToken returns None when no report is active" - <| async { - let reporter, _calls, _reportCount = createReporter "Typechecking" - use reporter = reporter - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.isTrue (ct = CancellationToken.None) "Should return None when no report is active" - } - - testCaseAsync "GetCancellationToken returns token from active report" - <| async { - let reporter, _calls, _reportCount = createReporter "Typechecking" - use reporter = reporter - - let! disp = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - do! settle () - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.isFalse (ct = CancellationToken.None) "Should return active token" - Expect.isFalse ct.IsCancellationRequested "Token should not be cancelled" - - do! disp.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - } - - testCaseAsync "GetCancellationToken returns token from batch-created report" - <| async { - let reporter, _calls, _reportCount = createReporter "Typechecking" - use reporter = reporter - - let! batchDisp = - reporter.BeginBatch ([| "C:/src/A.fs" |]) CancellationToken.None - |> Async.AwaitTask - - do! settle () - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.isFalse (ct = CancellationToken.None) "BeginBatch should eagerly create report with token" - - do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - } ] - - -let disabledReporterTests = - testList - "Disabled reporter" - [ testCaseAsync "Does not create progress reports" - <| async { - let client, calls = createMockClient () - let enabled = ref false - let reportCount = ref 0 - - use reporter = - new SharedTypecheckProgressReporter( - "Typechecking", - (fun () -> - Interlocked.Increment(reportCount) |> ignore - new ServerProgressReport(client)), - isEnabled = (fun () -> enabled.Value) - ) - - let! fileDisp = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - - let! batchDisp = - reporter.BeginBatch ([| "C:/src/A.fs" |]) CancellationToken.None - |> Async.AwaitTask - - use _batchSyncDisp = reporter.BeginBatchSync([| "C:/src/A.fs" |]) - - do! fileDisp.DisposeAsync().AsTask() |> Async.AwaitTask - do! batchDisp.DisposeAsync().AsTask() |> Async.AwaitTask - do! settle () - - Expect.equal reportCount.Value 0 "The disabled reporter must not create a report" - Expect.equal calls.Count 0 "The disabled reporter must not send progress" - - let! ct = reporter.GetCancellationToken() |> Async.AwaitTask - Expect.equal ct CancellationToken.None "The disabled reporter must not create a cancellation token" - - enabled.Value <- true - - let! enabledDisp = reporter.Begin ("C:/src/A.fs") CancellationToken.None |> Async.AwaitTask - do! settle () - - Expect.equal reportCount.Value 1 "The enabled reporter must create a report" - - do! enabledDisp.DisposeAsync().AsTask() |> Async.AwaitTask - } ] - - -let tests = - testList - "SharedTypecheckProgressReporter" - [ singleFileTests - batchTests - batchSyncTests - messageFormattingTests - cancellationTokenTests - disabledReporterTests ] From 9b59154be141f276bc9a97ea0f7cc467a5b0f1db Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Wed, 28 Oct 2026 15:32:14 +0100 Subject: [PATCH 08/10] Cancel progress outside the lookup lock, dispose unseen reports ServerProgressLookup.Cancel ran the token callbacks while holding the progress table lock, while SharedProgressReporter takes that lock through createReport while holding its own. No callback disposes a scope synchronously today, but the opposite lock order was one such callback away from a deadlock. Cancel now removes the entry under the lock and cancels after releasing it. A report whose operations started and ended between two syncs of the agent was never sent, so the agent never disposed it either. The reporter now disposes such a report when it retires it. --- .../LspServers/AdaptiveServerState.fs | 20 +++++++++++-------- .../LspServers/FSharpLspClient.fs | 10 +++++++++- 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index 7c98fd83a..9ca7a6b10 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -88,15 +88,19 @@ type ServerProgressLookup() = /// See LSP Spec on WorkDoneProgress Cancel for more information. /// member x.Cancel(token: ProgressToken) = - lock progressTable (fun () -> - match progressTable.TryGetValue(token) with - | true, weakRef -> - match weakRef.TryGetTarget() with - | true, progress -> - progress.Cancel() + let progress = + lock progressTable (fun () -> + match progressTable.TryGetValue(token) with + | true, weakRef -> progressTable.Remove(token) |> ignore - | _ -> () - | _ -> ()) + + match weakRef.TryGetTarget() with + | true, progress -> Some progress + | _ -> None + | _ -> None) + + // Cancel outside the lock: the token callbacks may take other locks. + progress |> Option.iter (fun p -> p.Cancel()) interface IDisposable with member x.Dispose() = diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fs b/src/FsAutoComplete/LspServers/FSharpLspClient.fs index 1e4373f19..30c9189d5 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fs +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fs @@ -226,6 +226,8 @@ type SharedProgressGeneration(report: ServerProgressReport) = member val Batches = 0 with get, set member val BatchTotal = 0 with get, set member val BatchCompleted = 0 with get, set + /// Set by the agent once it read this generation as the wanted state. + member val Seen = false with get, set member x.IsIdle = Map.isEmpty x.Operations && x.Batches = 0 type SharedProgressRequest = @@ -344,6 +346,7 @@ type SharedProgressReporter(title: string, createReport: unit -> ServerProgressR current |> Option.map (fun generation -> + generation.Seen <- true generation.Report, buildMessage generation, calcPercentage generation)) return! sync shown wanted |> Async.AwaitTask @@ -383,7 +386,12 @@ type SharedProgressReporter(title: string, createReport: unit -> ServerProgressR /// Call while holding syncRoot. let retireIfIdle (generation: SharedProgressGeneration) = match current with - | Some c when obj.ReferenceEquals(c, generation) && generation.IsIdle -> current <- None + | Some c when obj.ReferenceEquals(c, generation) && generation.IsIdle -> + current <- None + + // The agent never saw this report, so it will not end and dispose it either. + if not generation.Seen then + (generation.Report :> IDisposable).Dispose() | _ -> () /// Call while holding syncRoot. onEnd is called while holding syncRoot. From c1344b1cd269faac252a7c618eb51b095eca77ff Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Wed, 28 Oct 2026 15:38:16 +0100 Subject: [PATCH 09/10] Keep the built-in analyzers of a file in one progress report The built-in analyzers run one at a time behind analyzersLocker. Each one opened its own scope after taking the lock and closed it before releasing it, so the shared "Analyzing" report ended between two analyzers and the next one started a new report. A cancel also only stopped the analyzer that was running. The analyzers of a file now run in one batch. Each analyzer joins the batch before it waits for the lock and shows its label while it runs. A cancel of the report stops the analyzers that did not run yet. --- .../LspServers/AdaptiveServerState.fs | 46 +++++++++++++------ .../SharedProgressReporterTests.fs | 21 +++++++++ 2 files changed, 53 insertions(+), 14 deletions(-) diff --git a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs index 9ca7a6b10..8bdb81cf6 100644 --- a/src/FsAutoComplete/LspServers/AdaptiveServerState.fs +++ b/src/FsAutoComplete/LspServers/AdaptiveServerState.fs @@ -552,16 +552,18 @@ type AdaptiveState let inline getSourceLine lineNo = (source: ISourceText).GetLineString(lineNo - 1) - let checkUnusedOpens = + let checkUnusedOpens (batch: SharedProgressBatch) = asyncEx { try let! ct = Async.CancellationToken + use _operation = batch.Begin() use! _l = analyzersLocker.LockAsync(ct) - use progress = analyzerProgressReporter.Begin($"Unused opens - {fileName}") + batch.CancellationToken.ThrowIfCancellationRequested() + use _running = analyzerProgressReporter.Begin($"Unused opens - {fileName}") let! unused = UnusedOpens.getUnusedOpens (tyRes.GetCheckResults, getSourceLine) - |> Async.withCancellation progress.CancellationToken + |> Async.withCancellation batch.CancellationToken do! triggerNotificationAndWait @@ -572,18 +574,20 @@ type AdaptiveState | e -> logger.error (Log.setMessage "checkUnusedOpens failed" >> Log.addExn e) } - let checkUnusedDeclarations = + let checkUnusedDeclarations (batch: SharedProgressBatch) = asyncEx { try let! ct = Async.CancellationToken + use _operation = batch.Begin() use! _l = analyzersLocker.LockAsync(ct) - use progress = analyzerProgressReporter.Begin($"Unused declarations - {fileName}") + batch.CancellationToken.ThrowIfCancellationRequested() + use _running = analyzerProgressReporter.Begin($"Unused declarations - {fileName}") let isScript = Utils.isAScript (filePathUntag) let! unused = UnusedDeclarations.getUnusedDeclarations (tyRes.GetCheckResults, isScript) - |> Async.withCancellation progress.CancellationToken + |> Async.withCancellation batch.CancellationToken let unused = unused |> Seq.toArray @@ -593,16 +597,18 @@ type AdaptiveState | e -> logger.error (Log.setMessage "checkUnusedDeclarations failed" >> Log.addExn e) } - let checkSimplifiedNames = + let checkSimplifiedNames (batch: SharedProgressBatch) = asyncEx { try let! ct = Async.CancellationToken + use _operation = batch.Begin() use! _l = analyzersLocker.LockAsync(ct) - use progress = analyzerProgressReporter.Begin($"Simplify names - {fileName}") + batch.CancellationToken.ThrowIfCancellationRequested() + use _running = analyzerProgressReporter.Begin($"Simplify names - {fileName}") let! simplified = SimplifyNames.getSimplifiableNames (tyRes.GetCheckResults, getSourceLine) - |> Async.withCancellation progress.CancellationToken + |> Async.withCancellation batch.CancellationToken let simplified = Array.ofSeq simplified do! triggerNotificationAndWait (NotificationEvent.SimplifyNames(filePath, simplified, file.Version)) ct @@ -611,17 +617,19 @@ type AdaptiveState | e -> logger.error (Log.setMessage "checkSimplifiedNames failed" >> Log.addExn e) } - let checkUnnecessaryParentheses = + let checkUnnecessaryParentheses (batch: SharedProgressBatch) = asyncEx { try let! ct = Async.CancellationToken + use _operation = batch.Begin() use! _l = analyzersLocker.LockAsync(ct) - use progress = analyzerProgressReporter.Begin($"Unnecessary parens - {fileName}") + batch.CancellationToken.ThrowIfCancellationRequested() + use _running = analyzerProgressReporter.Begin($"Unnecessary parens - {fileName}") let unnecessaryParentheses = (System.Collections.Generic.HashSet(comparer = Range.comparer), tyRes.GetAST) ||> ParsedInput.fold (fun ranges path node -> - progress.CancellationToken.ThrowIfCancellationRequested() + batch.CancellationToken.ThrowIfCancellationRequested() match node with | SyntaxNode.SynExpr(SynExpr.Paren(expr = inner; rightParenRange = Some _; range = range)) when @@ -638,7 +646,7 @@ type AdaptiveState | _ -> ranges) - progress.CancellationToken.ThrowIfCancellationRequested() + batch.CancellationToken.ThrowIfCancellationRequested() do! triggerNotificationAndWait @@ -675,7 +683,17 @@ type AdaptiveState checkUnnecessaryParentheses ] - async { do! analyzers |> Async.parallel75 |> Async.Ignore } + async { + // The analyzers run one at a time behind analyzersLocker. One batch keeps them in one progress report, + // and a cancel of that report stops the analyzers that did not run yet. + use batch = analyzerProgressReporter.BeginBatch(analyzers.Length) + + do! + analyzers + |> List.map (fun check -> check batch) + |> Async.parallel75 + |> Async.Ignore + } let tryUriCreate (s: string) = match Uri.TryCreate(s, UriKind.Absolute) with diff --git a/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs b/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs index 8211dc1a2..0345f5189 100644 --- a/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs @@ -346,6 +346,27 @@ let batchTests = dispose running } + testCaseAsync "A batch keeps one report across operations that run one after another" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 2 + + for label in [ "A.fs"; "B.fs" ] do + use _operation = batch.Begin() + use _running = t.Reporter.Begin label + let! _ = t.Flush() + () + + let! calls = t.Flush() + Expect.isFalse (List.contains End calls) "The report should stay open between the operations" + Expect.equal t.Reports.Count 1 "One report should be used" + Expect.equal (lastState calls) (Some(Some "2/2 completed", Some 100u)) "Both operations completed" + + dispose batch + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the batch" + } + testCaseAsync "An empty batch shows nothing" <| async { let t = createReporter true From 05041b2c9db9ed66d370594e4f7d93eb569db953 Mon Sep 17 00:00:00 2001 From: Florian Verdonck Date: Thu, 29 Oct 2026 10:30:36 +0100 Subject: [PATCH 10/10] Count running operations when the last progress batch ends A progress report cannot drop its percentage. When the last batch of a shared report ended while labelled operations still ran (an open file check, the external analyzers), the client kept the last percentage, usually 100%, until the report ended. The operations that still run now join the count when the last batch ends, and operations that start later count too. The percentage stays below 100% until the report ends. Counters no longer reset between batches of one report, so a new batch adds to the count. Add the changelog entry for 0.86.1, so merging the PR releases it. --- CHANGELOG.md | 6 ++ .../LspServers/FSharpLspClient.fs | 51 +++++++++++----- .../SharedProgressReporterTests.fs | 58 +++++++++++++++++-- 3 files changed, 93 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 890dc2548..60bf05a6a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## [0.86.1] - 2026-10-29 + +### Changed + +- [Fewer progress notifications](https://github.com/ionide/FsAutoComplete/pull/1545) - Typechecks share one "Typechecking" progress report, instead of one report per file. Its message lists the files being checked and, after a save, how many of the dependent files are done, with a percentage. Cancelling the report stops the checks of the dependent files that did not run yet. With `backgroundServiceProgress` on, the analyzers of a file share one "Analyzing" report the same way. (thanks @TheAngryByrd!) + ## [0.86.0] - 2026-10-23 ### Changed diff --git a/src/FsAutoComplete/LspServers/FSharpLspClient.fs b/src/FsAutoComplete/LspServers/FSharpLspClient.fs index 30c9189d5..ea1ea25b5 100644 --- a/src/FsAutoComplete/LspServers/FSharpLspClient.fs +++ b/src/FsAutoComplete/LspServers/FSharpLspClient.fs @@ -224,8 +224,11 @@ type SharedProgressGeneration(report: ServerProgressReport) = /// The running operations and their labels, in the order they started. member val Operations = Map.empty with get, set member val Batches = 0 with get, set - member val BatchTotal = 0 with get, set - member val BatchCompleted = 0 with get, set + /// The operations that count towards the percentage, and how many of them completed. + member val Total = 0 with get, set + member val Completed = 0 with get, set + /// The running operations that count towards Total outside of a batch. + member val Counted = Set.empty with get, set /// Set by the agent once it read this generation as the wanted state. member val Seen = false with get, set member x.IsIdle = Map.isEmpty x.Operations && x.Batches = 0 @@ -269,8 +272,8 @@ type SharedProgressReporter(title: string, createReport: unit -> ServerProgressR Some(if hidden > 0 then $"{shown} (+{hidden} more)" else shown) let batchPart = - if generation.BatchTotal > 0 then - Some $"{generation.BatchCompleted}/{generation.BatchTotal} completed" + if generation.Total > 0 then + Some $"{generation.Completed}/{generation.Total} completed" else None @@ -281,10 +284,8 @@ type SharedProgressReporter(title: string, createReport: unit -> ServerProgressR | None, None -> None let calcPercentage (generation: SharedProgressGeneration) = - if generation.BatchTotal > 0 then - min 100 (generation.BatchCompleted * 100 / generation.BatchTotal) - |> uint32 - |> Some + if generation.Total > 0 then + min 100 (generation.Completed * 100 / generation.Total) |> uint32 |> Some else None @@ -394,18 +395,33 @@ type SharedProgressReporter(title: string, createReport: unit -> ServerProgressR (generation.Report :> IDisposable).Dispose() | _ -> () + /// Call while holding syncRoot. + let countOperation (generation: SharedProgressGeneration) operationId = + generation.Counted <- Set.add operationId generation.Counted + generation.Total <- generation.Total + 1 + /// Call while holding syncRoot. onEnd is called while holding syncRoot. let startOperation (generation: SharedProgressGeneration) (label: string option) (onEnd: unit -> unit) = lastOperationId <- lastOperationId + 1L let operationId = lastOperationId generation.Operations <- Map.add operationId label generation.Operations + // After the last batch ended, the report still shows a percentage, so new operations count towards it. + if generation.Batches = 0 && generation.Total > 0 then + countOperation generation operationId + new SharedProgressScope( generation.Report.CancellationToken, fun () -> lock syncRoot (fun () -> generation.Operations <- Map.remove operationId generation.Operations - onEnd () + + if Set.contains operationId generation.Counted then + generation.Counted <- Set.remove operationId generation.Counted + generation.Completed <- generation.Completed + 1 + else + onEnd () + retireIfIdle generation) requestSync () @@ -442,7 +458,7 @@ type SharedProgressReporter(title: string, createReport: unit -> ServerProgressR else let generation = currentGeneration () generation.Batches <- generation.Batches + 1 - generation.BatchTotal <- generation.BatchTotal + count + generation.Total <- generation.Total + count Some generation) match generation with @@ -458,7 +474,7 @@ type SharedProgressReporter(title: string, createReport: unit -> ServerProgressR let completeOperation () = if not ended.Value then completed.Value <- completed.Value + 1 - generation.BatchCompleted <- generation.BatchCompleted + 1 + generation.Completed <- generation.Completed + 1 let beginOperation () = let scope = @@ -477,12 +493,15 @@ type SharedProgressReporter(title: string, createReport: unit -> ServerProgressR ended.Value <- true generation.Batches <- generation.Batches - 1 + // Operations of this batch that never ran no longer count. + generation.Total <- generation.Total - max 0 (count - completed.Value) + + // A report cannot drop its percentage, so the operations that still run count towards it. + // Otherwise the percentage would stay at its last value until the report ends. if generation.Batches = 0 then - generation.BatchTotal <- 0 - generation.BatchCompleted <- 0 - else - // Operations of this batch that never ran no longer count towards the batches still running. - generation.BatchTotal <- generation.BatchTotal - max 0 (count - completed.Value) + for operationId in generation.Operations.Keys do + if not (Set.contains operationId generation.Counted) then + countOperation generation operationId retireIfIdle generation) diff --git a/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs b/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs index 0345f5189..cf14b2cc3 100644 --- a/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs +++ b/test/FsAutoComplete.Tests.Lsp/SharedProgressReporterTests.fs @@ -302,7 +302,43 @@ let batchTests = Expect.equal (List.last calls) End "The report should end with the last batch" } - testCaseAsync "A new batch starts from zero when the previous one ended" + testCaseAsync "When the last batch ends, the operations that still run count towards the percentage" + <| async { + let t = createReporter true + let batch = t.Reporter.BeginBatch 2 + let running = t.Reporter.Begin "Open.fs" + batch.Begin() |> dispose + batch.Begin() |> dispose + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "Open.fs (2/2 completed)", Some 100u)) "The batch completed" + + dispose batch + let! calls = t.Flush() + + Expect.equal + (lastState calls) + (Some(Some "Open.fs (2/3 completed)", Some 66u)) + "The running operation should count, so the report does not stay at 100%" + + let late = t.Reporter.Begin "Late.fs" + let! calls = t.Flush() + + Expect.equal + (lastState calls) + (Some(Some "Open.fs, Late.fs (2/4 completed)", Some 50u)) + "An operation that starts after the batch ended should count too" + + dispose running + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "Late.fs (3/4 completed)", Some 75u)) "One more operation completed" + + dispose late + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the last operation" + Expect.equal t.Reports.Count 1 "One report should be used" + } + + testCaseAsync "A new batch adds to the count of a report that is still open" <| async { let t = createReporter true // Keeps the report open between the two batches. @@ -310,19 +346,29 @@ let batchTests = let first = t.Reporter.BeginBatch 2 first.Begin() |> dispose dispose first - let! callsBefore = t.Flush() + let! calls = t.Flush() + Expect.equal (lastState calls) (Some(Some "Open.fs (1/2 completed)", Some 50u)) "The first batch ended early" let second = t.Reporter.BeginBatch 3 let! calls = t.Flush() - let newCalls = calls |> List.skip callsBefore.Length Expect.equal - newCalls - [ Report(Some "Open.fs (0/3 completed)", Some 0u) ] - "The second batch should not keep the counts of the first" + (lastState calls) + (Some(Some "Open.fs (1/5 completed)", Some 20u)) + "The second batch should add to the count" + + second.Begin() |> dispose + let! calls = t.Flush() + + Expect.equal + (lastState calls) + (Some(Some "Open.fs (2/5 completed)", Some 40u)) + "An operation of the second batch completed" dispose second dispose keepOpen + let! calls = t.Flush() + Expect.equal (List.last calls) End "The report should end with the last operation" } testCaseAsync "A batch that ends early stops counting its operations that did not complete"