From b80e9857d241bb25d3bf079517e1c5ede7ebd9db Mon Sep 17 00:00:00 2001 From: Tyler Benson Date: Thu, 20 Feb 2020 11:14:36 -0800 Subject: [PATCH] Reorder java-concurrent matchers Move the expensive matcher to last. In theory this should help, but did not seem to make a significant difference in basic startup benchmarks. --- .../AbstractExecutorInstrumentation.java | 51 ++++++++++--------- .../concurrent/FutureInstrumentation.java | 4 +- 2 files changed, 28 insertions(+), 27 deletions(-) diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/AbstractExecutorInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/AbstractExecutorInstrumentation.java index 4fe6518f94..a7f52c27ae 100644 --- a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/AbstractExecutorInstrumentation.java +++ b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/AbstractExecutorInstrumentation.java @@ -102,33 +102,34 @@ public abstract class AbstractExecutorInstrumentation extends Instrumenter.Defau @Override public ElementMatcher typeMatcher() { - final ElementMatcher.Junction matcher = - not(isInterface()).and(safeHasInterface(named(Executor.class.getName()))); - if (TRACE_ALL_EXECUTORS) { - return matcher; + ElementMatcher.Junction matcher = not(isInterface()); + if (!TRACE_ALL_EXECUTORS) { + matcher = + matcher.and( + new ElementMatcher() { + @Override + public boolean matches(final TypeDescription target) { + boolean whitelisted = WHITELISTED_EXECUTORS.contains(target.getName()); + + // Check for possible prefixes match only if not whitelisted already + if (!whitelisted) { + for (final String name : WHITELISTED_EXECUTORS_PREFIXES) { + if (target.getName().startsWith(name)) { + whitelisted = true; + break; + } + } + } + + if (!whitelisted) { + log.debug("Skipping executor instrumentation for {}", target.getName()); + } + return whitelisted; + } + }); } return matcher.and( - new ElementMatcher() { - @Override - public boolean matches(final TypeDescription target) { - boolean whitelisted = WHITELISTED_EXECUTORS.contains(target.getName()); - - // Check for possible prefixes match only if not whitelisted already - if (!whitelisted) { - for (final String name : WHITELISTED_EXECUTORS_PREFIXES) { - if (target.getName().startsWith(name)) { - whitelisted = true; - break; - } - } - } - - if (!whitelisted) { - log.debug("Skipping executor instrumentation for {}", target.getName()); - } - return whitelisted; - } - }); + safeHasInterface(named(Executor.class.getName()))); // Apply expensive matcher last. } @Override diff --git a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/FutureInstrumentation.java b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/FutureInstrumentation.java index bf1fd2f591..11e67055b4 100644 --- a/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/FutureInstrumentation.java +++ b/dd-java-agent/instrumentation/java-concurrent/src/main/java/datadog/trace/instrumentation/java/concurrent/FutureInstrumentation.java @@ -79,7 +79,6 @@ public final class FutureInstrumentation extends Instrumenter.Default { @Override public ElementMatcher typeMatcher() { return not(isInterface()) - .and(safeHasInterface(named(Future.class.getName()))) .and( new ElementMatcher() { @Override @@ -90,7 +89,8 @@ public final class FutureInstrumentation extends Instrumenter.Default { } return whitelisted; } - }); + }) + .and(safeHasInterface(named(Future.class.getName()))); // Apply expensive matcher last. } @Override