Memoization implies unique names

DataDog · Apr 24, 2023 · 60c3b7b · 60c3b7b
1 parent f068ff0
commit 60c3b7b
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 60 deletions.
diff --git a/...t/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/memoize/Memoizer.java b/...t/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<ElementMatcher, MemoizingMatcher> 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<BitSet> memos =
-      new TypeInfoCache<>(InstrumenterConfig.get().getResolverMemoPoolSize(), namesAreUnique);
+      new TypeInfoCache<>(InstrumenterConfig.get().getResolverMemoPoolSize(), true);
 
   // local memoized results, used to detect circular references
   static final ThreadLocal<Map<String, BitSet>> 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<String, BitSet> localMemos) {
-    if (namesAreUnique && NoMatchFilter.contains(target.getName())) {
+  static BitSet memoizeHierarchy(TypeDescription target, Map<String, BitSet> 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<String, BitSet> localMemos) {
+  static BitSet doMemoize(TypeDescription target, Map<String, BitSet> localMemos) {
 
     String targetName = target.getName();
     BitSet memo = localMemos.get(targetName);
@@ -194,37 +174,19 @@ static BitSet doMemoize(
 
     // existing memo from same classloader?
     SharedTypeInfo<BitSet> 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
@@ -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="