From 4e28f9e24c16da5eb606e5143a41e05572b9e1aa Mon Sep 17 00:00:00 2001 From: Kris De Volder Date: Fri, 21 Jun 2019 10:04:25 -0700 Subject: [PATCH] Optimise MemoizingProxy Allow re-use of proxy class instead of re-creating it every time. --- .../commons/commons-util/pom.xml | 11 +- .../vscode/commons/util/MemoizingProxy.java | 233 ++++++++++++------ .../commons/util/MemoizingProxyTest.java | 39 ++- headless-services/commons/pom.xml | 1 - 4 files changed, 201 insertions(+), 83 deletions(-) diff --git a/headless-services/commons/commons-util/pom.xml b/headless-services/commons/commons-util/pom.xml index 5b64f55cb..c5868aef2 100644 --- a/headless-services/commons/commons-util/pom.xml +++ b/headless-services/commons/commons-util/pom.xml @@ -1,4 +1,5 @@ - 4.0.0 commons-util @@ -49,13 +50,11 @@ reactor-core ${reactor-version} - - cglib - cglib - ${cglib-version} + net.bytebuddy + byte-buddy - + com.kotcrab.remark diff --git a/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/MemoizingProxy.java b/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/MemoizingProxy.java index 07f99e9b1..f4b1d09c1 100644 --- a/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/MemoizingProxy.java +++ b/headless-services/commons/commons-util/src/main/java/org/springframework/ide/vscode/commons/util/MemoizingProxy.java @@ -10,8 +10,10 @@ *******************************************************************************/ package org.springframework.ide.vscode.commons.util; +import static net.bytebuddy.matcher.ElementMatchers.*; + +import java.lang.reflect.Constructor; import java.lang.reflect.Method; -import java.lang.reflect.Modifier; import java.time.Duration; import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; @@ -19,9 +21,22 @@ import java.util.concurrent.TimeUnit; import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; -import net.sf.cglib.proxy.Enhancer; -import net.sf.cglib.proxy.MethodInterceptor; -import net.sf.cglib.proxy.MethodProxy; +import net.bytebuddy.ByteBuddy; +import net.bytebuddy.description.method.MethodDescription; +import net.bytebuddy.description.modifier.Visibility; +import net.bytebuddy.dynamic.DynamicType; +import net.bytebuddy.dynamic.scaffold.subclass.ConstructorStrategy; +import net.bytebuddy.implementation.MethodCall; +import net.bytebuddy.implementation.MethodDelegation; +import net.bytebuddy.implementation.MethodDelegation.WithCustomProperties; +import net.bytebuddy.implementation.bind.annotation.AllArguments; +import net.bytebuddy.implementation.bind.annotation.FieldProxy; +import net.bytebuddy.implementation.bind.annotation.FieldValue; +import net.bytebuddy.implementation.bind.annotation.Origin; +import net.bytebuddy.implementation.bind.annotation.RuntimeType; +import net.bytebuddy.implementation.bind.annotation.SuperCall; +import net.bytebuddy.jar.asm.Opcodes; +import net.bytebuddy.matcher.ElementMatcher.Junction; /** * Utility to instrument a given class, memoizing all it's zero-argument method invocations. @@ -29,7 +44,53 @@ import net.sf.cglib.proxy.MethodProxy; * it caches exception results as well as regularly returned values. */ public class MemoizingProxy { + + public interface Builder { + T build(Object... args); + } + private static final Junction CACHABLE_METHODS = + takesArguments(0).and(not(isStatic()).and(isPublic())); + + public interface IFieldProxy { + Object getValue(); + void setValue(Object value); + } + + private static final String F_CACHE = "__MemoizingProxy__cache"; + private static final String F_DURATION = "__MemoizingProxy__duration"; + + private static final MethodDelegation.WithCustomProperties METHOD_DELEGATION = MethodDelegation.withDefaultConfiguration() + .withBinders(FieldProxy.Binder.install(IFieldProxy.class)); + + public static class ConstructorInterceptor { + public static void intercept(@FieldProxy(F_CACHE) IFieldProxy fCache, @FieldValue(F_DURATION) long duration) { + fCache.setValue(CacheBuilder.newBuilder().expireAfterWrite(duration, TimeUnit.MILLISECONDS).build()); + } + } + + public static class MethodInterceptor { + @RuntimeType + public static Object intercept( + @Origin(cache = true) Method method, + @FieldValue(F_CACHE) Cache cache, + @SuperCall Callable zuper, + @AllArguments Object[] args + ) throws Exception { + synchronized (cache) { + String mname = method.getName(); + Result r = cache.get(mname, () -> new Result(() -> { + try { + return zuper.call(); + } catch (Throwable e) { + throw ExceptionUtil.exception(e); + } + })); + return r.get(); + } + } + } + static class Result { Throwable e; @@ -51,78 +112,108 @@ public class MemoizingProxy { } } - - /** - * Memoizes all zero-argument public methods for a given duration. - */ - @SuppressWarnings("unchecked") - public static T create(Class klass, Duration duration, Class[] argTypes, Object... args) { - Enhancer enhancer = new Enhancer(); - enhancer.setSuperclass(klass); - enhancer.setCallback(new MethodInterceptor() { - Cache cache = CacheBuilder.newBuilder() - .expireAfterWrite(duration.toMillis(), TimeUnit.MILLISECONDS) - .build(); - - public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable { - if (Modifier.isPublic(method.getModifiers()) && (args==null || args.length==0)) { - synchronized (cache) { - String mname = method.getName(); - Result r = cache.get(mname, () -> new Result(() -> { - try { - return proxy.invokeSuper(obj, args); - } catch (Throwable e) { - throw ExceptionUtil.exception(e); - } - })); - return r.get(); - } - } else { - return proxy.invokeSuper(obj, args); + public static Builder builder(Class klass, Duration duration, Class... argTypes) throws Exception { + DynamicType.Builder builder = new ByteBuddy() + .subclass(klass, ConstructorStrategy.Default.NO_CONSTRUCTORS) + .defineField(F_DURATION, long.class, Opcodes.ACC_FINAL | Opcodes.ACC_PRIVATE | Opcodes.ACC_STATIC).value(duration.toMillis()) + .defineField(F_CACHE, Cache.class, Opcodes.ACC_PRIVATE) + .method(CACHABLE_METHODS).intercept(MethodDelegation.to(MethodInterceptor.class)) + .defineConstructor(Visibility.PUBLIC).withParameters(argTypes).intercept( + MethodCall.invoke(klass.getConstructor(argTypes)).withAllArguments() + .andThen(METHOD_DELEGATION.to(ConstructorInterceptor.class)) + ); + + Constructor constructor = builder.make().load(klass.getClassLoader()).getLoaded().getConstructor(argTypes); + return new Builder() { + @Override + public T build(Object... args) { + try { + return constructor.newInstance(args); + } catch (Exception e) { + throw ExceptionUtil.unchecked(e); } } - }); - - return (T) enhancer.create(argTypes, args); + }; } - - public static class MemoizingProxyHandler implements MethodInterceptor { - - private final Object original; - private final Cache cache; - - public MemoizingProxyHandler(Object original, Duration cacheExpiresAfter) { - this.original = original; - this.cache = CacheBuilder.newBuilder() - .expireAfterWrite(cacheExpiresAfter.toMillis(), TimeUnit.MILLISECONDS) - .build(); + /** + * Deprecated: use the 'builder' method instead + */ + @Deprecated + public static T create(Class klass, Duration duration, Class[] argTypes, Object... args) { + try { + return builder(klass, duration, argTypes).build(args); + } catch (Exception e) { + throw ExceptionUtil.unchecked(e); } - - @Override - public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable { - if (Modifier.isPublic(method.getModifiers()) && (args == null || args.length < 2)) { - synchronized (cache) { - String mname = method.getName(); - - if (args != null && args.length == 1) { - mname += "-" + args[0].toString(); - } - - Result r = cache.get(mname, () -> new Result(() -> { - try { - return method.invoke(original, args); - } catch (Throwable e) { - throw ExceptionUtil.exception(e); - } - })); - return r.get(); - } - } else { - return method.invoke(original, args); - } - } - } + +// Enhancer enhancer = new Enhancer(); +// enhancer.setSuperclass(klass); +// enhancer.setCallback(new MethodInterceptor() { +// Cache cache = CacheBuilder.newBuilder() +// .expireAfterWrite(duration.toMillis(), TimeUnit.MILLISECONDS) +// .build(); +// +// public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable { +// if (Modifier.isPublic(method.getModifiers()) && (args==null || args.length==0)) { +// synchronized (cache) { +// String mname = method.getName(); +// Result r = cache.get(mname, () -> new Result(() -> { +// try { +// return proxy.invokeSuper(obj, args); +// } catch (Throwable e) { +// throw ExceptionUtil.exception(e); +// } +// })); +// return r.get(); +// } +// } else { +// return proxy.invokeSuper(obj, args); +// } +// } +// }); +// +// return (T) enhancer.create(argTypes, args); +// } + + +// public static class MemoizingProxyHandler implements MethodInterceptor { +// +// private final Object original; +// private final Cache cache; +// +// public MemoizingProxyHandler(Object original, Duration cacheExpiresAfter) { +// this.original = original; +// this.cache = CacheBuilder.newBuilder() +// .expireAfterWrite(cacheExpiresAfter.toMillis(), TimeUnit.MILLISECONDS) +// .build(); +// } +// +// @Override +// public Object intercept(Object obj, Method method, Object[] args, MethodProxy proxy) throws Throwable { +// if (Modifier.isPublic(method.getModifiers()) && (args == null || args.length < 2)) { +// synchronized (cache) { +// String mname = method.getName(); +// +// if (args != null && args.length == 1) { +// mname += "-" + args[0].toString(); +// } +// +// Result r = cache.get(mname, () -> new Result(() -> { +// try { +// return method.invoke(original, args); +// } catch (Throwable e) { +// throw ExceptionUtil.exception(e); +// } +// })); +// return r.get(); +// } +// } else { +// return method.invoke(original, args); +// } +// } +// +// } } diff --git a/headless-services/commons/commons-util/src/test/java/org/springframework/ide/vscode/commons/util/MemoizingProxyTest.java b/headless-services/commons/commons-util/src/test/java/org/springframework/ide/vscode/commons/util/MemoizingProxyTest.java index 4c619cc45..16c0ab992 100644 --- a/headless-services/commons/commons-util/src/test/java/org/springframework/ide/vscode/commons/util/MemoizingProxyTest.java +++ b/headless-services/commons/commons-util/src/test/java/org/springframework/ide/vscode/commons/util/MemoizingProxyTest.java @@ -11,7 +11,9 @@ package org.springframework.ide.vscode.commons.util; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; +import java.io.IOException; import java.time.Duration; import java.util.ArrayList; import java.util.List; @@ -38,6 +40,11 @@ public class MemoizingProxyTest { this.otherConstructorArg = otherConstructorArg; } + public int throwsError() throws IOException { + invocations.add("throwsError"); + throw new IOException("Problem"); + } + public String getName() { invocations.add("getName"); return name; @@ -53,6 +60,11 @@ public class MemoizingProxyTest { return getName(); } } + + private TestSubject defaultTestSubject() { + return MemoizingProxy.create(TestSubject.class, Duration.ofMinutes(1), CONSTRUCTOR_ARG_TYPES, "Johny", 45); + } + private void assertInvocations(String...expectedInvocations) { assertEquals(ImmutableList.copyOf(expectedInvocations), proxy.invocations); proxy.invocations.clear(); @@ -76,23 +88,40 @@ public class MemoizingProxyTest { @Test public void constructorCalled() throws Exception { - this.proxy = MemoizingProxy.create(TestSubject.class, Duration.ofMinutes(1), CONSTRUCTOR_ARG_TYPES, "Johny", 45); + this.proxy = defaultTestSubject(); assertEquals(proxy.name, "Johny"); //Constructor was called so name should be set assertEquals(proxy.otherConstructorArg, 45); //Constructor was called so name should be set } @Test public void zeroArgMethodCached() throws Exception { - this.proxy = MemoizingProxy.create(TestSubject.class, Duration.ofMinutes(1), CONSTRUCTOR_ARG_TYPES, "Johny", 45); + this.proxy = defaultTestSubject(); assertEquals(proxy.getName(), "Johny"); assertInvocations("getName"); assertEquals(proxy.getName(), "Johny"); assertInvocations(/*NONE*/); } + + @Test public void exceptionsCached() throws Exception { + this.proxy = defaultTestSubject(); + callMethodThatThrows(); + assertInvocations("throwsError"); + callMethodThatThrows(); + assertInvocations(/*NONE*/); + } + + private void callMethodThatThrows() { + try { + this.proxy.throwsError(); + fail("should have thrown"); + } catch (IOException e) { + assertEquals("Problem", e.getMessage()); + } + } @Test public void methodWithArgumentNotCached() throws Exception { - this.proxy = MemoizingProxy.create(TestSubject.class, Duration.ofMinutes(1), CONSTRUCTOR_ARG_TYPES, "Johny", 45); + this.proxy = defaultTestSubject(); assertInvocations(/*NONE*/); assertEquals(proxy.getMessage(" whatever"), "Johny whatever"); @@ -105,7 +134,7 @@ public class MemoizingProxyTest { @Test public void callsViaThisCached() throws Exception { - this.proxy = MemoizingProxy.create(TestSubject.class, Duration.ofMinutes(1), CONSTRUCTOR_ARG_TYPES, "Johny", 45); + this.proxy = defaultTestSubject(); assertInvocations(/*NONE*/); assertEquals(proxy.getMessage(" whatever"), "Johny whatever"); @@ -126,7 +155,7 @@ public class MemoizingProxyTest { @Test public void multiThreaded() throws Exception { - this.proxy = MemoizingProxy.create(TestSubject.class, Duration.ofMinutes(1), CONSTRUCTOR_ARG_TYPES, "Johny", 45); + this.proxy = defaultTestSubject(); ExecutorService manyThreads = Executors.newFixedThreadPool(100); Future[] futures = new Future[1000]; diff --git a/headless-services/commons/pom.xml b/headless-services/commons/pom.xml index dc5b49838..5816d213a 100644 --- a/headless-services/commons/pom.xml +++ b/headless-services/commons/pom.xml @@ -113,7 +113,6 @@ 2.5.0 2.10 0.7.2 - 3.2.7 3.8.0.RELEASE 3.1.5.RELEASE