From 00b262cca3744843ecdaef69f6eb1251918fbf16 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 29 Oct 2021 11:24:40 +0300 Subject: [PATCH 1/4] Muzzle match only once in each class loader --- .../InstrumentationModuleInstaller.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java index 0430458feb32..61c4eb9f1330 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java @@ -10,6 +10,7 @@ import static net.bytebuddy.matcher.ElementMatchers.named; import static net.bytebuddy.matcher.ElementMatchers.not; +import io.opentelemetry.instrumentation.api.caching.Cache; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; import io.opentelemetry.javaagent.tooling.HelperInjector; @@ -124,6 +125,8 @@ AgentBuilder install( private static class MuzzleMatcher implements AgentBuilder.RawMatcher { private final InstrumentationModule instrumentationModule; private final AtomicBoolean initialized = new AtomicBoolean(false); + private final Cache matchingResult = + Cache.builder().setWeakKeys().build(); private volatile ReferenceMatcher referenceMatcher; private MuzzleMatcher(InstrumentationModule instrumentationModule) { @@ -141,6 +144,12 @@ public boolean matches( if (classLoader == BOOTSTRAP_LOADER) { classLoader = Utils.getBootstrapProxy(); } + + Boolean cachedResult = matchingResult.get(classLoader); + if (cachedResult != null) { + return cachedResult.booleanValue(); + } + boolean isMatch = muzzle.matches(classLoader); if (!isMatch) { @@ -165,6 +174,8 @@ public boolean matches( } } + matchingResult.put(classLoader, isMatch); + return isMatch; } From d621760a66bd3f6ea7b15bfdfd762e69dc6da586 Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 29 Oct 2021 12:49:52 +0300 Subject: [PATCH 2/4] Move muzzle matcher caching from ReferenceMatcher to InstrumentationModuleInstaller --- .../InstrumentationModuleInstaller.java | 15 +++++---------- .../tooling/muzzle/ReferenceMatcher.java | 13 +++---------- 2 files changed, 8 insertions(+), 20 deletions(-) diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java index 61c4eb9f1330..fadba5008a5e 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java @@ -125,8 +125,7 @@ AgentBuilder install( private static class MuzzleMatcher implements AgentBuilder.RawMatcher { private final InstrumentationModule instrumentationModule; private final AtomicBoolean initialized = new AtomicBoolean(false); - private final Cache matchingResult = - Cache.builder().setWeakKeys().build(); + private final Cache matchCache = Cache.builder().setWeakKeys().build(); private volatile ReferenceMatcher referenceMatcher; private MuzzleMatcher(InstrumentationModule instrumentationModule) { @@ -140,16 +139,14 @@ public boolean matches( JavaModule module, Class classBeingRedefined, ProtectionDomain protectionDomain) { - ReferenceMatcher muzzle = getReferenceMatcher(); if (classLoader == BOOTSTRAP_LOADER) { classLoader = Utils.getBootstrapProxy(); } + return matchCache.computeIfAbsent(classLoader, this::doesMatch); + } - Boolean cachedResult = matchingResult.get(classLoader); - if (cachedResult != null) { - return cachedResult.booleanValue(); - } - + private boolean doesMatch(ClassLoader classLoader) { + ReferenceMatcher muzzle = getReferenceMatcher(); boolean isMatch = muzzle.matches(classLoader); if (!isMatch) { @@ -174,8 +171,6 @@ public boolean matches( } } - matchingResult.put(classLoader, isMatch); - return isMatch; } diff --git a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcher.java b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcher.java index 1db1d885f7f5..7119633876ce 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcher.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcher.java @@ -8,7 +8,6 @@ import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; -import io.opentelemetry.instrumentation.api.caching.Cache; import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.tooling.muzzle.references.ClassRef; import io.opentelemetry.javaagent.tooling.muzzle.references.FieldRef; @@ -31,7 +30,6 @@ /** Matches a set of references against a classloader. */ public final class ReferenceMatcher { - private final Cache mismatchCache = Cache.builder().setWeakKeys().build(); private final Map references; private final Set helperClassNames; private final HelperClassPredicate helperClassPredicate; @@ -55,16 +53,11 @@ public static ReferenceMatcher of(InstrumentationModule instrumentationModule) { /** * Matcher used by ByteBuddy. Fails fast and only caches empty results, or complete results * - * @param userClassLoader Classloader to validate against (cannot be {@code null}, must pass - * "bootstrap proxy" instead of bootstrap class loader) + * @param loader Classloader to validate against (cannot be {@code null}, must pass "bootstrap + * proxy" instead of bootstrap class loader) * @return true if all references match the classpath of loader */ - public boolean matches(ClassLoader userClassLoader) { - return mismatchCache.computeIfAbsent(userClassLoader, this::doesMatch); - } - - // loader cannot be null, must pass "bootstrap proxy" instead of bootstrap class loader - private boolean doesMatch(ClassLoader loader) { + public boolean matches(ClassLoader loader) { TypePool typePool = createTypePool(loader); for (ClassRef reference : references.values()) { if (!checkMatch(reference, typePool, loader).isEmpty()) { From fb09a9e94ad34dcbb0754500c70c8640dac657ba Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 29 Oct 2021 13:02:19 +0300 Subject: [PATCH 3/4] Update comment as caching was moved to a different method --- .../javaagent/tooling/muzzle/ReferenceMatcher.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcher.java b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcher.java index 7119633876ce..3d48e133de15 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcher.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ReferenceMatcher.java @@ -51,7 +51,8 @@ public static ReferenceMatcher of(InstrumentationModule instrumentationModule) { } /** - * Matcher used by ByteBuddy. Fails fast and only caches empty results, or complete results + * Matcher used by ByteBuddy. Caller is expected to cache the result if this method is called + * multiple times for given class loader. * * @param loader Classloader to validate against (cannot be {@code null}, must pass "bootstrap * proxy" instead of bootstrap class loader) @@ -68,7 +69,7 @@ public boolean matches(ClassLoader loader) { } /** - * Loads the full list of mismatches. Used in debug contexts only + * Loads the full list of mismatches. Used in debug contexts only. * * @param loader Classloader to validate against (cannot be {@code null}, must pass "bootstrap * * proxy" instead of bootstrap class loader) From bb1c7627220c9c0344bbaf6f14d985b3d83fac5a Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Fri, 29 Oct 2021 15:14:06 +0300 Subject: [PATCH 4/4] Fix comment --- .../instrumentation/InstrumentationModuleInstaller.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java index fadba5008a5e..5f75b482e8ab 100644 --- a/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java +++ b/javaagent-tooling/src/main/java/io/opentelemetry/javaagent/tooling/instrumentation/InstrumentationModuleInstaller.java @@ -174,10 +174,8 @@ private boolean doesMatch(ClassLoader classLoader) { return isMatch; } - // ReferenceMatcher internally caches the muzzle check results per classloader, that's why we - // keep its instance in a field - // it is lazily created to avoid unnecessarily loading the muzzle references from the module - // during the agent setup + // ReferenceMatcher is lazily created to avoid unnecessarily loading the muzzle references from + // the module during the agent setup private ReferenceMatcher getReferenceMatcher() { if (initialized.compareAndSet(false, true)) { referenceMatcher = ReferenceMatcher.of(instrumentationModule);