Avoid leaking a MeterListener per Cache in DEBUG builds

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 `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 state machine lowering dropping the side-effectful receiver of an unused unit-typed member access (e.g. inside `task { (effectful()).UnitProp }`). ([Issue #13099](https://github.com/dotnet/fsharp/issues/13099), [PR #19885](https://github.com/dotnet/fsharp/pull/19885))
 * `--deterministic` Release builds now produce byte-identical `FSharp.Compiler.Service.dll` under `--parallelcompilation+` and `--parallelcompilation-`, so it is restored to the determinism gate (now also checked sequential-vs-parallel). Code generation runs the same deferred per-file drain in both modes, with type/member/field emit-order keys and generated names derived from the file being emitted rather than thread-scheduling order. ([Issue #19928](https://github.com/dotnet/fsharp/issues/19928), [PR #19929](https://github.com/dotnet/fsharp/pull/19929))
 * Fix `[<CompiledName>]` silently producing duplicate IL entries (FS0192/FS2014) when applied to a multi-value let-binding (e.g. `let a, b = 1, 2`); now emits FS0755 at type-check time. ([Issue #6131](https://github.com/dotnet/fsharp/issues/6131), [PR #19924](https://github.com/dotnet/fsharp/pull/19924))

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

src/Compiler/Service/service.fsi

@@ -506,9 +506,6 @@ type public FSharpChecker =
     [<Obsolete("Please create an instance of FSharpChecker using FSharpChecker.Create")>]
     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
 

src/Compiler/Utilities/Caches.fs

@@ -22,14 +22,12 @@ module CacheMetrics =
     let creations = Meter.CreateCounter<int64>("creations", "count")
     let disposals = Meter.CreateCounter<int64>("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<TagList>) = adds.Add(1L, &tags)
@@ -78,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()
 
@@ -123,50 +125,6 @@ module CacheMetrics =
                 Console.WriteLine(StatsToString())
         }
 
-    [<Sealed>]
-    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()
-
 [<RequireQualifiedAccess>]
 type EvictionMode =
     | NoEviction
@@ -361,10 +319,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
@@ -430,9 +384,6 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke
             CacheMetrics.Update &tags
             post (EvictionQueueMessage.Update result)
 
-    member _.CreateMetricsListener() =
-        new CacheMetrics.CacheMetricsListener(tags)
-
     member _.Dispose() =
         if Interlocked.Exchange(&disposed, 1) = 0 then
             disposeEvictionProcessor ()
@@ -447,5 +398,8 @@ type Cache<'Key, 'Value when 'Key: not null> internal (options: CacheOptions<'Ke
     override this.Finalize() = this.Dispose()
 
 #if DEBUG
-    member _.DebugDisplay() = debugListener.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

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<string, int64>
+
+    /// 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.
-    [<Sealed>]
-    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<string, int64>
-        /// 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
-
 [<RequireQualifiedAccess; NoComparison>]
 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<unit>
     /// For testing only.
     member EvictionFailed: IEvent<unit>
-    /// For testing only. Creates a local telemetry listener for this cache instance.
-    member CreateMetricsListener: unit -> CacheMetrics.CacheMetricsListener

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.
+
 [<Fact>]
 let ``Create and dispose many`` () =
     let caches = 
@@ -28,8 +34,8 @@ let ``Create and dispose many`` () =
 [<Fact>]
 let ``Basic add and retrieve`` () =
     let name = "Basic_add_and_retrieve"
+    use _ = CacheMetrics.ListenToAll()
     use cache = new Cache<string, int>(defaultStructural(), name = name)
-    use metricsListener = cache.CreateMetricsListener()
 
     cache.TryAdd("key1", 1) |> shouldBeTrue
     cache.TryAdd("key2", 2) |> shouldBeTrue
@@ -45,14 +51,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
 
 [<Fact>]
 let ``Eviction of least recently used`` () =
     let name = "Eviction_of_least_recently_used"
+    use _ = CacheMetrics.ListenToAll()
     use cache = new Cache<string, int>({ defaultStructural() with TotalCapacity = 2; HeadroomPercentage = 0 }, name = name)
-    use metricsListener = cache.CreateMetricsListener()
 
     cache.TryAdd("key1", 1) |> shouldBeTrue
     cache.TryAdd("key2", 2) |> shouldBeTrue
@@ -76,7 +82,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
 
 [<Fact>]
@@ -85,14 +91,14 @@ let ``Stress test evictions`` () =
     let iterations = 10_000
     let name = "Stress test evictions"
 
+    use _ = CacheMetrics.ListenToAll()
     use cache = new Cache<string, int>({ defaultStructural() with TotalCapacity = cacheSize; HeadroomPercentage = 0 }, name = name)
-    use metricsListener = cache.CreateMetricsListener()
 
     let evictionsCompleted = new TaskCompletionSource<unit>()
     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 +120,14 @@ let ``Stress test evictions`` () =
     value |> shouldEqual iterations
 
     // Metrics assertions
-    let totals = metricsListener.GetTotals()
+    let totals = CacheMetrics.getTotalsByName name
     totals.["adds"] |> shouldEqual (int64 iterations)
 
 [<Fact>]
 let ``Metrics can be retrieved`` () =
-    use cache = new Cache<string, int>({ defaultStructural() with TotalCapacity = 2; HeadroomPercentage = 0 }, name = "test_metrics")
-    use metricsListener = cache.CreateMetricsListener()
+    let name = "test_metrics"
+    use _ = CacheMetrics.ListenToAll()
+    use cache = new Cache<string, int>({ defaultStructural() with TotalCapacity = 2; HeadroomPercentage = 0 }, name = name)
 
     cache.TryAdd("key1", 1) |> shouldBeTrue
     cache.TryAdd("key2", 2) |> shouldBeTrue
@@ -135,17 +142,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
 
 [<Fact>]
 let ``GetOrAdd basic usage`` () =
     let cacheName = "GetOrAdd_basic_usage"
+    use _ = CacheMetrics.ListenToAll()
     use cache = new Cache<string, int>(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 +164,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
 
 [<Fact>]
 let ``AddOrUpdate basic usage`` () =
     let cacheName = "AddOrUpdate_basic_usage"
+    use _ = CacheMetrics.ListenToAll()
     use cache = new Cache<string, int>(defaultStructural(), name = cacheName)
-    use metricsListener = cache.CreateMetricsListener()
     cache.AddOrUpdate("x", 1)
     let mutable value = 0
     cache.TryGetValue("x", &value) |> shouldBeTrue
@@ -179,10 +186,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 +198,8 @@ type BoxedKey = BoxedKey of int * int
 [<Fact>]
 let ``GetOrAdd with reference identity`` () =
     let cacheName = "GetOrAdd_with_Reference"
+    use _ = CacheMetrics.ListenToAll()
     use cache = new Cache<BoxedKey, int>(CacheOptions.getReferenceIdentity(), cacheName)
-    use metricsListener = cache.CreateMetricsListener()
     let t1 = BoxedKey (1, 2)
     let t2 = BoxedKey (1, 2)
     let t3 = BoxedKey (1, 2)
@@ -219,17 +226,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
 
 [<Fact>]
 let ``AddOrUpdate with reference identity`` () =
     let cacheName = "AddOrUpdate_with_Reference"
+    use _ = CacheMetrics.ListenToAll()
     use cache = new Cache<obj, int>(CacheOptions.getReferenceIdentity(), name = cacheName)
-    use metricsListener = cache.CreateMetricsListener()
     let t1 = box (3, 4)
     let t2 = box (3, 4)
     cache.AddOrUpdate(t1, 7)
@@ -248,9 +255,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

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