From 60c3b7ba333fd351a3d2695f2181f1da490f692d Mon Sep 17 00:00:00 2001 From: Stuart McCulloch Date: Mon, 24 Apr 2023 01:32:46 +0100 Subject: [PATCH] Memoization implies unique names --- .../tooling/bytebuddy/memoize/Memoizer.java | 65 ++++--------------- .../datadog/trace/api/InstrumenterConfig.java | 9 --- 2 files changed, 14 insertions(+), 60 deletions(-) diff --git a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/Memoizer.java b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/Memoizer.java index 5b2d14ffbf8..6e7a6e1ec30 100644 --- a/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/Memoizer.java +++ b/dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/Memoizer.java @@ -1,17 +1,14 @@ package datadog.trace.agent.tooling.bytebuddy.memoize; -import static datadog.trace.agent.tooling.bytebuddy.TypeInfoCache.UNKNOWN_CLASS_FILE; import static net.bytebuddy.matcher.ElementMatchers.not; import datadog.trace.agent.tooling.bytebuddy.TypeInfoCache; import datadog.trace.agent.tooling.bytebuddy.TypeInfoCache.SharedTypeInfo; import datadog.trace.agent.tooling.bytebuddy.outline.TypePoolFacade; -import datadog.trace.agent.tooling.bytebuddy.outline.WithLocation; import datadog.trace.api.InstrumenterConfig; import datadog.trace.api.cache.DDCache; import datadog.trace.api.cache.DDCaches; import de.thetaphi.forbiddenapis.SuppressForbidden; -import java.net.URL; import java.util.ArrayList; import java.util.BitSet; import java.util.HashMap; @@ -59,12 +56,9 @@ enum MatcherKind { private static final DDCache memoizingMatcherCache = DDCaches.newFixedSizeIdentityCache(8); - // whether the memoizer can assume types with the same name yield the same matches - private static final boolean namesAreUnique = InstrumenterConfig.get().isResolverNamesAreUnique(); - // caches positive memoized matches private static final TypeInfoCache memos = - new TypeInfoCache<>(InstrumenterConfig.get().getResolverMemoPoolSize(), namesAreUnique); + new TypeInfoCache<>(InstrumenterConfig.get().getResolverMemoPoolSize(), true); // local memoized results, used to detect circular references static final ThreadLocal> localMemosHolder = @@ -83,9 +77,7 @@ enum MatcherKind { public static void resetState() { // no need to reset the state if we haven't added any external matchers if (matchers.size() > INTERNAL_MATCHERS) { - if (namesAreUnique) { - NoMatchFilter.clear(); - } + NoMatchFilter.clear(); Memoizer.clear(); } } @@ -156,35 +148,23 @@ protected boolean doMatch(TypeDescription target) { return false; } - ClassLoader classLoader; - if (namesAreUnique) { - classLoader = null; // unused - } else { - classLoader = - target instanceof WithLocation - ? ((WithLocation) target).getClassLoader() - : TypePoolFacade.currentContext(); - } - - if (namesAreUnique && NoMatchFilter.contains(target.getName())) { + if (NoMatchFilter.contains(target.getName())) { return false; } else { - return doMemoize(target, classLoader, localMemosHolder.get()).get(matcherId); + return doMemoize(target, localMemosHolder.get()).get(matcherId); } } } - static BitSet memoizeHierarchy( - TypeDescription target, ClassLoader classLoader, Map localMemos) { - if (namesAreUnique && NoMatchFilter.contains(target.getName())) { + static BitSet memoizeHierarchy(TypeDescription target, Map localMemos) { + if (NoMatchFilter.contains(target.getName())) { return NO_MATCH; } else { - return doMemoize(target, classLoader, localMemos); + return doMemoize(target, localMemos); } } - static BitSet doMemoize( - TypeDescription target, ClassLoader classLoader, Map localMemos) { + static BitSet doMemoize(TypeDescription target, Map localMemos) { String targetName = target.getName(); BitSet memo = localMemos.get(targetName); @@ -194,37 +174,19 @@ static BitSet doMemoize( // existing memo from same classloader? SharedTypeInfo sharedMemo = memos.find(targetName); - if (null != sharedMemo - && (namesAreUnique - || targetName.startsWith("java.") - || sharedMemo.sameClassLoader(classLoader))) { + if (null != sharedMemo) { return sharedMemo.get(); } - URL classFile; - if (namesAreUnique) { - classFile = null; // unused - } else { - classFile = - target instanceof WithLocation - ? ((WithLocation) target).getClassFile() - : UNKNOWN_CLASS_FILE; - - // existing memo from same class file? - if (null != sharedMemo && sharedMemo.sameClassFile(classFile)) { - return sharedMemo.get(); - } - } - localMemos.put(targetName, memo = new BitSet(matchers.size())); boolean wasFullParsing = TypePoolFacade.disableFullDescriptions(); // only need outlines here try { TypeDescription.Generic superTarget = target.getSuperClass(); if (null != superTarget && !"java.lang.Object".equals(superTarget.getTypeName())) { - inherit(memoizeHierarchy(superTarget.asErasure(), classLoader, localMemos), memo); + inherit(memoizeHierarchy(superTarget.asErasure(), localMemos), memo); } for (TypeDescription.Generic intf : target.getInterfaces()) { - inherit(memoizeHierarchy(intf.asErasure(), classLoader, localMemos), memo); + inherit(memoizeHierarchy(intf.asErasure(), localMemos), memo); } for (AnnotationDescription ann : target.getDeclaredAnnotations()) { record(annotationMatcherIds, ann.getAnnotationType(), memo); @@ -255,11 +217,12 @@ static BitSet doMemoize( memo = NO_MATCH; // we're only interested if there's at least one external match } - if (namesAreUnique && memo.isEmpty()) { + if (memo.isEmpty()) { NoMatchFilter.add(targetName); } else { - memos.share(targetName, classLoader, classFile, memo); + memos.share(targetName, null, null, memo); } + return memo; } diff --git a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java index ce167731ead..09224ac5d61 100644 --- a/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java +++ b/internal-api/src/main/java/datadog/trace/api/InstrumenterConfig.java @@ -35,7 +35,6 @@ import static datadog.trace.api.config.TraceInstrumentationConfig.MEASURE_METHODS; import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_CACHE_CONFIG; import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_CACHE_DIR; -import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_NAMES_ARE_UNIQUE; import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_RESET_INTERVAL; import static datadog.trace.api.config.TraceInstrumentationConfig.RESOLVER_USE_LOADCLASS; import static datadog.trace.api.config.TraceInstrumentationConfig.RUNTIME_CONTEXT_FIELD_INJECTION; @@ -95,7 +94,6 @@ public class InstrumenterConfig { private final ResolverCacheConfig resolverCacheConfig; private final String resolverCacheDir; - private final boolean resolverNamesAreUnique; private final boolean resolverUseLoadClass; private final int resolverResetInterval; @@ -173,7 +171,6 @@ private InstrumenterConfig() { configProvider.getEnum( RESOLVER_CACHE_CONFIG, ResolverCacheConfig.class, ResolverCacheConfig.DEFAULT); resolverCacheDir = configProvider.getString(RESOLVER_CACHE_DIR); - resolverNamesAreUnique = configProvider.getBoolean(RESOLVER_NAMES_ARE_UNIQUE, true); resolverUseLoadClass = configProvider.getBoolean(RESOLVER_USE_LOADCLASS, true); resolverResetInterval = Platform.isNativeImageBuilder() @@ -322,10 +319,6 @@ public String getResolverCacheDir() { return resolverCacheDir; } - public boolean isResolverNamesAreUnique() { - return resolverNamesAreUnique; - } - public boolean isResolverUseLoadClass() { return resolverUseLoadClass; } @@ -429,8 +422,6 @@ public String toString() { + resolverCacheConfig + ", resolverCacheDir=" + resolverCacheDir - + ", resolverNamesAreUnique=" - + resolverNamesAreUnique + ", resolverUseLoadClass=" + resolverUseLoadClass + ", resolverResetInterval="