From 903ed871b896cf5b4c02068cefa30657daeb3a8b Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sat, 2 Mar 2024 19:05:20 +0100 Subject: [PATCH 1/5] fix: Cache dynamic proxy classes created by ByteBuddy --- .../io/appium/java_client/proxy/Helpers.java | 66 ++++++++++++------- 1 file changed, 43 insertions(+), 23 deletions(-) diff --git a/src/main/java/io/appium/java_client/proxy/Helpers.java b/src/main/java/io/appium/java_client/proxy/Helpers.java index 1b0c7a6d4..470a1659e 100644 --- a/src/main/java/io/appium/java_client/proxy/Helpers.java +++ b/src/main/java/io/appium/java_client/proxy/Helpers.java @@ -17,6 +17,7 @@ package io.appium.java_client.proxy; import com.google.common.base.Preconditions; +import lombok.Value; import net.bytebuddy.ByteBuddy; import net.bytebuddy.description.method.MethodDescription; import net.bytebuddy.description.modifier.Visibility; @@ -31,6 +32,8 @@ import java.util.Collection; import java.util.Collections; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -42,6 +45,13 @@ public class Helpers { .map(Method::getName) .collect(Collectors.toSet()); + // Each proxy classes created by ByteBuddy get automatically cached by the + // given class loader. It is important to have this cache here in order to improve + // the performance and to avoid extensive memory usage for our case, where + // the amount of instrumented proxy classes we create is low in comparison to the amount + // of proxy instances. + private static final ConcurrentMap> CACHED_PROXY_CLASSES = new ConcurrentHashMap<>(); + private Helpers() { } @@ -104,32 +114,35 @@ public static T createProxy( Collection listeners, @Nullable ElementMatcher extraMethodMatcher ) { - Preconditions.checkArgument(constructorArgs.length == constructorArgTypes.length, - String.format( - "Constructor arguments array length %d must be equal to the types array length %d", - constructorArgs.length, constructorArgTypes.length - ) - ); - Preconditions.checkArgument(!listeners.isEmpty(), "The collection of listeners must not be empty"); - requireNonNull(cls, "Class must not be null"); - Preconditions.checkArgument(!cls.isInterface(), "Class must not be an interface"); + var signature = ClassSignature.of(cls, constructorArgs, constructorArgTypes); + var proxyClass = CACHED_PROXY_CLASSES.computeIfAbsent(signature, (k) -> { + Preconditions.checkArgument(constructorArgs.length == constructorArgTypes.length, + String.format( + "Constructor arguments array length %d must be equal to the types array length %d", + constructorArgs.length, constructorArgTypes.length + ) + ); + Preconditions.checkArgument(!listeners.isEmpty(), "The collection of listeners must not be empty"); + requireNonNull(cls, "Class must not be null"); + Preconditions.checkArgument(!cls.isInterface(), "Class must not be an interface"); - ElementMatcher.Junction matcher = ElementMatchers.isPublic(); - //noinspection resource - Class proxy = new ByteBuddy() - .subclass(cls) - .method(extraMethodMatcher == null ? matcher : matcher.and(extraMethodMatcher)) - .intercept(MethodDelegation.to(Interceptor.class)) - // https://github.com/raphw/byte-buddy/blob/2caef35c172897cbdd21d163c55305a64649ce41/byte-buddy-dep/src/test/java/net/bytebuddy/ByteBuddyTutorialExamplesTest.java#L346 - .defineField("methodCallListeners", MethodCallListener[].class, Visibility.PRIVATE) - .implement(HasMethodCallListeners.class).intercept(FieldAccessor.ofBeanProperty()) - .make() - .load(ClassLoader.getSystemClassLoader(), ClassLoadingStrategy.Default.WRAPPER) - .getLoaded() - .asSubclass(cls); + ElementMatcher.Junction matcher = ElementMatchers.isPublic(); + //noinspection resource + return new ByteBuddy() + .subclass(cls) + .method(extraMethodMatcher == null ? matcher : matcher.and(extraMethodMatcher)) + .intercept(MethodDelegation.to(Interceptor.class)) + // https://github.com/raphw/byte-buddy/blob/2caef35c172897cbdd21d163c55305a64649ce41/byte-buddy-dep/src/test/java/net/bytebuddy/ByteBuddyTutorialExamplesTest.java#L346 + .defineField("methodCallListeners", MethodCallListener[].class, Visibility.PRIVATE) + .implement(HasMethodCallListeners.class).intercept(FieldAccessor.ofBeanProperty()) + .make() + .load(ClassLoader.getSystemClassLoader(), ClassLoadingStrategy.Default.WRAPPER) + .getLoaded() + .asSubclass(cls); + }); try { - T result = cls.cast(proxy.getConstructor(constructorArgTypes).newInstance(constructorArgs)); + T result = cls.cast(proxyClass.getConstructor(constructorArgTypes).newInstance(constructorArgs)); ((HasMethodCallListeners) result).setMethodCallListeners(listeners.toArray(MethodCallListener[]::new)); return result; } catch (SecurityException | ReflectiveOperationException e) { @@ -201,4 +214,11 @@ public static T createProxy( ) { return createProxy(cls, constructorArgs, constructorArgTypes, Collections.singletonList(listener)); } + + @Value(staticConstructor = "of") + private static class ClassSignature { + Class cls; + Object[] constructorArgs; + Class[] constructorArgTypes; + } } From 165ebaec26a0a47a804858f9da95b328f180fe5a Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sat, 2 Mar 2024 19:14:28 +0100 Subject: [PATCH 2/5] Add metcher --- src/main/java/io/appium/java_client/proxy/Helpers.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/main/java/io/appium/java_client/proxy/Helpers.java b/src/main/java/io/appium/java_client/proxy/Helpers.java index 470a1659e..c06a5679d 100644 --- a/src/main/java/io/appium/java_client/proxy/Helpers.java +++ b/src/main/java/io/appium/java_client/proxy/Helpers.java @@ -45,7 +45,7 @@ public class Helpers { .map(Method::getName) .collect(Collectors.toSet()); - // Each proxy classes created by ByteBuddy get automatically cached by the + // Each proxy class created by ByteBuddy gets automatically cached by the // given class loader. It is important to have this cache here in order to improve // the performance and to avoid extensive memory usage for our case, where // the amount of instrumented proxy classes we create is low in comparison to the amount @@ -114,7 +114,7 @@ public static T createProxy( Collection listeners, @Nullable ElementMatcher extraMethodMatcher ) { - var signature = ClassSignature.of(cls, constructorArgs, constructorArgTypes); + var signature = ClassSignature.of(cls, constructorArgs, constructorArgTypes, extraMethodMatcher); var proxyClass = CACHED_PROXY_CLASSES.computeIfAbsent(signature, (k) -> { Preconditions.checkArgument(constructorArgs.length == constructorArgTypes.length, String.format( @@ -220,5 +220,6 @@ private static class ClassSignature { Class cls; Object[] constructorArgs; Class[] constructorArgTypes; + ElementMatcher extraMethodMatcher; } } From c5ac9483caebdb3e1576a1888636a4508a6855d2 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sat, 2 Mar 2024 19:15:19 +0100 Subject: [PATCH 3/5] moar --- src/main/java/io/appium/java_client/proxy/Helpers.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/main/java/io/appium/java_client/proxy/Helpers.java b/src/main/java/io/appium/java_client/proxy/Helpers.java index c06a5679d..8156cfece 100644 --- a/src/main/java/io/appium/java_client/proxy/Helpers.java +++ b/src/main/java/io/appium/java_client/proxy/Helpers.java @@ -114,7 +114,7 @@ public static T createProxy( Collection listeners, @Nullable ElementMatcher extraMethodMatcher ) { - var signature = ClassSignature.of(cls, constructorArgs, constructorArgTypes, extraMethodMatcher); + var signature = ClassSignature.of(cls, constructorArgTypes, extraMethodMatcher); var proxyClass = CACHED_PROXY_CLASSES.computeIfAbsent(signature, (k) -> { Preconditions.checkArgument(constructorArgs.length == constructorArgTypes.length, String.format( @@ -218,7 +218,6 @@ public static T createProxy( @Value(staticConstructor = "of") private static class ClassSignature { Class cls; - Object[] constructorArgs; Class[] constructorArgTypes; ElementMatcher extraMethodMatcher; } From e446e5d050d37693e34c6b372c9285b4318c45a4 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sat, 2 Mar 2024 19:33:19 +0100 Subject: [PATCH 4/5] linter --- src/main/java/io/appium/java_client/proxy/Helpers.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/io/appium/java_client/proxy/Helpers.java b/src/main/java/io/appium/java_client/proxy/Helpers.java index 8156cfece..4c45aa515 100644 --- a/src/main/java/io/appium/java_client/proxy/Helpers.java +++ b/src/main/java/io/appium/java_client/proxy/Helpers.java @@ -115,7 +115,7 @@ public static T createProxy( @Nullable ElementMatcher extraMethodMatcher ) { var signature = ClassSignature.of(cls, constructorArgTypes, extraMethodMatcher); - var proxyClass = CACHED_PROXY_CLASSES.computeIfAbsent(signature, (k) -> { + var proxyClass = CACHED_PROXY_CLASSES.computeIfAbsent(signature, k -> { Preconditions.checkArgument(constructorArgs.length == constructorArgTypes.length, String.format( "Constructor arguments array length %d must be equal to the types array length %d", From 6dfade17527f0da2674241e79d87dbb1e136d930 Mon Sep 17 00:00:00 2001 From: Mykola Mokhnach Date: Sun, 3 Mar 2024 08:15:13 +0100 Subject: [PATCH 5/5] Rename dataclass --- src/main/java/io/appium/java_client/proxy/Helpers.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/main/java/io/appium/java_client/proxy/Helpers.java b/src/main/java/io/appium/java_client/proxy/Helpers.java index 4c45aa515..af724ae43 100644 --- a/src/main/java/io/appium/java_client/proxy/Helpers.java +++ b/src/main/java/io/appium/java_client/proxy/Helpers.java @@ -50,7 +50,7 @@ public class Helpers { // the performance and to avoid extensive memory usage for our case, where // the amount of instrumented proxy classes we create is low in comparison to the amount // of proxy instances. - private static final ConcurrentMap> CACHED_PROXY_CLASSES = new ConcurrentHashMap<>(); + private static final ConcurrentMap> CACHED_PROXY_CLASSES = new ConcurrentHashMap<>(); private Helpers() { } @@ -114,7 +114,7 @@ public static T createProxy( Collection listeners, @Nullable ElementMatcher extraMethodMatcher ) { - var signature = ClassSignature.of(cls, constructorArgTypes, extraMethodMatcher); + var signature = ProxyClassSignature.of(cls, constructorArgTypes, extraMethodMatcher); var proxyClass = CACHED_PROXY_CLASSES.computeIfAbsent(signature, k -> { Preconditions.checkArgument(constructorArgs.length == constructorArgTypes.length, String.format( @@ -216,7 +216,7 @@ public static T createProxy( } @Value(staticConstructor = "of") - private static class ClassSignature { + private static class ProxyClassSignature { Class cls; Class[] constructorArgTypes; ElementMatcher extraMethodMatcher;