From 6b73a9feac352c1b90bf82cacd6be8daaa100ee7 Mon Sep 17 00:00:00 2001
From: fogninid <d.github.75db@f12o.eu>
Date: Mon, 14 Oct 2024 12:06:48 +0000
Subject: [PATCH] improve average performance of long task timer for
 out-of-order start/stop of many tasks

---
 .../internal/DefaultLongTaskTimer.java        | 48 ++++++++++++++-----
 1 file changed, 36 insertions(+), 12 deletions(-)

diff --git a/micrometer-core/src/main/java/io/micrometer/core/instrument/internal/DefaultLongTaskTimer.java b/micrometer-core/src/main/java/io/micrometer/core/instrument/internal/DefaultLongTaskTimer.java
index 7f56455158..9ad28d1e1a 100644
--- a/micrometer-core/src/main/java/io/micrometer/core/instrument/internal/DefaultLongTaskTimer.java
+++ b/micrometer-core/src/main/java/io/micrometer/core/instrument/internal/DefaultLongTaskTimer.java
@@ -27,9 +27,9 @@
 
 import java.util.*;
 import java.util.concurrent.ArrayBlockingQueue;
-import java.util.concurrent.ConcurrentLinkedDeque;
-import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ConcurrentSkipListSet;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import java.util.stream.StreamSupport;
@@ -37,18 +37,25 @@
 public class DefaultLongTaskTimer extends AbstractMeter implements LongTaskTimer {
 
     /**
-     * Preferring {@link ConcurrentLinkedDeque} over {@link CopyOnWriteArrayList} here
+     * Preferring {@link ConcurrentSkipListSet} over other concurrent collections
      * because...
      * <p>
      * Retrieval of percentile values will be O(N) but starting/stopping tasks will be
-     * O(1). Starting/stopping tasks happen in the same thread as the main application
-     * code, where publishing generally happens in a separate thread. Also, shipping
-     * client-side percentiles should be relatively uncommon.
+     * O(log(N)). Starting/stopping tasks happen in the same thread as the main
+     * application code, where publishing generally happens in a separate thread. Also,
+     * shipping client-side percentiles should be relatively uncommon.
+     * <p>
+     * Tasks are naturally ordered at time of insertion, but task completion/removal can
+     * happen out-of-order, that causes removal of tasks from the middle of the list. A
+     * queue would provide O(1) insertion, but O(N) removal in average. A skip-list
+     * provides O(log(N)) average across all tasks.
      * <p>
      * Histogram creation is O(N) for both the queue and list options, because we have to
      * consider which bucket each active task belongs.
      */
-    private final Deque<SampleImpl> activeTasks = new ConcurrentLinkedDeque<>();
+    private final NavigableSet<SampleImpl> activeTasks = new ConcurrentSkipListSet<>();
+
+    private final AtomicLong taskId = new AtomicLong();
 
     private final Clock clock;
 
@@ -91,7 +98,7 @@ public DefaultLongTaskTimer(Id id, Clock clock, TimeUnit baseTimeUnit,
 
     @Override
     public Sample start() {
-        SampleImpl sample = new SampleImpl();
+        SampleImpl sample = new SampleImpl(taskId.incrementAndGet());
         activeTasks.add(sample);
         return sample;
     }
@@ -108,8 +115,13 @@ public double duration(TimeUnit unit) {
 
     @Override
     public double max(TimeUnit unit) {
-        Sample oldest = activeTasks.peek();
-        return oldest == null ? 0.0 : oldest.duration(unit);
+        try {
+            Sample oldest = activeTasks.first();
+            return oldest.duration(unit);
+        }
+        catch (NoSuchElementException e) {
+            return 0.0;
+        }
     }
 
     @Override
@@ -215,13 +227,16 @@ public HistogramSnapshot takeSnapshot() {
                 (ps, scaling) -> ps.print("Summary output for LongTaskTimer histograms is not supported."));
     }
 
-    class SampleImpl extends Sample {
+    class SampleImpl extends Sample implements Comparable<SampleImpl> {
 
         private final long startTime;
 
+        private final long id;
+
         private volatile boolean stopped;
 
-        private SampleImpl() {
+        private SampleImpl(long id) {
+            this.id = id;
             this.startTime = clock.monotonicTime();
         }
 
@@ -249,6 +264,15 @@ public String toString() {
                     + ", " + "duration(nanos)=" + durationInNanoseconds + ", " + "startTimeNanos=" + startTime + '}';
         }
 
+        @Override
+        public int compareTo(DefaultLongTaskTimer.SampleImpl o) {
+            int startCompare = Long.compare(startTime, o.startTime);
+            if (startCompare == 0) {
+                return Long.compare(id, o.id);
+            }
+            return startCompare;
+        }
+
     }
 
 }