Merge branch 'master' of github.com:DataDog/dd-trace-java

dd-java-agent/agent-tooling/src/main/java/datadog/trace/agent/tooling/bytebuddy/DDCachingPoolStrategy.java

@@ -42,7 +42,7 @@ public class DDCachingPoolStrategy implements PoolStrategy {
   static final int LOADER_CAPACITY = 64;
   static final int TYPE_CAPACITY = 64;
 
-  static final int BOOTSTRAP_HASH = 0;
+  static final int BOOTSTRAP_HASH = 7236344; // Just a random number
 
   /**
    * Cache of recent ClassLoader WeakReferences; used to...
@@ -89,7 +89,7 @@ public class DDCachingPoolStrategy implements PoolStrategy {
       loaderRefCache.put(classLoader, loaderRef);
     }
 
-    int loaderHash = classLoader.hashCode();
+    final int loaderHash = classLoader.hashCode();
     return createCachingTypePool(loaderHash, loaderRef, classFileLocator);
   }
 
@@ -140,7 +140,7 @@ public class DDCachingPoolStrategy implements PoolStrategy {
       this.loaderRef = loaderRef;
       this.className = className;
 
-      hashCode = (int) (31 * this.loaderHash) ^ className.hashCode();
+      hashCode = 31 * this.loaderHash + className.hashCode();
     }
 
     @Override
@@ -150,18 +150,23 @@ public class DDCachingPoolStrategy implements PoolStrategy {
 
     @Override
     public boolean equals(final Object obj) {
-      if (!(obj instanceof TypeCacheKey)) return false;
+      if (!(obj instanceof TypeCacheKey)) {
+        return false;
+      }
 
-      TypeCacheKey that = (TypeCacheKey) obj;
+      final TypeCacheKey that = (TypeCacheKey) obj;
 
-      if (loaderHash != that.loaderHash) return false;
+      if (loaderHash != that.loaderHash) {
+        return false;
+      }
+
+      if (className.equals(that.className)) {
+        // Fastpath loaderRef equivalence -- works because of WeakReference cache used
+        // Also covers the bootstrap null loaderRef case
+        if (loaderRef == that.loaderRef) {
+          return true;
+        }
 
-      // Fastpath loaderRef equivalence -- works because of WeakReference cache used
-      // Also covers the bootstrap null loaderRef case
-      if (loaderRef == that.loaderRef) {
-        // still need to check name
-        return className.equals(that.className);
-      } else if (className.equals(that.className)) {
         // need to perform a deeper loader check -- requires calling Reference.get
         // which can strengthen the Reference, so deliberately done last
 
@@ -172,11 +177,15 @@ public class DDCachingPoolStrategy implements PoolStrategy {
         // In this case, it is fine because that means the ClassLoader is no
         // longer live, so the entries will never match anyway and will fall
         // out of the cache.
-        ClassLoader thisLoader = loaderRef.get();
+        final ClassLoader thisLoader = loaderRef.get();
-        if (thisLoader == null) return false;
+        if (thisLoader == null) {
+          return false;
+        }
 
-        ClassLoader thatLoader = that.loaderRef.get();
+        final ClassLoader thatLoader = that.loaderRef.get();
-        if (thatLoader == null) return false;
+        if (thatLoader == null) {
+          return false;
+        }
 
         return (thisLoader == thatLoader);
       } else {
@@ -205,9 +214,11 @@ public class DDCachingPoolStrategy implements PoolStrategy {
 
     @Override
     public TypePool.Resolution find(final String className) {
-      TypePool.Resolution existingResolution =
+      final TypePool.Resolution existingResolution =
           sharedResolutionCache.getIfPresent(new TypeCacheKey(loaderHash, loaderRef, className));
-      if (existingResolution != null) return existingResolution;
+      if (existingResolution != null) {
+        return existingResolution;
+      }
 
       if (OBJECT_NAME.equals(className)) {
         return OBJECT_RESOLUTION;

dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/AbstractExecutorInstrumentation.java

@@ -103,6 +103,8 @@ public abstract class AbstractExecutorInstrumentation extends Instrumenter.Defau
   @Override
   public ElementMatcher<TypeDescription> typeMatcher() {
     ElementMatcher.Junction<TypeDescription> matcher = not(isInterface());
+    final ElementMatcher.Junction<TypeDescription> hasExecutorInterfaceMatcher =
+        hasInterface(named(Executor.class.getName()));
     if (!TRACE_ALL_EXECUTORS) {
       matcher =
           matcher.and(
@@ -121,15 +123,16 @@ public abstract class AbstractExecutorInstrumentation extends Instrumenter.Defau
                     }
                   }
 
-                  if (!whitelisted) {
+                  if (!whitelisted
+                      && log.isDebugEnabled()
+                      && hasExecutorInterfaceMatcher.matches(target)) {
                     log.debug("Skipping executor instrumentation for {}", target.getName());
                   }
                   return whitelisted;
                 }
               });
     }
-    return matcher.and(
+    return matcher.and(hasExecutorInterfaceMatcher); // Apply expensive matcher last.
-        hasInterface(named(Executor.class.getName()))); // Apply expensive matcher last.
   }
 
   @Override

dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/FutureInstrumentation.java

@@ -78,19 +78,23 @@ public final class FutureInstrumentation extends Instrumenter.Default {
 
   @Override
   public ElementMatcher<TypeDescription> typeMatcher() {
+    final ElementMatcher.Junction<TypeDescription> hasFutureInterfaceMatcher =
+        hasInterface(named(Future.class.getName()));
     return not(isInterface())
         .and(
             new ElementMatcher<TypeDescription>() {
               @Override
               public boolean matches(final TypeDescription target) {
                 final boolean whitelisted = WHITELISTED_FUTURES.contains(target.getName());
-                if (!whitelisted) {
+                if (!whitelisted
+                    && log.isDebugEnabled()
+                    && hasFutureInterfaceMatcher.matches(target)) {
                   log.debug("Skipping future instrumentation for {}", target.getName());
                 }
                 return whitelisted;
               }
             })
-        .and(hasInterface(named(Future.class.getName()))); // Apply expensive matcher last.
+        .and(hasFutureInterfaceMatcher); // Apply expensive matcher last.
   }
 
   @Override