Pool latency histograms for sessions to avoid GC hit (microsoft#529)

* Pool latency histograms for sessions to avoid GC hit. * nit * Address comments * fix test, add returns --------- Co-authored-by: Yoganand Rajasekaran <[email protected]>
babykart · Aug 1, 2024 · 61202cd · 61202cd
1 parent 8adef79
commit 61202cd
Show file tree

Hide file tree

Showing 14 changed files with 63 additions and 17 deletions.
diff --git a/benchmark/Resp.benchmark/TxnPerfBench.cs b/benchmark/Resp.benchmark/TxnPerfBench.cs
@@ -150,7 +150,7 @@ public void Run()
                 worker.Start();
             int iteration = -reportInterval;
             var histogram = new LongHistogram(1, TimeStamp.Seconds(100), 2);
-            var summary = histogram.Copy();
+            var summary = (LongHistogram)histogram.Copy();
 
             Stopwatch swatch = new();
             waiter.Set();
@@ -227,7 +227,8 @@ public void Run()
             Console.WriteLine($"Total time: {swatch.ElapsedMilliseconds:N2}ms for {total_ops_done:N2} ops");
             Console.WriteLine($"Throughput: {opsPerSecond:N2} ops/sec");
             DumpHistogram(summary);
-
+            summary.Return();
+            histogram.Return();
             total_ops_done = 0;
             waiter.Reset();
         }

diff --git a/libs/client/GarnetClient.cs b/libs/client/GarnetClient.cs
@@ -361,6 +361,7 @@ void Dispose(bool disposing)
             timeoutCheckerCts?.Cancel();
             socket?.Dispose();
             networkWriter?.Dispose();
+            latency?.Return();
         }
 
         void CheckLength(int totalLen, TcsWrapper tcs)

diff --git a/libs/client/GarnetClientMetrics.cs b/libs/client/GarnetClientMetrics.cs
@@ -16,7 +16,7 @@ public sealed partial class GarnetClient : IServerHook, IMessageConsumer, IDispo
         /// <summary>
         /// Get client latency histogram
         /// </summary>
-        public HistogramBase GetLatencyHistogram => latency?.Copy();
+        public LongHistogram CopyLatencyHistogram => (LongHistogram)(latency?.Copy());
 
         /// <summary>
         /// Reset internal latency histogram if enabled
@@ -25,9 +25,12 @@ public sealed partial class GarnetClient : IServerHook, IMessageConsumer, IDispo
 
         private MetricsItem[] GetPercentiles(LongHistogram longHistogram, double scaling)
         {
-            var histogram = GetLatencyHistogram;
+            var histogram = CopyLatencyHistogram;
             if (histogram == null || histogram.TotalCount == 0)
+            {
+                histogram?.Return();
                 return Array.Empty<MetricsItem>();
+            }
 
             var _min = (histogram.GetValueAtPercentile(0) / scaling).ToString("N2", CultureInfo.InvariantCulture);
             var _5 = (histogram.GetValueAtPercentile(5) / scaling).ToString("N2", CultureInfo.InvariantCulture);
@@ -49,6 +52,7 @@ private MetricsItem[] GetPercentiles(LongHistogram longHistogram, double scaling
                 new("99.9th", _999)
             };
 
+            histogram.Return();
             return percentiles;
         }
 

diff --git a/libs/server/Metrics/GarnetServerMetrics.cs b/libs/server/Metrics/GarnetServerMetrics.cs
@@ -48,5 +48,8 @@ public GarnetServerMetrics(bool trackStats, bool trackLatency, GarnetServerMonit
 
             globalLatencyMetrics = trackLatency ? new() : null;
         }
+
+        public void Dispose()
+            => globalLatencyMetrics?.Return();
     }
 }
diff --git a/libs/server/Metrics/GarnetServerMonitor.cs b/libs/server/Metrics/GarnetServerMonitor.cs
@@ -74,20 +74,22 @@ public void Dispose()
             done.WaitOne();
             cts.Dispose();
             done.Dispose();
+            globalMetrics.Dispose();
         }
 
         public void Start()
         {
             Task.Run(() => MainMonitorTask(cts.Token));
         }
 
-        public void AddMetricsHistory(GarnetSessionMetrics currSessionMetrics, GarnetLatencyMetricsSession currLatencyMetrics)
+        public void AddMetricsHistorySessionDispose(GarnetSessionMetrics currSessionMetrics, GarnetLatencyMetricsSession currLatencyMetrics)
         {
             rwLock.WriteLock();
             try
             {
                 if (currSessionMetrics != null) globalMetrics.historySessionMetrics.Add(currSessionMetrics);
                 if (currLatencyMetrics != null) globalMetrics.globalLatencyMetrics.Merge(currLatencyMetrics);
+                currLatencyMetrics?.Return();
             }
             finally { rwLock.WriteUnlock(); }
         }

diff --git a/libs/server/Metrics/Latency/GarnetLatencyMetrics.cs b/libs/server/Metrics/Latency/GarnetLatencyMetrics.cs
@@ -35,6 +35,15 @@ void Init()
                 metrics[(int)cmd] = new LatencyMetricsEntry();
         }
 
+        public void Return()
+        {
+            foreach (var cmd in defaultLatencyTypes)
+            {
+                metrics[(int)cmd].Return();
+            }
+            metrics = null;
+        }
+
         public void Merge(GarnetLatencyMetricsSession lm)
         {
             if (lm.metrics == null) return;

diff --git a/libs/server/Metrics/Latency/GarnetLatencyMetricsSession.cs b/libs/server/Metrics/Latency/GarnetLatencyMetricsSession.cs
@@ -24,6 +24,15 @@ public GarnetLatencyMetricsSession(GarnetServerMonitor monitor)
             Init();
         }
 
+        public void Return()
+        {
+            foreach (var cmd in defaultLatencyTypes)
+            {
+                metrics[(int)cmd].Return();
+            }
+            metrics = null;
+        }
+
         private void Init()
         {
             metrics = new LatencyMetricsEntrySession[defaultLatencyTypes.Length];

diff --git a/libs/server/Metrics/Latency/LatencyMetricsEntry.cs b/libs/server/Metrics/Latency/LatencyMetricsEntry.cs
@@ -16,5 +16,8 @@ public LatencyMetricsEntry()
         {
             latency = new LongHistogram(HISTOGRAM_LOWER_BOUND, HISTOGRAM_UPPER_BOUND, 2);
         }
+
+        public void Return()
+            => latency.Return();
     }
 }
diff --git a/libs/server/Metrics/Latency/LatencyMetricsEntrySession.cs b/libs/server/Metrics/Latency/LatencyMetricsEntrySession.cs
@@ -21,10 +21,16 @@ static bool IsValidRange(long value)
 
         public LatencyMetricsEntrySession()
         {
-            latency = new LongHistogram[2] { new(HISTOGRAM_LOWER_BOUND, HISTOGRAM_UPPER_BOUND, 2), new(HISTOGRAM_LOWER_BOUND, HISTOGRAM_UPPER_BOUND, 2) };
+            latency = [new(HISTOGRAM_LOWER_BOUND, HISTOGRAM_UPPER_BOUND, 2), new(HISTOGRAM_LOWER_BOUND, HISTOGRAM_UPPER_BOUND, 2)];
             startTimestamp = 0;
         }
 
+        public void Return()
+        {
+            latency[0].Return();
+            latency[1].Return();
+        }
+
         public void Start()
         {
             startTimestamp = Stopwatch.GetTimestamp();

diff --git a/libs/server/Resp/LocalServerSession.cs b/libs/server/Resp/LocalServerSession.cs
@@ -54,7 +54,7 @@ public void Dispose()
             logger?.LogDebug("Disposing LocalServerSession");
 
             if (storeWrapper.serverOptions.MetricsSamplingFrequency > 0 || storeWrapper.serverOptions.LatencyMonitor)
-                storeWrapper.monitor.AddMetricsHistory(sessionMetrics, LatencyMetrics);
+                storeWrapper.monitor.AddMetricsHistorySessionDispose(sessionMetrics, LatencyMetrics);
 
             storageSession.Dispose();
         }

diff --git a/libs/server/Resp/RespServerSession.cs b/libs/server/Resp/RespServerSession.cs
@@ -198,7 +198,7 @@ public override void Dispose()
             }
 
             if (storeWrapper.serverOptions.MetricsSamplingFrequency > 0 || storeWrapper.serverOptions.LatencyMonitor)
-                storeWrapper.monitor.AddMetricsHistory(sessionMetrics, latencyMetrics);
+                storeWrapper.monitor.AddMetricsHistorySessionDispose(sessionMetrics, latencyMetrics);
 
             subscribeBroker?.RemoveSubscription(this);
             itemBroker?.HandleSessionDisposed(this);

diff --git a/metrics/HdrHistogram/LongHistogram.cs b/metrics/HdrHistogram/LongHistogram.cs
@@ -7,6 +7,7 @@
  */
 
 using System;
+using System.Buffers;
 
 namespace HdrHistogram
 {
@@ -66,7 +67,8 @@ public LongHistogram(long lowestTrackableValue, long highestTrackableValue,
                          int numberOfSignificantValueDigits)
             : base(lowestTrackableValue, highestTrackableValue, numberOfSignificantValueDigits)
         {
-            _counts = new long[CountsArrayLength];
+            _counts = ArrayPool<long>.Shared.Rent(CountsArrayLength);
+            Array.Clear(_counts, 0, CountsArrayLength);
         }
 
         /// <summary>
@@ -94,9 +96,15 @@ public LongHistogram(long instanceId, long lowestTrackableValue, long highestTra
                          int numberOfSignificantValueDigits)
             : base(instanceId, lowestTrackableValue, highestTrackableValue, numberOfSignificantValueDigits)
         {
-            _counts = new long[CountsArrayLength];
+            _counts = ArrayPool<long>.Shared.Rent(CountsArrayLength);
+            Array.Clear(_counts, 0, CountsArrayLength);
         }
 
+        /// <summary>
+        /// Return the histogram long array to the pool for recycling.
+        /// </summary>
+        public void Return()
+            => ArrayPool<long>.Shared.Return(_counts);
 
         /// <summary>
         /// Gets the total number of recorded values.
@@ -180,7 +188,7 @@ protected override void ClearCounts()
         /// <param name="target">The array to write each count value into.</param>
         protected override void CopyCountsInto(long[] target)
         {
-            Array.Copy(_counts, target, target.Length);
+            Array.Copy(_counts, target, Math.Min(_counts.Length, target.Length));
         }
     }
 }
diff --git a/playground/GarnetClientStress/TaskScaling.cs b/playground/GarnetClientStress/TaskScaling.cs
@@ -77,7 +77,7 @@ private bool PrintHistogram(int iter, int resetInterval)
             bool printHeader = false;
             for (int i = 0; i < gclientArray.Length; i++)
             {
-                var histogram = gclientArray[i]?.GetLatencyHistogram;
+                var histogram = gclientArray[i]?.CopyLatencyHistogram;
                 if (histogram != null && histogram.TotalCount > 0)
                 {
                     Console.WriteLine("{0,12:0.0}; {1,12:0.0}; {2,12:0.0}; {3,12:0.0}; {4,12:0.0}; {5,12:0.0}; {6,12:0.0}; {7,16:N0}; {8,12:N0}; {9,12:N0}; {10,12:N0}; {11,14:N0}",
@@ -99,6 +99,7 @@ private bool PrintHistogram(int iter, int resetInterval)
                         printHeader = true;
                     }
                 }
+                histogram?.Return();
             }
             return printHeader;
         }

diff --git a/test/Garnet.test/GarnetClientTests.cs b/test/Garnet.test/GarnetClientTests.cs
@@ -153,7 +153,7 @@ public void SimpleMetricsTest([Values] bool recordLatency)
             db.Connect();
 
             var metrics = db.GetLatencyMetrics();
-            Assert.AreEqual(metrics.Length, 0);
+            Assert.AreEqual(0, metrics.Length);
 
             if (recordLatency)
             {
@@ -163,17 +163,16 @@ public void SimpleMetricsTest([Values] bool recordLatency)
                     var task = db.StringGetAsync("mykey");
                     task.Wait();
                     var resp = task.Result;
-                    Assert.AreEqual(resp, null);
+                    Assert.AreEqual(null, resp);
                 }
 
                 metrics = db.GetLatencyMetrics();
-                Assert.AreEqual(metrics.Length, 8);
+                Assert.AreEqual(8, metrics.Length);
             }
 
             db.Dispose();
             metrics = db.GetLatencyMetrics();
-            if (recordLatency) Assert.AreEqual(metrics.Length, 8);
-            else Assert.AreEqual(metrics.Length, 0);
+            Assert.AreEqual(0, metrics.Length); // Should be 0 after dispose
         }
 
         [Test]