From bb929f54b4f675d1f0827cf7aa760360817c74fc Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Tue, 21 Feb 2023 15:48:15 +0200 Subject: [PATCH 1/2] Reduce memory usage for ClassLoaderHasClassesNamedMatcher --- .../ClassLoaderMatcherCacheHolder.java | 6 +-- .../ClassLoaderHasClassesNamedMatcher.java | 51 +++++++++++++++++-- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/bootstrap/internal/ClassLoaderMatcherCacheHolder.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/bootstrap/internal/ClassLoaderMatcherCacheHolder.java index 1c23650bbc7f..583c40d180e8 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/bootstrap/internal/ClassLoaderMatcherCacheHolder.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/bootstrap/internal/ClassLoaderMatcherCacheHolder.java @@ -22,11 +22,11 @@ public final class ClassLoaderMatcherCacheHolder { @GuardedBy("allCaches") - private static final List> allCaches = new ArrayList<>(); + private static final List> allCaches = new ArrayList<>(); private ClassLoaderMatcherCacheHolder() {} - public static void addCache(Cache cache) { + public static void addCache(Cache cache) { synchronized (allCaches) { allCaches.add(cache); } @@ -34,7 +34,7 @@ public static void addCache(Cache cache) { public static void invalidateAllCachesForClassLoader(ClassLoader loader) { synchronized (allCaches) { - for (Cache cache : allCaches) { + for (Cache cache : allCaches) { cache.remove(loader); } } diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/matcher/ClassLoaderHasClassesNamedMatcher.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/matcher/ClassLoaderHasClassesNamedMatcher.java index 368effcd3447..1cf8cd68d9ca 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/matcher/ClassLoaderHasClassesNamedMatcher.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/matcher/ClassLoaderHasClassesNamedMatcher.java @@ -8,20 +8,24 @@ import io.opentelemetry.instrumentation.api.internal.cache.Cache; import io.opentelemetry.javaagent.bootstrap.internal.ClassLoaderMatcherCacheHolder; import io.opentelemetry.javaagent.bootstrap.internal.InClassLoaderMatcher; +import java.util.BitSet; +import java.util.List; +import java.util.concurrent.CopyOnWriteArrayList; +import java.util.concurrent.atomic.AtomicInteger; import net.bytebuddy.matcher.ElementMatcher; class ClassLoaderHasClassesNamedMatcher extends ElementMatcher.Junction.AbstractBase { - - private final Cache cache = Cache.weak(); + private static final AtomicInteger counter = new AtomicInteger(); private final String[] resources; + private final int index = counter.getAndIncrement(); ClassLoaderHasClassesNamedMatcher(String... classNames) { resources = classNames; for (int i = 0; i < resources.length; i++) { resources[i] = resources[i].replace(".", "/") + ".class"; } - ClassLoaderMatcherCacheHolder.addCache(cache); + Manager.INSTANCE.add(this); } @Override @@ -30,10 +34,10 @@ public boolean matches(ClassLoader cl) { // Can't match the bootstrap class loader. return false; } - return cache.computeIfAbsent(cl, this::hasResources); + return Manager.INSTANCE.match(this, cl); } - private boolean hasResources(ClassLoader cl) { + private static boolean hasResources(ClassLoader cl, String... resources) { boolean priorValue = InClassLoaderMatcher.getAndSet(true); try { for (String resource : resources) { @@ -46,4 +50,41 @@ private boolean hasResources(ClassLoader cl) { } return true; } + + private static class Manager { + private static final BitSet EMPTY = new BitSet(0); + static final Manager INSTANCE = new Manager(); + private final List matchers = new CopyOnWriteArrayList<>(); + private final Cache enabled = Cache.weak(); + private volatile boolean matchCalled = false; + + Manager() { + ClassLoaderMatcherCacheHolder.addCache(enabled); + } + + void add(ClassLoaderHasClassesNamedMatcher matcher) { + if (matchCalled) { + throw new IllegalStateException("All matchers should be create before match is called"); + } + matchers.add(matcher); + } + + boolean match(ClassLoaderHasClassesNamedMatcher matcher, ClassLoader cl) { + matchCalled = true; + BitSet set = enabled.get(cl); + if (set == null) { + set = new BitSet(counter.get()); + for (ClassLoaderHasClassesNamedMatcher m : matchers) { + if (hasResources(cl, m.resources)) { + set.set(m.index); + } + } + enabled.put(cl, set.isEmpty() ? EMPTY : set); + enabled.put(cl, set); + } else if (set.isEmpty()) { + return false; + } + return set.get(matcher.index); + } + } } From 153b71370a1ddafc897ec954c48e722920fd01ea Mon Sep 17 00:00:00 2001 From: Lauri Tulmin Date: Wed, 22 Feb 2023 10:08:12 +0200 Subject: [PATCH 2/2] Disable matcher cache for running muzzle --- .../muzzle/matcher/MuzzleGradlePluginUtil.kt | 3 --- .../ClassLoaderHasClassesNamedMatcher.java | 13 +++++++++++-- .../tooling/muzzle/ClassLoaderMatcher.java | 17 +++++++++++++++++ 3 files changed, 28 insertions(+), 5 deletions(-) diff --git a/gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt b/gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt index 7b4ab0b360b9..f7c3b8039227 100644 --- a/gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt +++ b/gradle-plugins/src/main/kotlin/io/opentelemetry/javaagent/muzzle/matcher/MuzzleGradlePluginUtil.kt @@ -57,9 +57,6 @@ class MuzzleGradlePluginUtil { val matcherClass = agentClassLoader.loadClass("io.opentelemetry.javaagent.tooling.muzzle.ClassLoaderMatcher") - // We cannot reference Mismatch class directly here, because we are loaded from a different - // class loader. - // We cannot reference Mismatch class directly here, because we are loaded from a different // class loader. val allMismatches = matcherClass diff --git a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/matcher/ClassLoaderHasClassesNamedMatcher.java b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/matcher/ClassLoaderHasClassesNamedMatcher.java index 1cf8cd68d9ca..cd6523e2e4bd 100644 --- a/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/matcher/ClassLoaderHasClassesNamedMatcher.java +++ b/javaagent-extension-api/src/main/java/io/opentelemetry/javaagent/extension/matcher/ClassLoaderHasClassesNamedMatcher.java @@ -15,6 +15,9 @@ import net.bytebuddy.matcher.ElementMatcher; class ClassLoaderHasClassesNamedMatcher extends ElementMatcher.Junction.AbstractBase { + // caching is disabled for build time muzzle checks + // this field is set via reflection from ClassLoaderMatcher + static boolean useCache = true; private static final AtomicInteger counter = new AtomicInteger(); private final String[] resources; @@ -25,7 +28,9 @@ class ClassLoaderHasClassesNamedMatcher extends ElementMatcher.Junction.Abstract for (int i = 0; i < resources.length; i++) { resources[i] = resources[i].replace(".", "/") + ".class"; } - Manager.INSTANCE.add(this); + if (useCache) { + Manager.INSTANCE.add(this); + } } @Override @@ -34,7 +39,11 @@ public boolean matches(ClassLoader cl) { // Can't match the bootstrap class loader. return false; } - return Manager.INSTANCE.match(this, cl); + if (useCache) { + return Manager.INSTANCE.match(this, cl); + } else { + return hasResources(cl, resources); + } } private static boolean hasResources(ClassLoader cl, String... resources) { diff --git a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ClassLoaderMatcher.java b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ClassLoaderMatcher.java index aa1a66ad4555..7b63fe4a96b7 100644 --- a/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ClassLoaderMatcher.java +++ b/muzzle/src/main/java/io/opentelemetry/javaagent/tooling/muzzle/ClassLoaderMatcher.java @@ -7,6 +7,7 @@ import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; import io.opentelemetry.javaagent.tooling.HelperInjector; +import java.lang.reflect.Field; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -32,6 +33,8 @@ public class ClassLoaderMatcher { */ public static Map> matchesAll( ClassLoader classLoader, boolean injectHelpers, Set excludedInstrumentationNames) { + disableMatcherCache(); + Map> result = new HashMap<>(); ServiceLoader.load(InstrumentationModule.class, ClassLoaderMatcher.class.getClassLoader()) .forEach( @@ -101,5 +104,19 @@ private static List checkHelperInjection( return mismatches; } + private static void disableMatcherCache() { + try { + Class matcherClass = + Class.forName( + "io.opentelemetry.javaagent.extension.matcher.ClassLoaderHasClassesNamedMatcher"); + Field field = matcherClass.getDeclaredField("useCache"); + field.setAccessible(true); + field.setBoolean(null, false); + } catch (Exception exception) { + throw new IllegalStateException( + "Failed to disable cache for ClassLoaderHasClassesNamedMatcher", exception); + } + } + private ClassLoaderMatcher() {} }