Remove remaining threadlocals for loom support (#3286)

* Replaced signature parser ThreadLocal with pool

* Replaced transactions metrics labels ThreadLocal with transaction field

* Removed OkHttp cached StringBuilder

* updated changelog

* Remove unnecessary setLength call

Author: JonasKunz

CHANGELOG.asciidoc

@@ -33,7 +33,7 @@ Use subheadings with the "=====" level for adding notes for unreleased changes:
 
 [float]
 ===== Features
-* Virtual thread support - {pull}3244[#3244]
+* Virtual thread support - {pull}3244[#3244], {pull}3286[#3286]
 
 [float]
 ===== Bug fixes

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

@@ -49,12 +49,11 @@
  */
 public class Transaction extends AbstractSpan<Transaction> implements co.elastic.apm.agent.tracer.Transaction<Transaction> {
 
-    private static final ThreadLocal<Labels.Mutable> labelsThreadLocal = new ThreadLocal<Labels.Mutable>() {
-        @Override
-        protected Labels.Mutable initialValue() {
-            return Labels.Mutable.of();
-        }
-    };
+    /**
+     * Mutable labels instance used when reporting transaction metrics.
+     * This is a field to prevent allocations.
+     */
+    private final Labels.Mutable labelsMutable = Labels.Mutable.of();
 
     /**
      * Context
@@ -507,9 +506,8 @@ private void trackMetrics() {
             if (type == null) {
                 return;
             }
-            final Labels.Mutable labels = labelsThreadLocal.get();
-            labels.resetState();
-            labels.serviceName(getTraceContext().getServiceName())
+            labelsMutable.resetState();
+            labelsMutable.serviceName(getTraceContext().getServiceName())
                 .serviceVersion(getTraceContext().getServiceVersion())
                 .transactionName(name)
                 .transactionType(type);
@@ -529,8 +527,8 @@ private void trackMetrics() {
                                 if (subtype.equals("")) {
                                     subtype = null;
                                 }
-                                labels.spanType(spanType).spanSubType(subtype);
-                                metricRegistry.updateTimer("span.self_time", labels, timer.getTotalTimeUs(), timer.getCount());
+                                labelsMutable.spanType(spanType).spanSubType(subtype);
+                                metricRegistry.updateTimer("span.self_time", labelsMutable, timer.getTotalTimeUs(), timer.getCount());
                                 timer.resetState();
                             }
                         }

apm-agent-plugin-sdk/src/main/java/co/elastic/apm/agent/sdk/internal/db/signature/SignatureParser.java

@@ -18,9 +18,9 @@
  */
 package co.elastic.apm.agent.sdk.internal.db.signature;
 
-import co.elastic.apm.agent.sdk.weakconcurrent.DetachedThreadLocal;
-import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent;
-import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap;
+import co.elastic.apm.agent.sdk.internal.pooling.ObjectHandle;
+import co.elastic.apm.agent.sdk.internal.pooling.ObjectPool;
+import co.elastic.apm.agent.sdk.internal.pooling.ObjectPooling;
 
 import javax.annotation.Nullable;
 import java.util.concurrent.Callable;
@@ -44,7 +44,7 @@ public class SignatureParser {
      */
     private static final int QUERY_LENGTH_CACHE_UPPER_THRESHOLD = 10_000;
 
-    private final DetachedThreadLocal<Scanner> scanner;
+    private final ObjectPool<? extends ObjectHandle<Scanner>> scannerPool;
 
     /**
      * Not using weak keys because ORMs like Hibernate generate equal SQL strings for the same query but don't reuse the same string instance.
@@ -64,20 +64,7 @@ public Scanner call() {
     }
 
     public SignatureParser(final Callable<Scanner> scannerAllocator) {
-        scanner = WeakConcurrent
-            .<Scanner>threadLocalBuilder()
-            .withDefaultValueSupplier(new WeakMap.DefaultValueSupplier<Thread, Scanner>() {
-                @Nullable
-                @Override
-                public Scanner getDefaultValue(Thread key) {
-                    try {
-                        return scannerAllocator.call();
-                    } catch (Exception e) {
-                        throw new RuntimeException(e);
-                    }
-                }
-            })
-            .build();
+        scannerPool = ObjectPooling.createWithDefaultFactory(scannerAllocator);
     }
 
     public void querySignature(String query, StringBuilder signature, boolean preparedStatement) {
@@ -98,13 +85,15 @@ public void querySignature(String query, StringBuilder signature, @Nullable Stri
                 return;
             }
         }
-        Scanner scanner = this.scanner.get();
-        scanner.setQuery(query);
-        parse(scanner, query, signature, dbLink);
+        try (ObjectHandle<Scanner> pooledScanner = scannerPool.createInstance()) {
+            Scanner scanner = pooledScanner.get();
+            scanner.setQuery(query);
+            parse(scanner, query, signature, dbLink);
 
-        if (cacheable && signatureCache.size() <= DISABLE_CACHE_THRESHOLD) {
-            // we don't mind a small overshoot due to race conditions
-            signatureCache.put(query, new String[]{signature.toString(), dbLink != null ? dbLink.toString() : ""});
+            if (cacheable && signatureCache.size() <= DISABLE_CACHE_THRESHOLD) {
+                // we don't mind a small overshoot due to race conditions
+                signatureCache.put(query, new String[]{signature.toString(), dbLink != null ? dbLink.toString() : ""});
+            }
         }
     }
 

apm-agent-plugins/apm-okhttp-plugin/src/main/java/co/elastic/apm/agent/okhttp/OkHttpClientHelper.java

@@ -18,43 +18,25 @@
  */
 package co.elastic.apm.agent.okhttp;
 
-import co.elastic.apm.agent.sdk.state.GlobalState;
-import co.elastic.apm.agent.sdk.weakconcurrent.DetachedThreadLocal;
-import co.elastic.apm.agent.sdk.weakconcurrent.WeakConcurrent;
-import co.elastic.apm.agent.sdk.weakconcurrent.WeakMap;
-
 import javax.annotation.Nullable;
 
-@GlobalState
-public class OkHttpClientHelper {
 
-    /**
-     * Used to avoid allocations when calculating destination host name.
-     */
-    public static final DetachedThreadLocal<StringBuilder> destinationHostName = WeakConcurrent
-        .<StringBuilder>threadLocalBuilder()
-        .withDefaultValueSupplier(new WeakMap.DefaultValueSupplier<Thread, StringBuilder>() {
-            @Override
-            public StringBuilder getDefaultValue(Thread t) {
-                return new StringBuilder();
-            }
-        })
-        .build();
+public class OkHttpClientHelper {
 
     /**
-     * NOTE: this method returns a StringBuilder instance that is kept as this class's ThreadLocal. Callers of this
-     * method MAY NOT KEEP A REFERENCE TO THE RETURNED OBJECT, only copy its contents.
+     * If this method needs to perform corrections on the hostname, it has to allocate a new StringBuilder.
+     * We accept this due to the fact that this method is called once per HTTP call, making the allocation neglectable
+     * overhead compared to the allocations performed for the HTTP call itself.
      *
      * @param originalHostName the original host name retrieved from the OkHttp client
-     * @return a StringBuilder instance that is kept as a ThreadLocal
+     * @return the potentially corrected host name
      */
     @Nullable
     public static CharSequence computeHostName(@Nullable String originalHostName) {
         CharSequence hostName = originalHostName;
         // okhttp represents IPv6 addresses without square brackets, as opposed to all others, so we should add them
         if (originalHostName != null && originalHostName.contains(":") && !originalHostName.startsWith("[")) {
-            StringBuilder sb = destinationHostName.get();
-            sb.setLength(0);
+            StringBuilder sb = new StringBuilder(originalHostName.length() + 2);
             sb.append("[").append(originalHostName).append("]");
             hostName = sb;
         }