Make tracking breakdown metrics garbage free

Author: felixbarny

apm-agent-benchmarks/src/main/java/co/elastic/apm/agent/benchmark/ElasticApmContinuousBenchmark.java

@@ -145,6 +145,7 @@ public void setUp(Blackhole blackhole) {
 
     @TearDown
     public void tearDown() throws ExecutionException, InterruptedException {
+        Thread.sleep(1000);
         tracer.getReporter().flush().get();
         server.stop();
         System.out.println("Reported: " + tracer.getReporter().getReported());

apm-agent-core/src/main/java/co/elastic/apm/agent/impl/transaction/Transaction.java

@@ -24,20 +24,25 @@
 import co.elastic.apm.agent.impl.sampling.Sampler;
 import co.elastic.apm.agent.metrics.Labels;
 import co.elastic.apm.agent.metrics.Timer;
+import co.elastic.apm.agent.util.KeyListConcurrentHashMap;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.annotation.Nullable;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
+import java.util.List;
 
 /**
  * Data captured by an agent representing an event occurring in a monitored service
  */
 public class Transaction extends AbstractSpan<Transaction> {
 
     private static final Logger logger = LoggerFactory.getLogger(Transaction.class);
+    private static final ThreadLocal<Labels> labelsThreadLocal = new ThreadLocal<>() {
+        @Override
+        protected Labels initialValue() {
+            return new Labels();
+        }
+    };
 
     public static final String TYPE_REQUEST = "request";
 
@@ -48,7 +53,7 @@ public class Transaction extends AbstractSpan<Transaction> {
      */
     private final TransactionContext context = new TransactionContext();
     private final SpanCount spanCount = new SpanCount();
-    private final ConcurrentMap<String, Timer> spanTimings = new ConcurrentHashMap<>();
+    private final KeyListConcurrentHashMap<String, Timer> spanTimings = new KeyListConcurrentHashMap<>();
 
     /**
      * The result of the transaction. HTTP status code for HTTP-related transactions.
@@ -175,7 +180,6 @@ public void setUser(String id, String email, String username) {
 
     @Override
     public void doEnd(long epochMicros) {
-        incrementTimer("transaction", getSelfDuration());
         if (!isSampled()) {
             context.resetState();
         }
@@ -191,7 +195,7 @@ public SpanCount getSpanCount() {
         return spanCount;
     }
 
-    public ConcurrentMap<String, Timer> getSpanTimings() {
+    public KeyListConcurrentHashMap<String, Timer> getSpanTimings() {
         return spanTimings;
     }
 
@@ -263,18 +267,20 @@ private void trackMetrics() {
         if (type == null) {
             return;
         }
-        final StringBuilder transactionName = getName();
-        final Labels labels = new Labels();
-        for (Map.Entry<String, Timer> entry : getSpanTimings().entrySet()) {
-            final Timer timer = entry.getValue();
+        final Labels labels = labelsThreadLocal.get();
+        labels.resetState();
+        labels.transactionName(name).transactionType(type);
+        final KeyListConcurrentHashMap<String, Timer> spanTimings = getSpanTimings();
+        List<String> keyList = spanTimings.keyList();
+        for (int i = 0; i < keyList.size(); i++) {
+            String spanType = keyList.get(i);
+            final Timer timer = spanTimings.get(spanType);
             if (timer.getCount() > 0) {
-                labels.resetState();
-                labels.transactionName(transactionName)
-                    .transactionType(type)
-                    .spanType(entry.getKey());
-                tracer.getMetricRegistry().timer("self_time", labels).update(timer.getTotalTimeNs(), timer.getCount());
+                tracer.getMetricRegistry().timer("self_time", labels.spanType(spanType)).update(timer.getTotalTimeUs(), timer.getCount());
                 timer.resetState();
             }
         }
+        tracer.getMetricRegistry().timer("self_time", labels.spanType("transaction")).update(getSelfDuration());
+        tracer.getMetricRegistry().timer("duration", labels.spanType("transaction")).update(getDuration());
     }
 }

apm-agent-core/src/main/java/co/elastic/apm/agent/metrics/Labels.java

@@ -142,7 +142,7 @@ public List<CharSequence> getValues() {
     }
 
     public boolean isEmpty() {
-        return keys.isEmpty();
+        return keys.isEmpty() && transactionName == null && transactionType == null && spanType == null;
     }
 
     public int size() {
@@ -162,11 +162,11 @@ public boolean equals(Object o) {
         if (this == o) return true;
         if (o == null || getClass() != o.getClass()) return false;
         Labels labels = (Labels) o;
-        return keys.equals(labels.keys) &&
-            isEqual(values, labels.values) &&
-            contentEquals(transactionName, labels.transactionName) &&
+        return Objects.equals(spanType, labels.spanType) &&
             Objects.equals(transactionType, labels.transactionType) &&
-            Objects.equals(spanType, labels.spanType);
+            contentEquals(transactionName, labels.transactionName) &&
+            keys.equals(labels.keys) &&
+            isEqual(values, labels.values);
     }
 
     @Override
@@ -179,8 +179,8 @@ public int hashCode() {
             h = 31 * h + hash(i);
         }
         h = 31 * h + hash(transactionName);
-        h = 31 * h + Objects.hashCode(transactionType);
-        h = 31 * h + Objects.hashCode(spanType);
+        h = 31 * h + (transactionType != null ? transactionType.hashCode() : 0);
+        h = 31 * h + (spanType != null ? spanType.hashCode() : 0);
         return h;
     }
 

apm-agent-core/src/main/java/co/elastic/apm/agent/metrics/MetricSet.java

@@ -40,7 +40,8 @@
 public class MetricSet {
     private final Labels labels;
     private final ConcurrentMap<String, DoubleSupplier> gauges = new ConcurrentHashMap<>();
-    private final ConcurrentMap<String, Timer> timers = new ConcurrentHashMap<>();
+    // low load factor as hash collisions are quite costly when tracking breakdown metrics
+    private final ConcurrentMap<String, Timer> timers = new ConcurrentHashMap<>(32, 0.5f, Runtime.getRuntime().availableProcessors());
     private volatile boolean hasNonEmptyTimer;
 
     public MetricSet(Labels labels) {

apm-agent-core/src/main/java/co/elastic/apm/agent/metrics/Timer.java

@@ -30,16 +30,16 @@ public class Timer implements Recyclable {
     private AtomicLong totalTime = new AtomicLong();
     private AtomicLong count = new AtomicLong();
 
-    public void update(long durationNs) {
-        update(durationNs, 1);
+    public void update(long durationUs) {
+        update(durationUs, 1);
     }
 
-    public void update(long durationNs, long count) {
-        this.totalTime.addAndGet(durationNs);
+    public void update(long durationUs, long count) {
+        this.totalTime.addAndGet(durationUs);
         this.count.addAndGet(count);
     }
 
-    public long getTotalTimeNs() {
+    public long getTotalTimeUs() {
         return totalTime.get();
     }
 

apm-agent-core/src/main/java/co/elastic/apm/agent/util/KeyListConcurrentHashMap.java

@@ -0,0 +1,77 @@
+package co.elastic.apm.agent.util;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * A subclass of {@link ConcurrentHashMap} which maintains a {@link List} of all the keys in the map.
+ * It can be used to iterate over the map's keys without allocating an {@link java.util.Iterator}
+ */
+public class KeyListConcurrentHashMap<K, V> extends ConcurrentHashMap<K, V> {
+    private final List<K> keyList = Collections.synchronizedList(new ArrayList<>());
+
+    @Override
+    public V put(K key, V value) {
+        final V previousValue = super.put(key, value);
+        if (previousValue == null) {
+            keyList.add(key);
+        }
+        return previousValue;
+    }
+
+    @Override
+    public void putAll(Map<? extends K, ? extends V> m) {
+        for (Entry<? extends K, ? extends V> entry : m.entrySet()) {
+            put(entry.getKey(), entry.getValue());
+        }
+    }
+
+    @Override
+    public V remove(Object key) {
+        keyList.remove(key);
+        return super.remove(key);
+    }
+
+    @Override
+    public void clear() {
+        keyList.clear();
+        super.clear();
+    }
+
+    @Override
+    public V putIfAbsent(K key, V value) {
+        final V previousValue = super.putIfAbsent(key, value);
+        if (previousValue == null) {
+            keyList.add(key);
+        }
+        return previousValue;
+    }
+
+    @Override
+    public boolean remove(Object key, Object value) {
+        final boolean remove = super.remove(key, value);
+        if (remove) {
+            keyList.remove(key);
+        }
+        return remove;
+    }
+
+    /**
+     * Returns a mutable {@link List}, roughly equal to the {@link #keySet()}.
+     * <p>
+     * Note that in concurrent scenarios, the key list may be a subset of the values of the respective {@link #keySet()}.
+     * Entries added via the {@code compute*} family of methods are not reflected in the list.
+     * </p>
+     * <p>
+     * Do not modify this list.
+     * </p>
+     *
+     * @return a {@link List}, roughly equal to the {@link #keySet()}
+     */
+    public List<K> keyList() {
+        return keyList;
+    }
+}