From f96b70e1c112af68b5e14fd75fca4bc6ca7308c2 Mon Sep 17 00:00:00 2001 From: Nat Elkins Date: Thu, 25 Jun 2026 14:25:37 -0400 Subject: [PATCH 1/5] Avoid leaking a MeterListener per Cache in DEBUG builds In DEBUG builds, every Cache instance created a CacheMetrics.CacheMetricsListener, which starts a System.Diagnostics.Metrics.MeterListener registered in the process-global metrics registry. These were never disposed, so they accumulated for the lifetime of the process. Because every cache hit/miss/add publishes a measurement to all registered listeners, the per-operation cost grew linearly with the number of leaked listeners, so workloads that create many caches (for example repeated ParseAndCheckProject / per-file checks) slowed down steadily. Track the per-cache totals used by DebugDisplay directly, incrementing a small Stats object alongside the existing global Meter counters, instead of via a per-cache MeterListener. No listener is created, so nothing leaks, and DebugDisplay still works. The now-unused CacheMetrics.Hit/Miss/Add/Update/Eviction/EvictionFail helpers are replaced by a single recordMetric helper. --- .../.FSharp.Compiler.Service/11.0.100.md | 1 + src/Compiler/Utilities/Caches.fs | 49 +++++++++++-------- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index ba843ca702c..1024e0eb30e 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,5 +1,6 @@ ### Fixed +* Stop leaking a `System.Diagnostics.Metrics.MeterListener` per `Cache` in DEBUG builds. Each cache created a `CacheMetricsListener` (which starts a `MeterListener` registered in the process-global metrics registry) and never disposed it, so listeners accumulated for the lifetime of the process. Because every cache hit/miss/add published to all registered listeners, the per-operation cost grew linearly with the number of leaked listeners, so repeated checks (and Debug FCS test runs) slowed down over time. The per-cache `DebugDisplay` totals are now tracked directly instead of via a `MeterListener`. ([PR #19995](https://github.com/dotnet/fsharp/pull/19995)) * Semantic classification no longer marks recursive object self-references (`as this`, `let rec` self-refs) as mutable. ([Issue #5229](https://github.com/dotnet/fsharp/issues/5229)) * Fix `MethodAccessException` under `--realsig+` when a closure (inner `let rec`, `task`/`async` state machine, or quotation splice) inside a member defined in an intrinsic type augmentation (`type C with member ...`) accesses a `private` member of `C`. The synthesized closure is now nested inside the declaring type instead of beside it in the module class. ([Issue #19933](https://github.com/dotnet/fsharp/issues/19933), [PR #19955](https://github.com/dotnet/fsharp/pull/19955)) * Preserve source range for type errors on empty-bodied computation expressions (e.g. `foo {}`) in pipelines, function arguments, and type-annotated contexts, instead of reporting `unknown(1,1)`. ([Issue #19550](https://github.com/dotnet/fsharp/issues/19550), [PR #19849](https://github.com/dotnet/fsharp/pull/19849)) diff --git a/src/Compiler/Utilities/Caches.fs b/src/Compiler/Utilities/Caches.fs index fb024844e09..17d0c8e2ac1 100644 --- a/src/Compiler/Utilities/Caches.fs +++ b/src/Compiler/Utilities/Caches.fs @@ -32,12 +32,6 @@ module CacheMetrics = tags.Add("cacheId", box cacheId) tags - let Add (tags: inref) = adds.Add(1L, &tags) - let Update (tags: inref) = updates.Add(1L, &tags) - let Hit (tags: inref) = hits.Add(1L, &tags) - let Miss (tags: inref) = misses.Add(1L, &tags) - let Eviction (tags: inref) = evictions.Add(1L, &tags) - let EvictionFail (tags: inref) = evictionFails.Add(1L, &tags) let Created (tags: inref) = creations.Add(1L, &tags) let Disposed (tags: inref) = disposals.Add(1L, &tags) @@ -279,6 +273,23 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke let tags = CacheMetrics.mkTags name + // Per-cache metric totals used by DebugDisplay. These are incremented directly (see recordMetric) + // rather than via a per-cache CacheMetrics.CacheMetricsListener: a listener starts a + // System.Diagnostics.Metrics.MeterListener which registers in the process-global metrics registry, + // so one per Cache would never be disposed (leak) and would make every measurement O(number of + // caches) as listeners accumulate. +#if DEBUG + let debugStats = CacheMetrics.Stats() +#endif + + // Publish one measurement to the global Meter counter (for --times / StatsToString) and, in DEBUG, + // record it in this cache's own totals for DebugDisplay. + let recordMetric (counter: Counter) = + counter.Add(1L, &tags) +#if DEBUG + debugStats.Incr counter.Name 1L +#endif + // Track disposal state (0 = not disposed, 1 = disposed) let mutable disposed = 0 @@ -310,10 +321,10 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke match store.TryRemove(first.Value.Key) with | true, _ -> - CacheMetrics.Eviction &tags + recordMetric CacheMetrics.evictions evicted.Trigger() | _ -> - CacheMetrics.EvictionFail &tags + recordMetric CacheMetrics.evictionFails evictionFailed.Trigger() deadKeysCount <- deadKeysCount + 1 @@ -361,10 +372,6 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke post, dispose -#if DEBUG - let debugListener = new CacheMetrics.CacheMetricsListener(tags) -#endif - do CacheMetrics.Created &tags member val Evicted = evicted.Publish @@ -373,12 +380,12 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke member _.TryGetValue(key: 'Key, value: outref<'Value>) = match store.TryGetValue(key) with | true, entity -> - CacheMetrics.Hit &tags + recordMetric CacheMetrics.hits post (EvictionQueueMessage.Update entity) value <- entity.Value true | _ -> - CacheMetrics.Miss &tags + recordMetric CacheMetrics.misses value <- Unchecked.defaultof<'Value> false @@ -388,7 +395,7 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke let added = store.TryAdd(key, entity) if added then - CacheMetrics.Add &tags + recordMetric CacheMetrics.adds post (EvictionQueueMessage.Add(entity, store)) added @@ -405,11 +412,11 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke if wasMiss then post (EvictionQueueMessage.Add(result, store)) - CacheMetrics.Add &tags - CacheMetrics.Miss &tags + recordMetric CacheMetrics.adds + recordMetric CacheMetrics.misses else post (EvictionQueueMessage.Update result) - CacheMetrics.Hit &tags + recordMetric CacheMetrics.hits result.Value @@ -424,10 +431,10 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke // Returned value tells us if the entity was added or updated. if Object.ReferenceEquals(addValue, result) then - CacheMetrics.Add &tags + recordMetric CacheMetrics.adds post (EvictionQueueMessage.Add(addValue, store)) else - CacheMetrics.Update &tags + recordMetric CacheMetrics.updates post (EvictionQueueMessage.Update result) member _.CreateMetricsListener() = @@ -447,5 +454,5 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke override this.Finalize() = this.Dispose() #if DEBUG - member _.DebugDisplay() = debugListener.ToString() + member _.DebugDisplay() = debugStats.ToString() #endif From 18f7fd5abcb8f606b8e6cce8ad782c058ee7aa14 Mon Sep 17 00:00:00 2001 From: Nat Elkins Date: Mon, 6 Jul 2026 20:25:14 -0400 Subject: [PATCH 2/5] Address review: drop per-cache CacheMetricsListener and cacheId tag - Remove the CacheMetrics.CacheMetricsListener type. Its only per-cache use was the #if DEBUG debugListener each Cache created and never disposed, which was the leak this PR set out to fix. (majocha) - Drop the per-instance cacheId tag (and nextCacheId). Measurements now carry only the cache name, shrinking the payload published to any connected exporter and removing the per-instance filtering that was cacheId's only purpose. (majocha) - DebugDisplay and the cache tests read the existing name-aggregated stats via CacheMetrics.getTotalsByName / getRatioByName, populated by the single process-wide ListenToAll listener. No per-cache listener is created and no per-operation cost is added in any configuration, so there is no DEBUG-only overhead left to gate behind a separate directive. (T-Gro) - Overload-cache tests enable ListenToAll and snapshot totals before/after to stay scoped to their own compilation; FSharpChecker .CreateOverloadCacheMetricsListener is removed. --- .../.FSharp.Compiler.Service/11.0.100.md | 2 +- src/Compiler/Service/service.fs | 3 - src/Compiler/Service/service.fsi | 3 - src/Compiler/Utilities/Caches.fs | 104 +++++------------- src/Compiler/Utilities/Caches.fsi | 25 ++--- .../CompilerService/Caches.fs | 47 ++++---- .../OverloadCacheTests.fs | 31 ++++-- 7 files changed, 78 insertions(+), 137 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md index 6442c200003..03d69baed43 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.100.md @@ -1,6 +1,6 @@ ### Fixed -* Stop leaking a `System.Diagnostics.Metrics.MeterListener` per `Cache` in DEBUG builds. Each cache created a `CacheMetricsListener` (which starts a `MeterListener` registered in the process-global metrics registry) and never disposed it, so listeners accumulated for the lifetime of the process. Because every cache hit/miss/add published to all registered listeners, the per-operation cost grew linearly with the number of leaked listeners, so repeated checks (and Debug FCS test runs) slowed down over time. The per-cache `DebugDisplay` totals are now tracked directly instead of via a `MeterListener`. ([PR #19995](https://github.com/dotnet/fsharp/pull/19995)) +* Stop leaking a `System.Diagnostics.Metrics.MeterListener` per `Cache` in DEBUG builds. Each cache created a `CacheMetrics.CacheMetricsListener` (which starts a `MeterListener` registered in the process-global metrics registry) and never disposed it, so listeners accumulated for the lifetime of the process. Because every cache hit/miss/add published to all registered listeners, the per-operation cost grew linearly with the number of leaked listeners, so repeated checks (and Debug FCS test runs) slowed down over time. The per-cache `CacheMetricsListener` and the per-instance `cacheId` tag are removed; `DebugDisplay` and tests now read the existing name-aggregated stats populated by the single `ListenToAll` listener, so no per-cache listener is created and no per-operation cost is added. ([PR #19995](https://github.com/dotnet/fsharp/pull/19995)) * Fix FS3236 when taking the address of an untyped generalized `let` binding (e.g. `let ffff = ValueNone`) passed to an `inref` parameter. ([Issue #19608](https://github.com/dotnet/fsharp/issues/19608), [PR #19948](https://github.com/dotnet/fsharp/pull/19948)) * Fix IntelliSense not suggesting later named arguments after the first one is autocompleted, for methods (including overloaded methods like `Task.Factory.StartNew`). ([Issue #19906](https://github.com/dotnet/fsharp/issues/19906), [PR #19940](https://github.com/dotnet/fsharp/pull/19940)) * Restore packaging of an F# design-time type provider that is activated via a `ProjectReference` carrying `IsFSharpDesignTimeProvider="true"`. The provider assembly is again included under `fsharp41` when packing (including `pack --no-build`); `PackageFSharpDesignTimeTools` now resolves the provider via `GetTargetPath`, which works in `dotnet pack`'s `BuildProjectReferences=false` content build without forcing an early `ResolveReferences`. ([Issue #18924](https://github.com/dotnet/fsharp/issues/18924), [PR #19979](https://github.com/dotnet/fsharp/pull/19979)) diff --git a/src/Compiler/Service/service.fs b/src/Compiler/Service/service.fs index c0dd6e21d09..3584ca61e49 100644 --- a/src/Compiler/Service/service.fs +++ b/src/Compiler/Service/service.fs @@ -623,9 +623,6 @@ type FSharpChecker static member Instance = globalInstance.Force() - static member internal CreateOverloadCacheMetricsListener() = - new CacheMetrics.CacheMetricsListener("overloadResolutionCache") - member internal _.FrameworkImportsCache = backgroundCompiler.FrameworkImportsCache /// Tokenize a single line, returning token information and a tokenization state represented by an integer diff --git a/src/Compiler/Service/service.fsi b/src/Compiler/Service/service.fsi index ae2b253c676..1584e19562b 100644 --- a/src/Compiler/Service/service.fsi +++ b/src/Compiler/Service/service.fsi @@ -506,9 +506,6 @@ type public FSharpChecker = [] static member Instance: FSharpChecker - /// Creates a listener for overload resolution cache metrics, aggregating across all compilations. - static member internal CreateOverloadCacheMetricsListener: unit -> CacheMetrics.CacheMetricsListener - member internal FrameworkImportsCache: FrameworkImportsCache member internal ReferenceResolver: LegacyReferenceResolver diff --git a/src/Compiler/Utilities/Caches.fs b/src/Compiler/Utilities/Caches.fs index 17d0c8e2ac1..9c69a1b4e36 100644 --- a/src/Compiler/Utilities/Caches.fs +++ b/src/Compiler/Utilities/Caches.fs @@ -22,16 +22,20 @@ module CacheMetrics = let creations = Meter.CreateCounter("creations", "count") let disposals = Meter.CreateCounter("disposals", "count") - let mutable private nextCacheId = 0 - let mkTags (name: string) = - let cacheId = Interlocked.Increment &nextCacheId // Avoid TagList(ReadOnlySpan<...>) to support net472 runtime + // Only the cache name is tagged: a per-instance id would be published on every measurement, + // inflating the tag payload sent to any connected exporter for no in-process benefit. let mutable tags = TagList() tags.Add("name", box name) - tags.Add("cacheId", box cacheId) tags + let Add (tags: inref) = adds.Add(1L, &tags) + let Update (tags: inref) = updates.Add(1L, &tags) + let Hit (tags: inref) = hits.Add(1L, &tags) + let Miss (tags: inref) = misses.Add(1L, &tags) + let Eviction (tags: inref) = evictions.Add(1L, &tags) + let EvictionFail (tags: inref) = evictionFails.Add(1L, &tags) let Created (tags: inref) = creations.Add(1L, &tags) let Disposed (tags: inref) = disposals.Add(1L, &tags) @@ -72,6 +76,10 @@ module CacheMetrics = let getStatsByName name = statsByName.GetOrAdd(name, fun _ -> Stats()) + let getTotalsByName name = (getStatsByName name).GetTotals() + + let getRatioByName name = (getStatsByName name).Ratio + let ListenToAll () = let listener = new MeterListener() @@ -117,50 +125,6 @@ module CacheMetrics = Console.WriteLine(StatsToString()) } - [] - type CacheMetricsListener(cacheTags: TagList, ?nameOnlyFilter: string) = - - let stats = Stats() - let listener = new MeterListener() - - do - for instrument in allCounters do - listener.EnableMeasurementEvents instrument - - listener.SetMeasurementEventCallback(fun instrument v tags _ -> - let shouldIncrement = - match nameOnlyFilter with - | Some filterName -> - match tags[0].Value with - | :? string as name when name = filterName -> true - | _ -> false - | None -> tags[0] = cacheTags[0] && tags[1] = cacheTags[1] - - if shouldIncrement then - stats.Incr instrument.Name v) - - listener.Start() - - /// Creates a listener that aggregates metrics across all cache instances with the given name. - new(cacheName: string) = new CacheMetricsListener(TagList(), nameOnlyFilter = cacheName) - - interface IDisposable with - member _.Dispose() = listener.Dispose() - - /// Gets the current totals for each metric type. - member _.GetTotals() = stats.GetTotals() - - /// Gets the current hit ratio (hits / (hits + misses)). - member _.Ratio = stats.Ratio - - /// Gets the total number of cache hits. - member _.Hits = stats.GetTotals().[hits.Name] - - /// Gets the total number of cache misses. - member _.Misses = stats.GetTotals().[misses.Name] - - override _.ToString() = stats.ToString() - [] type EvictionMode = | NoEviction @@ -273,23 +237,6 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke let tags = CacheMetrics.mkTags name - // Per-cache metric totals used by DebugDisplay. These are incremented directly (see recordMetric) - // rather than via a per-cache CacheMetrics.CacheMetricsListener: a listener starts a - // System.Diagnostics.Metrics.MeterListener which registers in the process-global metrics registry, - // so one per Cache would never be disposed (leak) and would make every measurement O(number of - // caches) as listeners accumulate. -#if DEBUG - let debugStats = CacheMetrics.Stats() -#endif - - // Publish one measurement to the global Meter counter (for --times / StatsToString) and, in DEBUG, - // record it in this cache's own totals for DebugDisplay. - let recordMetric (counter: Counter) = - counter.Add(1L, &tags) -#if DEBUG - debugStats.Incr counter.Name 1L -#endif - // Track disposal state (0 = not disposed, 1 = disposed) let mutable disposed = 0 @@ -321,10 +268,10 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke match store.TryRemove(first.Value.Key) with | true, _ -> - recordMetric CacheMetrics.evictions + CacheMetrics.Eviction &tags evicted.Trigger() | _ -> - recordMetric CacheMetrics.evictionFails + CacheMetrics.EvictionFail &tags evictionFailed.Trigger() deadKeysCount <- deadKeysCount + 1 @@ -380,12 +327,12 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke member _.TryGetValue(key: 'Key, value: outref<'Value>) = match store.TryGetValue(key) with | true, entity -> - recordMetric CacheMetrics.hits + CacheMetrics.Hit &tags post (EvictionQueueMessage.Update entity) value <- entity.Value true | _ -> - recordMetric CacheMetrics.misses + CacheMetrics.Miss &tags value <- Unchecked.defaultof<'Value> false @@ -395,7 +342,7 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke let added = store.TryAdd(key, entity) if added then - recordMetric CacheMetrics.adds + CacheMetrics.Add &tags post (EvictionQueueMessage.Add(entity, store)) added @@ -412,11 +359,11 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke if wasMiss then post (EvictionQueueMessage.Add(result, store)) - recordMetric CacheMetrics.adds - recordMetric CacheMetrics.misses + CacheMetrics.Add &tags + CacheMetrics.Miss &tags else post (EvictionQueueMessage.Update result) - recordMetric CacheMetrics.hits + CacheMetrics.Hit &tags result.Value @@ -431,15 +378,12 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke // Returned value tells us if the entity was added or updated. if Object.ReferenceEquals(addValue, result) then - recordMetric CacheMetrics.adds + CacheMetrics.Add &tags post (EvictionQueueMessage.Add(addValue, store)) else - recordMetric CacheMetrics.updates + CacheMetrics.Update &tags post (EvictionQueueMessage.Update result) - member _.CreateMetricsListener() = - new CacheMetrics.CacheMetricsListener(tags) - member _.Dispose() = if Interlocked.Exchange(&disposed, 1) = 0 then disposeEvictionProcessor () @@ -454,5 +398,7 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke override this.Finalize() = this.Dispose() #if DEBUG - member _.DebugDisplay() = debugStats.ToString() + // Shows the totals aggregated for this cache's name. Populated only while a metrics listener + // (CacheMetrics.ListenToAll, e.g. under --times or the editor's metrics view) is running. + member _.DebugDisplay() = (CacheMetrics.getStatsByName name).ToString() #endif diff --git a/src/Compiler/Utilities/Caches.fsi b/src/Compiler/Utilities/Caches.fsi index 3e1c98e9bb1..e0bff618fcb 100644 --- a/src/Compiler/Utilities/Caches.fsi +++ b/src/Compiler/Utilities/Caches.fsi @@ -8,25 +8,18 @@ module CacheMetrics = /// Global telemetry Meter for all caches. Exposed for testing purposes. /// Set FSHARP_OTEL_EXPORT environment variable to enable OpenTelemetry export to external collectors in tests. val Meter: Meter + + /// Current metric totals aggregated across all cache instances with the given name. + /// Totals only accumulate while a listener from ListenToAll is running. + val internal getTotalsByName: name: string -> Map + + /// Current hit ratio (hits / (hits + misses)) aggregated across all cache instances with the given name. + val internal getRatioByName: name: string -> float + val internal ListenToAll: unit -> IDisposable val internal StatsToString: unit -> string val internal CaptureStatsAndWriteToConsole: unit -> IDisposable - /// A listener that captures cache metrics, matching by cache name or exact cache tags. - [] - type CacheMetricsListener = - /// Creates a listener that aggregates metrics across all cache instances with the given name. - new: cacheName: string -> CacheMetricsListener - /// Gets the current totals for each metric type. - member GetTotals: unit -> Map - /// Gets the current hit ratio (hits / (hits + misses)). - member Ratio: float - /// Gets the total number of cache hits. - member Hits: int64 - /// Gets the total number of cache misses. - member Misses: int64 - interface IDisposable - [] type internal EvictionMode = /// Do not evict items, cache is effectively a ConcurrentDictionary. @@ -74,5 +67,3 @@ type internal Cache<'Key, 'Value when 'Key: not null> = member Evicted: IEvent /// For testing only. member EvictionFailed: IEvent - /// For testing only. Creates a local telemetry listener for this cache instance. - member CreateMetricsListener: unit -> CacheMetrics.CacheMetricsListener diff --git a/tests/FSharp.Compiler.ComponentTests/CompilerService/Caches.fs b/tests/FSharp.Compiler.ComponentTests/CompilerService/Caches.fs index b7aac72a93b..21462ce1961 100644 --- a/tests/FSharp.Compiler.ComponentTests/CompilerService/Caches.fs +++ b/tests/FSharp.Compiler.ComponentTests/CompilerService/Caches.fs @@ -28,8 +28,8 @@ let ``Create and dispose many`` () = [] let ``Basic add and retrieve`` () = let name = "Basic_add_and_retrieve" + use _ = CacheMetrics.ListenToAll() use cache = new Cache(defaultStructural(), name = name) - use metricsListener = cache.CreateMetricsListener() cache.TryAdd("key1", 1) |> shouldBeTrue cache.TryAdd("key2", 2) |> shouldBeTrue @@ -45,14 +45,14 @@ let ``Basic add and retrieve`` () = cache.TryGetValue("key3", &value) |> shouldBeFalse // Metrics assertions - let totals = metricsListener.GetTotals() + let totals = CacheMetrics.getTotalsByName name totals.["adds"] |> shouldEqual 2L [] let ``Eviction of least recently used`` () = let name = "Eviction_of_least_recently_used" + use _ = CacheMetrics.ListenToAll() use cache = new Cache({ defaultStructural() with TotalCapacity = 2; HeadroomPercentage = 0 }, name = name) - use metricsListener = cache.CreateMetricsListener() cache.TryAdd("key1", 1) |> shouldBeTrue cache.TryAdd("key2", 2) |> shouldBeTrue @@ -76,7 +76,7 @@ let ``Eviction of least recently used`` () = value |> shouldEqual 3 // Metrics assertions - let totals = metricsListener.GetTotals() + let totals = CacheMetrics.getTotalsByName name totals.["adds"] |> shouldEqual 3L [] @@ -85,14 +85,14 @@ let ``Stress test evictions`` () = let iterations = 10_000 let name = "Stress test evictions" + use _ = CacheMetrics.ListenToAll() use cache = new Cache({ defaultStructural() with TotalCapacity = cacheSize; HeadroomPercentage = 0 }, name = name) - use metricsListener = cache.CreateMetricsListener() let evictionsCompleted = new TaskCompletionSource() let expectedEvictions = iterations - cacheSize cache.Evicted.Add <| fun () -> - if metricsListener.GetTotals().["evictions"] = expectedEvictions then + if (CacheMetrics.getTotalsByName name).["evictions"] = expectedEvictions then evictionsCompleted.SetResult() cache.EvictionFailed.Add <| fun _ -> @@ -114,13 +114,14 @@ let ``Stress test evictions`` () = value |> shouldEqual iterations // Metrics assertions - let totals = metricsListener.GetTotals() + let totals = CacheMetrics.getTotalsByName name totals.["adds"] |> shouldEqual (int64 iterations) [] let ``Metrics can be retrieved`` () = - use cache = new Cache({ defaultStructural() with TotalCapacity = 2; HeadroomPercentage = 0 }, name = "test_metrics") - use metricsListener = cache.CreateMetricsListener() + let name = "test_metrics" + use _ = CacheMetrics.ListenToAll() + use cache = new Cache({ defaultStructural() with TotalCapacity = 2; HeadroomPercentage = 0 }, name = name) cache.TryAdd("key1", 1) |> shouldBeTrue cache.TryAdd("key2", 2) |> shouldBeTrue @@ -135,17 +136,17 @@ let ``Metrics can be retrieved`` () = cache.TryAdd("key3", 3) |> shouldBeTrue evictionCompleted.Task.Wait shouldNeverTimeout |> shouldBeTrue - let totals = metricsListener.GetTotals() + let totals = CacheMetrics.getTotalsByName name - metricsListener.Ratio |> shouldEqual 1.0 + CacheMetrics.getRatioByName name |> shouldEqual 1.0 totals.["evictions"] |> shouldEqual 1L totals.["adds"] |> shouldEqual 3L [] let ``GetOrAdd basic usage`` () = let cacheName = "GetOrAdd_basic_usage" + use _ = CacheMetrics.ListenToAll() use cache = new Cache(defaultStructural(), name = cacheName) - use metricsListener = cache.CreateMetricsListener() let mutable factoryCalls = 0 let factory k = factoryCalls <- factoryCalls + 1; String.length k let v1 = cache.GetOrAdd("abc", factory) @@ -157,17 +158,17 @@ let ``GetOrAdd basic usage`` () = v3 |> shouldEqual 4 factoryCalls |> shouldEqual 2 // Metrics assertions - let totals = metricsListener.GetTotals() + let totals = CacheMetrics.getTotalsByName cacheName totals.["hits"] |> shouldEqual 1L totals.["misses"] |> shouldEqual 2L - metricsListener.Ratio |> shouldEqual (1.0/3.0) + CacheMetrics.getRatioByName cacheName |> shouldEqual (1.0/3.0) totals.["adds"] |> shouldEqual 2L [] let ``AddOrUpdate basic usage`` () = let cacheName = "AddOrUpdate_basic_usage" + use _ = CacheMetrics.ListenToAll() use cache = new Cache(defaultStructural(), name = cacheName) - use metricsListener = cache.CreateMetricsListener() cache.AddOrUpdate("x", 1) let mutable value = 0 cache.TryGetValue("x", &value) |> shouldBeTrue @@ -179,10 +180,10 @@ let ``AddOrUpdate basic usage`` () = cache.TryGetValue("y", &value) |> shouldBeTrue value |> shouldEqual 99 // Metrics assertions - let totals = metricsListener.GetTotals() + let totals = CacheMetrics.getTotalsByName cacheName totals.["hits"] |> shouldEqual 3L // 3 cache hits totals.["misses"] |> shouldEqual 0L // 0 cache misses - metricsListener.Ratio |> shouldEqual 1.0 + CacheMetrics.getRatioByName cacheName |> shouldEqual 1.0 totals.["adds"] |> shouldEqual 2L // "x" and "y" added totals.["updates"] |> shouldEqual 1L // "x" updated @@ -191,8 +192,8 @@ type BoxedKey = BoxedKey of int * int [] let ``GetOrAdd with reference identity`` () = let cacheName = "GetOrAdd_with_Reference" + use _ = CacheMetrics.ListenToAll() use cache = new Cache(CacheOptions.getReferenceIdentity(), cacheName) - use metricsListener = cache.CreateMetricsListener() let t1 = BoxedKey (1, 2) let t2 = BoxedKey (1, 2) let t3 = BoxedKey (1, 2) @@ -219,17 +220,17 @@ let ``GetOrAdd with reference identity`` () = v1'' |> shouldEqual v1' v2'' |> shouldEqual v2' // Metrics assertions - let totals = metricsListener.GetTotals() + let totals = CacheMetrics.getTotalsByName cacheName totals.["hits"] |> shouldEqual 4L totals.["misses"] |> shouldEqual 3L - metricsListener.Ratio |> shouldEqual (4.0 / 7.0) + CacheMetrics.getRatioByName cacheName |> shouldEqual (4.0 / 7.0) totals.["adds"] |> shouldEqual 2L [] let ``AddOrUpdate with reference identity`` () = let cacheName = "AddOrUpdate_with_Reference" + use _ = CacheMetrics.ListenToAll() use cache = new Cache(CacheOptions.getReferenceIdentity(), name = cacheName) - use metricsListener = cache.CreateMetricsListener() let t1 = box (3, 4) let t2 = box (3, 4) cache.AddOrUpdate(t1, 7) @@ -248,9 +249,9 @@ let ``AddOrUpdate with reference identity`` () = cache.TryGetValue(t1, &value1Updated) |> shouldBeTrue value1Updated |> shouldEqual 9 // Metrics assertions - let totals = metricsListener.GetTotals() + let totals = CacheMetrics.getTotalsByName cacheName totals.["hits"] |> shouldEqual 3L // 3 cache hits totals.["misses"] |> shouldEqual 0L // 0 cache misses - metricsListener.Ratio |> shouldEqual 1.0 + CacheMetrics.getRatioByName cacheName |> shouldEqual 1.0 totals.["adds"] |> shouldEqual 2L // t1 and t2 added totals.["updates"] |> shouldEqual 1L // t1 updated once diff --git a/tests/FSharp.Compiler.Service.Tests/OverloadCacheTests.fs b/tests/FSharp.Compiler.Service.Tests/OverloadCacheTests.fs index cf6032afb3a..33d837da91a 100644 --- a/tests/FSharp.Compiler.Service.Tests/OverloadCacheTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/OverloadCacheTests.fs @@ -54,23 +54,31 @@ let generateRepetitiveOverloadCalls (callCount: int) = [] let ``Overload cache hit rate exceeds 70 percent for repetitive int-int calls`` () = - use listener = FSharpChecker.CreateOverloadCacheMetricsListener() + use _ = CacheMetrics.ListenToAll() checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients() - + + // Measure only this compilation's activity: the per-name totals are process-global, so snapshot + // before/after and diff rather than reading absolute counts. + let before = CacheMetrics.getTotalsByName "overloadResolutionCache" + let callCount = 150 let source = generateRepetitiveOverloadCalls callCount checkSourceHasNoErrors source |> ignore - - let hits = listener.Hits - let misses = listener.Misses + + let after = CacheMetrics.getTotalsByName "overloadResolutionCache" + let hits = after.["hits"] - before.["hits"] + let misses = after.["misses"] - before.["misses"] Assert.True(hits + misses > 0L, "Expected cache activity but got no hits or misses - is the cache enabled?") - Assert.True(listener.Ratio > 0.70, sprintf "Expected hit ratio > 70%%, but got %.2f%%" (listener.Ratio * 100.0)) + let ratio = float hits / float (hits + misses) + Assert.True(ratio > 0.70, sprintf "Expected hit ratio > 70%%, but got %.2f%%" (ratio * 100.0)) [] let ``Overload cache returns correct resolution`` () = - use listener = FSharpChecker.CreateOverloadCacheMetricsListener() + use _ = CacheMetrics.ListenToAll() checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients() - + + let before = CacheMetrics.getTotalsByName "overloadResolutionCache" + let source = """ type Overloaded = static member Process(x: int) = "int" @@ -91,7 +99,9 @@ let f2 = Overloaded.Process(2.0) """ checkSourceHasNoErrors source |> ignore - Assert.True(listener.Hits > 0L, "Expected cache hits for repeated overload calls") + + let after = CacheMetrics.getTotalsByName "overloadResolutionCache" + Assert.True(after.["hits"] - before.["hits"] > 0L, "Expected cache hits for repeated overload calls") let overloadCorrectnessTestCases () : obj[] seq = seq { @@ -273,9 +283,8 @@ let ``Overload resolution correctness`` (_scenario: string, source: string) = [] let ``Overload cache benefits from rigid generic type parameters`` () = - use listener = FSharpChecker.CreateOverloadCacheMetricsListener() checker.ClearLanguageServiceRootCachesAndCollectAndFinalizeAllTransients() - + let source = """ type Assert = static member Equal(expected: int, actual: int) = expected = actual From ba4de570b2d18cdea6aa912f9044bf48d8632a57 Mon Sep 17 00:00:00 2001 From: Nat Elkins Date: Mon, 6 Jul 2026 20:43:23 -0400 Subject: [PATCH 3/5] Update public SurfaceArea baseline after removing CacheMetricsListener CacheMetricsListener was a public type, so dropping it changes the recorded public surface. Remove its 10 entries from FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl; the SurfaceArea test now passes. Also note the single-listener assumption the cache metric tests rely on. --- .../CompilerService/Caches.fs | 6 ++++++ ...harp.Compiler.Service.SurfaceArea.netstandard20.bsl | 10 ---------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/FSharp.Compiler.ComponentTests/CompilerService/Caches.fs b/tests/FSharp.Compiler.ComponentTests/CompilerService/Caches.fs index 21462ce1961..c00f1af81ba 100644 --- a/tests/FSharp.Compiler.ComponentTests/CompilerService/Caches.fs +++ b/tests/FSharp.Compiler.ComponentTests/CompilerService/Caches.fs @@ -16,6 +16,12 @@ let shouldNeverTimeout = 200_000 let defaultStructural() = CacheOptions.getDefault HashIdentity.Structural +// Metrics assertions below read absolute per-name totals via CacheMetrics.getTotalsByName. Those totals +// are aggregated process-globally while a CacheMetrics.ListenToAll() listener is running. This works +// because each test uses a unique cache name and this module is the only ListenToAll caller in the +// assembly, so nothing else increments those names. A second concurrently-active listener would +// double-count every measurement, so keep it that way. + [] let ``Create and dispose many`` () = let caches = diff --git a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl index 0a8b95305b6..0c81c8df894 100644 --- a/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl +++ b/tests/FSharp.Compiler.Service.Tests/FSharp.Compiler.Service.SurfaceArea.netstandard20.bsl @@ -2035,16 +2035,6 @@ FSharp.Compiler.AbstractIL.ILBinaryReader: FSharp.Compiler.AbstractIL.ILBinaryRe FSharp.Compiler.AbstractIL.ILBinaryReader: FSharp.Compiler.AbstractIL.ILBinaryReader+MetadataOnlyFlag FSharp.Compiler.AbstractIL.ILBinaryReader: FSharp.Compiler.AbstractIL.ILBinaryReader+ReduceMemoryFlag FSharp.Compiler.AbstractIL.ILBinaryReader: FSharp.Compiler.AbstractIL.ILBinaryReader+Shim -FSharp.Compiler.Caches.CacheMetrics+CacheMetricsListener: Double Ratio -FSharp.Compiler.Caches.CacheMetrics+CacheMetricsListener: Double get_Ratio() -FSharp.Compiler.Caches.CacheMetrics+CacheMetricsListener: Int64 Hits -FSharp.Compiler.Caches.CacheMetrics+CacheMetricsListener: Int64 Misses -FSharp.Compiler.Caches.CacheMetrics+CacheMetricsListener: Int64 get_Hits() -FSharp.Compiler.Caches.CacheMetrics+CacheMetricsListener: Int64 get_Misses() -FSharp.Compiler.Caches.CacheMetrics+CacheMetricsListener: Microsoft.FSharp.Collections.FSharpMap`2[System.String,System.Int64] GetTotals() -FSharp.Compiler.Caches.CacheMetrics+CacheMetricsListener: System.String ToString() -FSharp.Compiler.Caches.CacheMetrics+CacheMetricsListener: Void .ctor(System.String) -FSharp.Compiler.Caches.CacheMetrics: FSharp.Compiler.Caches.CacheMetrics+CacheMetricsListener FSharp.Compiler.Caches.CacheMetrics: System.Diagnostics.Metrics.Meter Meter FSharp.Compiler.Caches.CacheMetrics: System.Diagnostics.Metrics.Meter get_Meter() FSharp.Compiler.Cancellable: Boolean HasCancellationToken From 1c1f7a9f7a222f823f1659234bfa49921d81427d Mon Sep 17 00:00:00 2001 From: Nat Elkins Date: Mon, 6 Jul 2026 20:53:09 -0400 Subject: [PATCH 4/5] Apply fantomas formatting to Caches.fs --- src/Compiler/Utilities/Caches.fs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Compiler/Utilities/Caches.fs b/src/Compiler/Utilities/Caches.fs index 9c69a1b4e36..d8c143cebc6 100644 --- a/src/Compiler/Utilities/Caches.fs +++ b/src/Compiler/Utilities/Caches.fs @@ -400,5 +400,6 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke #if DEBUG // Shows the totals aggregated for this cache's name. Populated only while a metrics listener // (CacheMetrics.ListenToAll, e.g. under --times or the editor's metrics view) is running. - member _.DebugDisplay() = (CacheMetrics.getStatsByName name).ToString() + member _.DebugDisplay() = + (CacheMetrics.getStatsByName name).ToString() #endif From 239b6ee3c36e5dec58ed2bf00ff78611abff1087 Mon Sep 17 00:00:00 2001 From: Nat Elkins Date: Tue, 7 Jul 2026 08:58:50 -0400 Subject: [PATCH 5/5] Document why OverloadCacheTests is not parallelizable (global cache metrics state) --- tests/FSharp.Compiler.Service.Tests/OverloadCacheTests.fs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/FSharp.Compiler.Service.Tests/OverloadCacheTests.fs b/tests/FSharp.Compiler.Service.Tests/OverloadCacheTests.fs index 33d837da91a..4abe9d9c46a 100644 --- a/tests/FSharp.Compiler.Service.Tests/OverloadCacheTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/OverloadCacheTests.fs @@ -1,5 +1,10 @@ // Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. +// These tests are serialized (NotThreadSafeResourceCollection) because they read process-global state: +// the shared language-service `checker`, and the process-global cache metrics that +// `CacheMetrics.ListenToAll` aggregates by name (see the `use _ = CacheMetrics.ListenToAll()` in each +// test). Running them in parallel with each other, or alongside anything else that drives caches while +// a listener is attached, would let counts from unrelated work bleed into the before/after deltas. [] module FSharp.Compiler.Service.Tests.OverloadCacheTests