From cb52ddc55a27299b17512762ccb17109af3447ca Mon Sep 17 00:00:00 2001 From: Mateusz Rzeszutek Date: Fri, 19 Nov 2021 06:29:45 +0100 Subject: [PATCH] Remove LongAdder and ClassValue usages from instrumentation-api (#4671) --- .../internal/RuntimeVirtualFieldSupplier.java | 17 ++++----- .../api/internal/SupportabilityMetrics.java | 38 +++++++++---------- .../api/tracer/ClassNames.java | 36 +++++++++--------- .../instrumentation/api/tracer/SpanNames.java | 16 ++++---- 4 files changed, 51 insertions(+), 56 deletions(-) diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/RuntimeVirtualFieldSupplier.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/RuntimeVirtualFieldSupplier.java index 3e87a2f7a8c1..b2f94206070f 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/RuntimeVirtualFieldSupplier.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/RuntimeVirtualFieldSupplier.java @@ -7,8 +7,6 @@ import io.opentelemetry.instrumentation.api.caching.Cache; import io.opentelemetry.instrumentation.api.field.VirtualField; -import java.util.Map; -import java.util.concurrent.ConcurrentHashMap; import javax.annotation.Nullable; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -39,18 +37,17 @@ public static VirtualFieldSupplier get() { return instance; } - private static final class CacheBasedVirtualFieldSupplier - extends ClassValue, VirtualField>> implements VirtualFieldSupplier { + private static final class CacheBasedVirtualFieldSupplier implements VirtualFieldSupplier { + + private final Cache, Cache, VirtualField>> + ownerToFieldToImplementationMap = Cache.builder().setWeakKeys().build(); @Override public VirtualField find(Class type, Class fieldType) { return (VirtualField) - get(type).computeIfAbsent(fieldType, k -> new CacheBasedVirtualField<>()); - } - - @Override - protected Map, VirtualField> computeValue(Class type) { - return new ConcurrentHashMap<>(); + ownerToFieldToImplementationMap + .computeIfAbsent(type, c -> Cache.builder().setWeakKeys().build()) + .computeIfAbsent(fieldType, c -> new CacheBasedVirtualField<>()); } } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SupportabilityMetrics.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SupportabilityMetrics.java index 706eff0e7798..0a2aa09a201e 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SupportabilityMetrics.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/internal/SupportabilityMetrics.java @@ -11,7 +11,7 @@ import java.util.concurrent.ConcurrentMap; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.LongAdder; +import java.util.concurrent.atomic.AtomicLong; import java.util.function.Consumer; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -22,7 +22,7 @@ public final class SupportabilityMetrics { private final Consumer reporter; private final ConcurrentMap suppressionCounters = new ConcurrentHashMap<>(); - private final ConcurrentMap counters = new ConcurrentHashMap<>(); + private final ConcurrentMap counters = new ConcurrentHashMap<>(); private static final SupportabilityMetrics INSTANCE = new SupportabilityMetrics(Config.get(), logger::debug).start(); @@ -52,7 +52,7 @@ public void incrementCounter(String counterName) { return; } - counters.computeIfAbsent(counterName, k -> new LongAdder()).increment(); + counters.computeIfAbsent(counterName, k -> new AtomicLong()).incrementAndGet(); } // visible for testing @@ -69,7 +69,7 @@ void report() { }); counters.forEach( (counterName, counter) -> { - long value = counter.sumThenReset(); + long value = counter.getAndSet(0); if (value > 0) { reporter.accept("Counter '" + counterName + "' : " + value); } @@ -100,28 +100,28 @@ private CounterNames() {} // this class is threadsafe. private static class KindCounters { - private final LongAdder server = new LongAdder(); - private final LongAdder client = new LongAdder(); - private final LongAdder internal = new LongAdder(); - private final LongAdder consumer = new LongAdder(); - private final LongAdder producer = new LongAdder(); + private final AtomicLong server = new AtomicLong(); + private final AtomicLong client = new AtomicLong(); + private final AtomicLong internal = new AtomicLong(); + private final AtomicLong consumer = new AtomicLong(); + private final AtomicLong producer = new AtomicLong(); void increment(SpanKind kind) { switch (kind) { case INTERNAL: - internal.increment(); + internal.incrementAndGet(); break; case SERVER: - server.increment(); + server.incrementAndGet(); break; case CLIENT: - client.increment(); + client.incrementAndGet(); break; case PRODUCER: - producer.increment(); + producer.incrementAndGet(); break; case CONSUMER: - consumer.increment(); + consumer.incrementAndGet(); break; } } @@ -129,15 +129,15 @@ void increment(SpanKind kind) { long getAndReset(SpanKind kind) { switch (kind) { case INTERNAL: - return internal.sumThenReset(); + return internal.getAndSet(0); case SERVER: - return server.sumThenReset(); + return server.getAndSet(0); case CLIENT: - return client.sumThenReset(); + return client.getAndSet(0); case PRODUCER: - return producer.sumThenReset(); + return producer.getAndSet(0); case CONSUMER: - return consumer.sumThenReset(); + return consumer.getAndSet(0); } return 0; } diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/ClassNames.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/ClassNames.java index 8e3865fb36e2..8e1fac2ada0a 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/ClassNames.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/ClassNames.java @@ -5,32 +5,32 @@ package io.opentelemetry.instrumentation.api.tracer; +import io.opentelemetry.instrumentation.api.caching.Cache; + public final class ClassNames { - private static final ClassValue simpleNames = - new ClassValue() { - @Override - protected String computeValue(Class type) { - if (!type.isAnonymousClass()) { - return type.getSimpleName(); - } - String className = type.getName(); - if (type.getPackage() != null) { - String pkgName = type.getPackage().getName(); - if (!pkgName.isEmpty()) { - className = className.substring(pkgName.length() + 1); - } - } - return className; - } - }; + private static final Cache, String> simpleNames = Cache.builder().setWeakKeys().build(); /** * This method is used to generate a simple name based on a given class reference, e.g. for use in * span names and span attributes. Anonymous classes are named based on their parent. */ public static String simpleName(Class type) { - return simpleNames.get(type); + return simpleNames.computeIfAbsent(type, ClassNames::computeSimpleName); + } + + private static String computeSimpleName(Class type) { + if (!type.isAnonymousClass()) { + return type.getSimpleName(); + } + String className = type.getName(); + if (type.getPackage() != null) { + String pkgName = type.getPackage().getName(); + if (!pkgName.isEmpty()) { + className = className.substring(pkgName.length() + 1); + } + } + return className; } private ClassNames() {} diff --git a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/SpanNames.java b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/SpanNames.java index d1fd88c00505..8b3999124fa2 100644 --- a/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/SpanNames.java +++ b/instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/SpanNames.java @@ -5,6 +5,7 @@ package io.opentelemetry.instrumentation.api.tracer; +import io.opentelemetry.instrumentation.api.caching.Cache; import io.opentelemetry.instrumentation.api.util.ClassAndMethod; import java.lang.reflect.Method; import java.util.Map; @@ -13,14 +14,8 @@ public final class SpanNames { - private static final ClassValue> spanNameCaches = - new ClassValue>() { - @Override - protected Map computeValue(Class clazz) { - // the cache is naturally bounded by the number of methods in a class - return new ConcurrentHashMap<>(); - } - }; + private static final Cache, Map> spanNameCaches = + Cache.builder().setWeakKeys().build(); /** * This method is used to generate a span name based on a method. Anonymous classes are named @@ -51,7 +46,10 @@ public static String fromMethod(ClassAndMethod classAndMethod) { * based on their parent. */ public static String fromMethod(Class clazz, String methodName) { - Map spanNameCache = spanNameCaches.get(clazz); + // the cache (ConcurrentHashMap) is naturally bounded by the number of methods in a class + Map spanNameCache = + spanNameCaches.computeIfAbsent(clazz, c -> new ConcurrentHashMap<>()); + // not using computeIfAbsent, because it would require a capturing (allocating) lambda String spanName = spanNameCache.get(methodName); if (spanName != null) {