From 0ed5b7b4a7b1102e90757b38e0f4b402173e26ed Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Thu, 2 Feb 2023 13:47:06 +0100 Subject: [PATCH 1/3] ArC - reduce allocations for intercepted methods - forwarding lambdas are stateless and thus may become part of immutable InterceptedMethodMetadata - note that metadata are shared accross all invocations of an intercepted method --- .../InterceptedStaticMethodsProcessor.java | 17 +-- .../arc/processor/MethodDescriptors.java | 6 +- .../arc/processor/SubclassGenerator.java | 109 +++++++------- .../arc/impl/AbstractInvocationContext.java | 52 ++----- .../AroundConstructInvocationContext.java | 20 ++- .../impl/AroundInvokeInvocationContext.java | 138 ++++++++++++++---- .../arc/impl/InterceptedMethodMetadata.java | 11 +- .../arc/impl/InterceptedStaticMethods.java | 30 +--- .../quarkus/arc/impl/InvocationContexts.java | 19 +-- .../LifecycleCallbackInvocationContext.java | 31 +++- ...ceptor.java => FirstParamInterceptor.java} | 3 +- .../parameters/ParamInterceptorTest.java | 9 +- .../parameters/SecondParamInterceptor.java | 24 +++ .../parameters/ThirdParamInterceptor.java | 24 +++ 14 files changed, 306 insertions(+), 187 deletions(-) rename independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/{ParamInterceptor.java => FirstParamInterceptor.java} (95%) create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/SecondParamInterceptor.java create mode 100644 independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/ThirdParamInterceptor.java diff --git a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/staticmethods/InterceptedStaticMethodsProcessor.java b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/staticmethods/InterceptedStaticMethodsProcessor.java index c8716fb2ed2d2..e570438c545ae 100644 --- a/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/staticmethods/InterceptedStaticMethodsProcessor.java +++ b/extensions/arc/deployment/src/main/java/io/quarkus/arc/deployment/staticmethods/InterceptedStaticMethodsProcessor.java @@ -46,7 +46,6 @@ import io.quarkus.arc.impl.CreationalContextImpl; import io.quarkus.arc.impl.InterceptedMethodMetadata; import io.quarkus.arc.impl.InterceptedStaticMethods; -import io.quarkus.arc.impl.InterceptedStaticMethods.InterceptedStaticMethod; import io.quarkus.arc.processor.AnnotationLiteralProcessor; import io.quarkus.arc.processor.BeanProcessor; import io.quarkus.arc.processor.DotNames; @@ -77,7 +76,7 @@ public class InterceptedStaticMethodsProcessor { private static final Logger LOGGER = Logger.getLogger(InterceptedStaticMethodsProcessor.class); static final MethodDescriptor INTERCEPTED_STATIC_METHODS_REGISTER = MethodDescriptor - .ofMethod(InterceptedStaticMethods.class, "register", void.class, String.class, InterceptedStaticMethod.class); + .ofMethod(InterceptedStaticMethods.class, "register", void.class, String.class, InterceptedMethodMetadata.class); static final MethodDescriptor INTERCEPTED_STATIC_METHODS_AROUND_INVOKE = MethodDescriptor .ofMethod(InterceptedStaticMethods.class, "aroundInvoke", Object.class, String.class, Object[].class); @@ -322,23 +321,19 @@ private String implementInit(IndexView index, ClassCreator initializer, } } + // Create forwarding function + ResultHandle forwardingFunc = createForwardingFunction(init, interceptedStaticMethod.getTarget(), method); + // Now create metadata for the given intercepted method ResultHandle metadataHandle = init.newInstance(MethodDescriptors.INTERCEPTED_METHOD_METADATA_CONSTRUCTOR, - chainHandle, methodHandle, bindingsHandle); + chainHandle, methodHandle, bindingsHandle, forwardingFunc); // Needed when running on native image reflectiveMethods.produce(new ReflectiveMethodBuildItem(method)); - // Create forwarding function - ResultHandle forwardingFunc = createForwardingFunction(init, interceptedStaticMethod.getTarget(), method); - - ResultHandle staticMethodHandle = init.newInstance( - MethodDescriptor.ofConstructor(InterceptedStaticMethod.class, Function.class, InterceptedMethodMetadata.class), - forwardingFunc, metadataHandle); - // Call InterceptedStaticMethods.register() init.invokeStaticMethod(INTERCEPTED_STATIC_METHODS_REGISTER, init.load(interceptedStaticMethod.getHash()), - staticMethodHandle); + metadataHandle); init.returnValue(null); return name; } diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java index 970c768e45c31..bc5f890b16348 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/MethodDescriptors.java @@ -163,9 +163,7 @@ public final class MethodDescriptors { public static final MethodDescriptor INVOCATION_CONTEXTS_PERFORM_AROUND_INVOKE = MethodDescriptor.ofMethod( InvocationContexts.class, - "performAroundInvoke", - Object.class, Object.class, Method.class, Function.class, Object[].class, List.class, - Set.class); + "performAroundInvoke", Object.class, Object.class, Object[].class, InterceptedMethodMetadata.class); public static final MethodDescriptor INVOCATION_CONTEXTS_AROUND_CONSTRUCT = MethodDescriptor.ofMethod( InvocationContexts.class, @@ -230,7 +228,7 @@ public final class MethodDescriptors { public static final MethodDescriptor INTERCEPTED_METHOD_METADATA_CONSTRUCTOR = MethodDescriptor.ofConstructor( InterceptedMethodMetadata.class, - List.class, Method.class, Set.class); + List.class, Method.class, Set.class, Function.class); public static final MethodDescriptor CREATIONAL_CTX_HAS_DEPENDENT_INSTANCES = MethodDescriptor.ofMethod( CreationalContextImpl.class, diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java index dcd1c2cfa8696..c19e063838bc4 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/SubclassGenerator.java @@ -4,7 +4,6 @@ import static org.objectweb.asm.Opcodes.ACC_FINAL; import static org.objectweb.asm.Opcodes.ACC_PRIVATE; -import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.util.ArrayList; import java.util.Collection; @@ -74,12 +73,6 @@ public class SubclassGenerator extends AbstractGenerator { protected static final String FIELD_NAME_PREDESTROYS = "arc$preDestroys"; protected static final String FIELD_NAME_CONSTRUCTED = "arc$constructed"; - protected static final FieldDescriptor FIELD_METADATA_METHOD = FieldDescriptor.of(InterceptedMethodMetadata.class, "method", - Method.class); - protected static final FieldDescriptor FIELD_METADATA_CHAIN = FieldDescriptor.of(InterceptedMethodMetadata.class, "chain", - List.class); - protected static final FieldDescriptor FIELD_METADATA_BINDINGS = FieldDescriptor.of(InterceptedMethodMetadata.class, - "bindings", Set.class); private final Predicate applicationClassPredicate; private final Set existingClasses; @@ -349,6 +342,7 @@ public String apply(List keys) { } MethodDescriptor methodDescriptor = MethodDescriptor.of(method); + MethodDescriptor originalMethodDescriptor = MethodDescriptor.of(method); InterceptionInfo interception = bean.getInterceptedMethods().get(method); DecorationInfo decoration = bean.getDecoratedMethods().get(method); MethodDescriptor forwardDescriptor = forwardingMethods.get(methodDescriptor); @@ -392,10 +386,58 @@ public String apply(List keys) { initMetadataMethodFinal.getMethodParam(1), initMetadataMethodFinal.load(bindingKey)); }); + DecoratorInfo decorator = decoration != null ? decoration.decorators.get(0) : null; + ResultHandle decoratorHandle = null; + if (decorator != null) { + decoratorHandle = initMetadataMethod.readInstanceField(FieldDescriptor.of(subclass.getClassName(), + decorator.getIdentifier(), Object.class.getName()), initMetadataMethod.getThis()); + } + + // Instantiate the forwarding function + // Function forward = ctx -> super.foo((java.lang.String)ctx.getParameters()[0]) + FunctionCreator func = initMetadataMethod.createFunction(Function.class); + BytecodeCreator funcBytecode = func.getBytecode(); + ResultHandle ctxHandle = funcBytecode.getMethodParam(0); + ResultHandle[] superParamHandles; + if (parameters.isEmpty()) { + superParamHandles = new ResultHandle[0]; + } else { + superParamHandles = new ResultHandle[parameters.size()]; + ResultHandle ctxParamsHandle = funcBytecode.invokeInterfaceMethod( + MethodDescriptor.ofMethod(InvocationContext.class, "getParameters", Object[].class), + ctxHandle); + // autoboxing is handled inside Gizmo + for (int i = 0; i < superParamHandles.length; i++) { + superParamHandles[i] = funcBytecode.readArrayValue(ctxParamsHandle, i); + } + } + // If a decorator is bound then invoke the method upon the decorator instance instead of the generated forwarding method + if (decorator != null) { + AssignableResultHandle funDecoratorInstance = funcBytecode.createVariable(Object.class); + funcBytecode.assign(funDecoratorInstance, decoratorHandle); + String declaringClass = decorator.getBeanClass().toString(); + if (decorator.isAbstract()) { + String baseName = DecoratorGenerator.createBaseName(decorator.getTarget().get().asClass()); + String targetPackage = DotNames.packageName(decorator.getProviderType().name()); + declaringClass = generatedNameFromTarget(targetPackage, baseName, + DecoratorGenerator.ABSTRACT_IMPL_SUFFIX); + } + MethodDescriptor virtualMethodDescriptor = MethodDescriptor.ofMethod(declaringClass, + originalMethodDescriptor.getName(), + originalMethodDescriptor.getReturnType(), originalMethodDescriptor.getParameterTypes()); + funcBytecode + .returnValue(funcBytecode.invokeVirtualMethod(virtualMethodDescriptor, funDecoratorInstance, + superParamHandles)); + } else { + ResultHandle superResult = funcBytecode.invokeVirtualMethod(forwardDescriptor, initMetadataMethod.getThis(), + superParamHandles); + funcBytecode.returnValue(superResult != null ? superResult : funcBytecode.loadNull()); + } + // Now create metadata for the given intercepted method ResultHandle methodMetadataHandle = initMetadataMethod.newInstance( MethodDescriptors.INTERCEPTED_METHOD_METADATA_CONSTRUCTOR, - chainHandle, methodHandle, bindingsHandle); + chainHandle, methodHandle, bindingsHandle, func.getInstance()); FieldDescriptor metadataField = FieldDescriptor.of(subclass.getClassName(), "arc$" + methodIdx++, InterceptedMethodMetadata.class.getName()); @@ -769,52 +811,6 @@ private void createInterceptedMethod(ClassOutput classOutput, BeanInfo bean, Met notConstructed.returnValue(notConstructed.invokeVirtualMethod(forwardMethod, notConstructed.getThis(), params)); } - ResultHandle decoratorHandle = null; - if (decorator != null) { - decoratorHandle = interceptedMethod.readInstanceField(FieldDescriptor.of(subclass.getClassName(), - decorator.getIdentifier(), Object.class.getName()), interceptedMethod.getThis()); - } - - // Forwarding function - // Function forward = ctx -> super.foo((java.lang.String)ctx.getParameters()[0]) - FunctionCreator func = interceptedMethod.createFunction(Function.class); - BytecodeCreator funcBytecode = func.getBytecode(); - ResultHandle ctxHandle = funcBytecode.getMethodParam(0); - ResultHandle[] superParamHandles; - if (parameters.isEmpty()) { - superParamHandles = new ResultHandle[0]; - } else { - superParamHandles = new ResultHandle[parameters.size()]; - ResultHandle ctxParamsHandle = funcBytecode.invokeInterfaceMethod( - MethodDescriptor.ofMethod(InvocationContext.class, "getParameters", Object[].class), - ctxHandle); - // autoboxing is handled inside Gizmo - for (int i = 0; i < superParamHandles.length; i++) { - superParamHandles[i] = funcBytecode.readArrayValue(ctxParamsHandle, i); - } - } - // If a decorator is bound then invoke the method upon the decorator instance instead of the generated forwarding method - if (decorator != null) { - AssignableResultHandle funDecoratorInstance = funcBytecode.createVariable(Object.class); - funcBytecode.assign(funDecoratorInstance, decoratorHandle); - String declaringClass = decorator.getBeanClass().toString(); - if (decorator.isAbstract()) { - String baseName = DecoratorGenerator.createBaseName(decorator.getTarget().get().asClass()); - String targetPackage = DotNames.packageName(decorator.getProviderType().name()); - declaringClass = generatedNameFromTarget(targetPackage, baseName, DecoratorGenerator.ABSTRACT_IMPL_SUFFIX); - } - MethodDescriptor methodDescriptor = MethodDescriptor.ofMethod( - declaringClass, originalMethodDescriptor.getName(), - originalMethodDescriptor.getReturnType(), originalMethodDescriptor.getParameterTypes()); - funcBytecode - .returnValue(funcBytecode.invokeVirtualMethod(methodDescriptor, funDecoratorInstance, superParamHandles)); - - } else { - ResultHandle superResult = funcBytecode.invokeVirtualMethod(forwardMethod, interceptedMethod.getThis(), - superParamHandles); - funcBytecode.returnValue(superResult != null ? superResult : funcBytecode.loadNull()); - } - for (Type declaredException : method.exceptions()) { interceptedMethod.addException(declaredException.name().toString()); } @@ -857,10 +853,7 @@ private void createInterceptedMethod(ClassOutput classOutput, BeanInfo bean, Met // InvocationContexts.performAroundInvoke(...) ResultHandle methodMetadataHandle = tryCatch.readInstanceField(metadataField, tryCatch.getThis()); ResultHandle ret = tryCatch.invokeStaticMethod(MethodDescriptors.INVOCATION_CONTEXTS_PERFORM_AROUND_INVOKE, - tryCatch.getThis(), - tryCatch.readInstanceField(FIELD_METADATA_METHOD, methodMetadataHandle), func.getInstance(), paramsHandle, - tryCatch.readInstanceField(FIELD_METADATA_CHAIN, methodMetadataHandle), - tryCatch.readInstanceField(FIELD_METADATA_BINDINGS, methodMetadataHandle)); + tryCatch.getThis(), paramsHandle, methodMetadataHandle); tryCatch.returnValue(ret); } diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AbstractInvocationContext.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AbstractInvocationContext.java index 8bae659f2a40b..3feff00e1769e 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AbstractInvocationContext.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AbstractInvocationContext.java @@ -2,6 +2,7 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Constructor; +import java.lang.reflect.Executable; import java.lang.reflect.Method; import java.util.ArrayList; import java.util.Arrays; @@ -16,25 +17,14 @@ abstract class AbstractInvocationContext implements ArcInvocationContext { private static final Object[] EMPTY_PARAMS = new Object[0]; - protected final Method method; - protected final Constructor constructor; - protected final Set interceptorBindings; - protected final List chain; protected Object target; protected Object[] parameters; protected ContextDataMap contextData; - protected AbstractInvocationContext(Object target, Method method, - Constructor constructor, - Object[] parameters, ContextDataMap contextData, - Set interceptorBindings, List chain) { + protected AbstractInvocationContext(Object target, Object[] parameters, ContextDataMap contextData) { this.target = target; - this.method = method; - this.constructor = constructor; this.parameters = parameters != null ? parameters : EMPTY_PARAMS; - this.contextData = contextData != null ? contextData : new ContextDataMap(interceptorBindings); - this.interceptorBindings = interceptorBindings; - this.chain = chain; + this.contextData = contextData; } @Override @@ -42,15 +32,10 @@ public Map getContextData() { return contextData; } - @Override - public Set getInterceptorBindings() { - return interceptorBindings; - } - @SuppressWarnings("unchecked") @Override public T findIterceptorBinding(Class annotationType) { - for (Annotation annotation : interceptorBindings) { + for (Annotation annotation : getInterceptorBindings()) { if (annotation.annotationType().equals(annotationType)) { return (T) annotation; } @@ -62,7 +47,7 @@ public T findIterceptorBinding(Class annotationType) { @Override public List findIterceptorBindings(Class annotationType) { List found = new ArrayList<>(); - for (Annotation annotation : (Set) interceptorBindings) { + for (Annotation annotation : (Set) getInterceptorBindings()) { if (annotation.annotationType().equals(annotationType)) { found.add((T) annotation); } @@ -70,25 +55,9 @@ public List findIterceptorBindings(Class annotation return found; } - @Override - public Method getMethod() { - return method; - } - - @Override - public Object[] getParameters() { - return parameters; - } - - @Override - public void setParameters(Object[] params) { - validateParameters(params); - this.parameters = params; - } - - protected void validateParameters(Object[] params) { + protected void validateParameters(Executable executable, Object[] params) { int newParametersCount = Objects.requireNonNull(params).length; - Class[] parameterTypes = method.getParameterTypes(); + Class[] parameterTypes = executable.getParameterTypes(); if (parameterTypes.length != newParametersCount) { throw new IllegalArgumentException( "Wrong number of parameters - method has " + Arrays.toString(parameterTypes) + ", attempting to set " @@ -108,6 +77,11 @@ protected void validateParameters(Object[] params) { } } + @Override + public Method getMethod() { + return null; + } + @Override public Object getTarget() { return target; @@ -120,7 +94,7 @@ public Object getTimer() { @Override public Constructor getConstructor() { - return constructor; + return null; } } diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundConstructInvocationContext.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundConstructInvocationContext.java index 24e4328e540be..532f9a51cb552 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundConstructInvocationContext.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundConstructInvocationContext.java @@ -11,16 +11,34 @@ */ class AroundConstructInvocationContext extends LifecycleCallbackInvocationContext { + private final Constructor constructor; private final Supplier aroundConstructForward; AroundConstructInvocationContext(Constructor constructor, Object[] parameters, Set interceptorBindings, List chain, Supplier aroundConstructForward) { - super(null, constructor, parameters, interceptorBindings, chain); + super(null, parameters, interceptorBindings, chain); this.aroundConstructForward = aroundConstructForward; + this.constructor = constructor; } protected void interceptorChainCompleted() throws Exception { target = aroundConstructForward.get(); } + @Override + public Constructor getConstructor() { + return constructor; + } + + @Override + public Object[] getParameters() { + return parameters; + } + + @Override + public void setParameters(Object[] params) { + validateParameters(constructor, params); + this.parameters = params; + } + } diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundInvokeInvocationContext.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundInvokeInvocationContext.java index c2d76f8b092e9..4a0fd83943795 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundInvokeInvocationContext.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/AroundInvokeInvocationContext.java @@ -1,57 +1,77 @@ package io.quarkus.arc.impl; import java.lang.annotation.Annotation; +import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.List; +import java.util.Map; import java.util.Set; -import java.util.function.Function; -import jakarta.interceptor.InvocationContext; +import io.quarkus.arc.ArcInvocationContext; /** - * Special type of InvocationContext for AroundInvoke interceptors. + * An {@link javax.interceptor.InvocationContext} for {@link javax.interceptor.AroundInvoke} interceptors. *

- * A new instance of {@link AroundInvokeInvocationContext} is created for each interceptor in the chain. This does not comply - * with the spec but allows for "asynchronous continuation" of an interceptor chain execution. In other words, it is possible to - * "cut off" the chain (interceptors executed before dispatch return immediately) and execute all remaining interceptors - * asynchronously, possibly on a different thread. + * A new instance is created for the first interceptor in the chain. Furthermore, subsequent interceptors receive a new instance + * of {@link NextAroundInvokeInvocationContext}. This does not comply with the spec but allows for "asynchronous continuation" + * of an interceptor chain execution. In other words, it is possible to "cut off" the chain (interceptors executed before + * dispatch return immediately) and execute all remaining interceptors asynchronously, possibly on a different thread. *

* Note that context data and method parameters are mutable and are not guarded/synchronized. We expect them to be modified * before or after dispatch. If modified before and after dispatch an unpredictable behavior may occur. + *

+ * Note that {@link #getParameters()} only reflects modifications of the current interceptor. If an interceptor with higher + * priority in the same chain calls {@link #setParameters(Object[])} then the changes are not reflected. + * */ class AroundInvokeInvocationContext extends AbstractInvocationContext { - private final int position; - private final Function aroundInvokeForward; + private final InterceptedMethodMetadata metadata; + + AroundInvokeInvocationContext(Object target, Object[] args, InterceptedMethodMetadata metadata) { + super(target, args, new ContextDataMap(metadata.bindings)); + this.metadata = metadata; + } - AroundInvokeInvocationContext(Object target, Method method, Object[] parameters, - ContextDataMap contextData, Set interceptorBindings, int position, - List chain, Function aroundInvokeForward) { - super(target, method, null, parameters, contextData, interceptorBindings, chain); - this.position = position; - this.aroundInvokeForward = aroundInvokeForward; + static Object perform(Object target, Object[] args, InterceptedMethodMetadata metadata) throws Exception { + return metadata.chain.get(0).invoke(new AroundInvokeInvocationContext(target, args, metadata)); } - static Object perform(Object target, Method method, - Function aroundInvokeForward, Object[] parameters, - List chain, - Set interceptorBindings) throws Exception { + @Override + public Set getInterceptorBindings() { + return metadata.bindings; + } - return chain.get(0).invoke(new AroundInvokeInvocationContext(target, method, - parameters, null, interceptorBindings, 1, chain, aroundInvokeForward)); + public Method getMethod() { + return metadata.method; + } + + @Override + public Object[] getParameters() { + return parameters; + } + + @Override + public void setParameters(Object[] params) { + validateParameters(metadata.method, params); + this.parameters = params; } @Override public Object proceed() throws Exception { + return proceed(1); + } + + private Object proceed(int currentPosition) throws Exception { try { - if (position < chain.size()) { + if (currentPosition < metadata.chain.size()) { // Invoke the next interceptor in the chain - return chain.get(position).invoke(new AroundInvokeInvocationContext(target, method, - parameters, contextData, interceptorBindings, position + 1, chain, aroundInvokeForward)); + return metadata.chain.get(currentPosition) + .invoke(new NextAroundInvokeInvocationContext(currentPosition + 1, parameters)); } else { // Invoke the target method - return aroundInvokeForward.apply(this); + return metadata.aroundInvokeForward.apply(this); } } catch (InvocationTargetException e) { Throwable cause = e.getCause(); @@ -65,4 +85,72 @@ public Object proceed() throws Exception { } } + class NextAroundInvokeInvocationContext implements ArcInvocationContext { + + private final int position; + protected Object[] parameters; + + public NextAroundInvokeInvocationContext(int position, Object[] parameters) { + this.position = position; + this.parameters = parameters; + } + + @Override + public Object proceed() throws Exception { + return AroundInvokeInvocationContext.this.proceed(position); + } + + @Override + public Object getTarget() { + return AroundInvokeInvocationContext.this.getTarget(); + } + + @Override + public Object getTimer() { + return AroundInvokeInvocationContext.this.getTimer(); + } + + @Override + public Method getMethod() { + return AroundInvokeInvocationContext.this.getMethod(); + } + + @Override + public Constructor getConstructor() { + return AroundInvokeInvocationContext.this.getConstructor(); + } + + @Override + public Object[] getParameters() { + return parameters; + } + + @Override + public void setParameters(Object[] params) { + validateParameters(metadata.method, params); + this.parameters = params; + } + + @Override + public Map getContextData() { + return AroundInvokeInvocationContext.this.getContextData(); + } + + @Override + public Set getInterceptorBindings() { + return AroundInvokeInvocationContext.this.getInterceptorBindings(); + } + + @Override + public T findIterceptorBinding(Class annotationType) { + return AroundInvokeInvocationContext.this.findIterceptorBinding(annotationType); + } + + @Override + public List findIterceptorBindings(Class annotationType) { + return AroundInvokeInvocationContext.this.findIterceptorBindings(annotationType); + } + + } + } diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InterceptedMethodMetadata.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InterceptedMethodMetadata.java index 58a7c429def0f..fe59b437c7636 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InterceptedMethodMetadata.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InterceptedMethodMetadata.java @@ -4,17 +4,26 @@ import java.lang.reflect.Method; import java.util.List; import java.util.Set; +import java.util.function.Function; +import jakarta.interceptor.InvocationContext; + +/** + * Immutable metadata for a specific intercepted method. + */ public class InterceptedMethodMetadata { public final List chain; public final Method method; public final Set bindings; + public final Function aroundInvokeForward; - public InterceptedMethodMetadata(List chain, Method method, Set bindings) { + public InterceptedMethodMetadata(List chain, Method method, Set bindings, + Function aroundInvokeForward) { this.chain = chain; this.method = method; this.bindings = bindings; + this.aroundInvokeForward = aroundInvokeForward; } } diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InterceptedStaticMethods.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InterceptedStaticMethods.java index 80c51464ecc36..cbe8e528d55a2 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InterceptedStaticMethods.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InterceptedStaticMethods.java @@ -2,44 +2,28 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.function.Function; - -import jakarta.interceptor.InvocationContext; public final class InterceptedStaticMethods { - private static final ConcurrentMap METHODS = new ConcurrentHashMap<>(); + private static final ConcurrentMap METADATA = new ConcurrentHashMap<>(); private InterceptedStaticMethods() { } - public static void register(String key, InterceptedStaticMethod method) { - METHODS.putIfAbsent(key, method); + public static void register(String key, InterceptedMethodMetadata metadata) { + METADATA.putIfAbsent(key, metadata); } public static Object aroundInvoke(String key, Object[] args) throws Exception { - InterceptedStaticMethod method = METHODS.get(key); - if (method == null) { + InterceptedMethodMetadata metadata = METADATA.get(key); + if (metadata == null) { throw new IllegalArgumentException("Intercepted method metadata not found for key: " + key); } - return InvocationContexts.performAroundInvoke(null, method.metadata.method, method.forward, args, method.metadata.chain, - method.metadata.bindings); - } - - public static final class InterceptedStaticMethod { - - final Function forward; - final InterceptedMethodMetadata metadata; - - public InterceptedStaticMethod(Function forward, InterceptedMethodMetadata metadata) { - this.forward = forward; - this.metadata = metadata; - } - + return InvocationContexts.performAroundInvoke(null, args, metadata); } static void clear() { - METHODS.clear(); + METADATA.clear(); } } diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InvocationContexts.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InvocationContexts.java index 5132867850d2f..51aee7b76d825 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InvocationContexts.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/InvocationContexts.java @@ -2,10 +2,8 @@ import java.lang.annotation.Annotation; import java.lang.reflect.Constructor; -import java.lang.reflect.Method; import java.util.List; import java.util.Set; -import java.util.function.Function; import java.util.function.Supplier; import jakarta.interceptor.InvocationContext; @@ -18,19 +16,14 @@ private InvocationContexts() { /** * * @param target - * @param method - * @param aroundInvokeForward * @param args - * @param chain - * @param interceptorBindings + * @param metadata * @return the return value * @throws Exception */ - public static Object performAroundInvoke(Object target, Method method, - Function aroundInvokeForward, Object[] args, - List chain, - Set interceptorBindings) throws Exception { - return AroundInvokeInvocationContext.perform(target, method, aroundInvokeForward, args, chain, interceptorBindings); + public static Object performAroundInvoke(Object target, Object[] args, InterceptedMethodMetadata metadata) + throws Exception { + return AroundInvokeInvocationContext.perform(target, args, metadata); } /** @@ -42,7 +35,7 @@ public static Object performAroundInvoke(Object target, Method method, */ public static InvocationContext postConstruct(Object target, List chain, Set interceptorBindings) { - return new LifecycleCallbackInvocationContext(target, null, null, interceptorBindings, chain); + return new LifecycleCallbackInvocationContext(target, null, interceptorBindings, chain); } /** @@ -54,7 +47,7 @@ public static InvocationContext postConstruct(Object target, List chain, Set interceptorBindings) { - return new LifecycleCallbackInvocationContext(target, null, null, interceptorBindings, chain); + return new LifecycleCallbackInvocationContext(target, null, interceptorBindings, chain); } /** diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/LifecycleCallbackInvocationContext.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/LifecycleCallbackInvocationContext.java index 6aa8ee4e6e48b..3d70c8c910fd7 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/LifecycleCallbackInvocationContext.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/LifecycleCallbackInvocationContext.java @@ -1,23 +1,27 @@ package io.quarkus.arc.impl; import java.lang.annotation.Annotation; -import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; import java.util.List; import java.util.Set; /** - * A simple InvocationContext implementation used for PostConstruct and PreDestroy callbacks. + * A simple stateful {@link javax.interceptor.InvocationContext} implementation used for {@link javax.annotation.PostConstruct} + * and {@link javax.annotation.PreDestroy} callbacks. *

* All lifecycle callback interceptors of a specific chain must be invoked on the same thread. */ class LifecycleCallbackInvocationContext extends AbstractInvocationContext { + protected final Set bindings; + protected final List chain; private int position = 0; - LifecycleCallbackInvocationContext(Object target, Constructor constructor, Object[] parameters, - Set interceptorBindings, List chain) { - super(target, null, constructor, parameters, null, interceptorBindings, chain); + LifecycleCallbackInvocationContext(Object target, Object[] parameters, + Set bindings, List chain) { + super(target, parameters, new ContextDataMap(bindings)); + this.chain = chain; + this.bindings = bindings; } @Override @@ -44,11 +48,26 @@ public Object proceed() throws Exception { } } + @Override + public Set getInterceptorBindings() { + return bindings; + } + + @Override + public Object[] getParameters() { + throw new IllegalStateException(); + } + + @Override + public void setParameters(Object[] params) { + throw new IllegalStateException(); + } + protected void interceptorChainCompleted() throws Exception { // No-op } - protected Object invokeNext() throws Exception { + private Object invokeNext() throws Exception { try { return chain.get(position++).invoke(this); } finally { diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/ParamInterceptor.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/FirstParamInterceptor.java similarity index 95% rename from independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/ParamInterceptor.java rename to independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/FirstParamInterceptor.java index 0182b0ad641cd..6ee50a46d13c5 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/ParamInterceptor.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/FirstParamInterceptor.java @@ -8,11 +8,10 @@ @Simple @Priority(1) @Interceptor -public class ParamInterceptor { +public class FirstParamInterceptor { @AroundInvoke Object interceptParameters(InvocationContext ctx) throws Exception { - Object[] params = ctx.getParameters(); if (params.length == 1 && params[0] != null) { if (params[0] instanceof CharSequence) { diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/ParamInterceptorTest.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/ParamInterceptorTest.java index d27867c112d53..baac156da39d9 100644 --- a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/ParamInterceptorTest.java +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/ParamInterceptorTest.java @@ -15,7 +15,8 @@ public class ParamInterceptorTest { @RegisterExtension - public ArcTestContainer container = new ArcTestContainer(SimpleBean.class, Simple.class, ParamInterceptor.class); + public ArcTestContainer container = new ArcTestContainer(SimpleBean.class, Simple.class, FirstParamInterceptor.class, + SecondParamInterceptor.class, ThirdParamInterceptor.class); @Test public void testInterception() { @@ -40,13 +41,13 @@ public void testInterception() { }); simpleBean.setPrimitiveIntVal(0); - assertEquals("123456", simpleBean.getVal()); + assertEquals("123458", simpleBean.getVal()); simpleBean.setIntVal(1); - assertEquals("123456", simpleBean.getVal()); + assertEquals("123458", simpleBean.getVal()); simpleBean.setNumberVal(2L); - assertEquals("123456", simpleBean.getVal()); + assertEquals("123458", simpleBean.getVal()); handle.destroy(); } diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/SecondParamInterceptor.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/SecondParamInterceptor.java new file mode 100644 index 0000000000000..3badce0fd7be0 --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/SecondParamInterceptor.java @@ -0,0 +1,24 @@ +package io.quarkus.arc.test.interceptors.parameters; + +import jakarta.annotation.Priority; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InvocationContext; + +@Simple +@Priority(2) +@Interceptor +public class SecondParamInterceptor { + + @AroundInvoke + Object interceptParameters(InvocationContext ctx) throws Exception { + Object[] params = ctx.getParameters(); + if (params.length == 1 && params[0] != null) { + if (params[0] instanceof Number) { + params[0] = ((Number) params[0]).intValue() + 1; + } + ctx.setParameters(params); + } + return ctx.proceed(); + } +} diff --git a/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/ThirdParamInterceptor.java b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/ThirdParamInterceptor.java new file mode 100644 index 0000000000000..c26a54c42114e --- /dev/null +++ b/independent-projects/arc/tests/src/test/java/io/quarkus/arc/test/interceptors/parameters/ThirdParamInterceptor.java @@ -0,0 +1,24 @@ +package io.quarkus.arc.test.interceptors.parameters; + +import jakarta.annotation.Priority; +import jakarta.interceptor.AroundInvoke; +import jakarta.interceptor.Interceptor; +import jakarta.interceptor.InvocationContext; + +@Simple +@Priority(2) +@Interceptor +public class ThirdParamInterceptor { + + @AroundInvoke + Object interceptParameters(InvocationContext ctx) throws Exception { + Object[] params = ctx.getParameters(); + if (params.length == 1 && params[0] != null) { + if (params[0] instanceof Number) { + params[0] = ((Number) params[0]).intValue() + 1; + } + ctx.setParameters(params); + } + return ctx.proceed(); + } +} From 8f942423aef93865e0b6edf36db3fcb6b94f5862 Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Fri, 3 Feb 2023 09:43:47 +0100 Subject: [PATCH 2/3] ArC intercepted subclasses - skip non-bridge synthetic methods --- .../src/main/java/io/quarkus/arc/processor/Methods.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java index d7357a00c23cb..282701110245b 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java @@ -554,6 +554,9 @@ public boolean test(MethodInfo method) { // Skip bridge methods that have a corresponding "implementation method" on the same class // The algorithm we use to detect these methods is best effort, i.e. there might be use cases where the detection fails return hasImplementation(method); + } else if (method.isSynthetic()) { + // Skip non-bridge synthetic methods + return true; } if (method.hasAnnotation(DotNames.POST_CONSTRUCT) || method.hasAnnotation(DotNames.PRE_DESTROY)) { // @PreDestroy and @PostConstruct methods declared on the bean are NOT candidates for around invoke interception From 0d3d6920abf09a5c4e73952393947fa5fad0fb3b Mon Sep 17 00:00:00 2001 From: Martin Kouba Date: Fri, 3 Feb 2023 11:02:49 +0100 Subject: [PATCH 3/3] ArC intercepted subclasses - skip private methods - that are not observers/producers --- .../io/quarkus/arc/processor/BeanDeployment.java | 15 +++++++++++++++ .../java/io/quarkus/arc/processor/BeanInfo.java | 2 +- .../java/io/quarkus/arc/processor/Methods.java | 11 +++++++++-- .../arc/processor/SubclassSkipPredicateTest.java | 4 +++- 4 files changed, 28 insertions(+), 4 deletions(-) diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java index c2fb9ac8493d8..f3bf13d753cd6 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanDeployment.java @@ -674,6 +674,21 @@ Integer computeAlternativePriority(AnnotationTarget target, List return alternativePriorities != null ? alternativePriorities.compute(target, stereotypes) : null; } + Set getObserverAndProducerMethods() { + Set ret = new HashSet<>(); + for (ObserverInfo observer : observers) { + if (!observer.isSynthetic()) { + ret.add(observer.getObserverMethod()); + } + } + for (BeanInfo bean : beans) { + if (bean.isProducerMethod()) { + ret.add(bean.getTarget().get().asMethod()); + } + } + return ret; + } + private void buildContextPut(String key, Object value) { if (buildContext != null) { buildContext.putInternal(key, value); diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java index b1d40056dff4c..f879fbe285d44 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/BeanInfo.java @@ -626,7 +626,7 @@ private Map initDecoratedMethods() { ClassInfo classInfo = target.get().asClass(); addDecoratedMethods(candidates, classInfo, classInfo, bound, new SubclassSkipPredicate(beanDeployment.getAssignabilityCheck()::isAssignableFrom, - beanDeployment.getBeanArchiveIndex())); + beanDeployment.getBeanArchiveIndex(), beanDeployment.getObserverAndProducerMethods())); Map decoratedMethods = new HashMap<>(candidates.size()); for (Entry entry : candidates.entrySet()) { diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java index 282701110245b..3c8740327d5b3 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/Methods.java @@ -162,7 +162,7 @@ static Set addInterceptedMethodCandidates(BeanDeployment beanDeploym return addInterceptedMethodCandidates(beanDeployment, classInfo, classInfo, candidates, Set.copyOf(classLevelBindings), bytecodeTransformerConsumer, transformUnproxyableClasses, new SubclassSkipPredicate(beanDeployment.getAssignabilityCheck()::isAssignableFrom, - beanDeployment.getBeanArchiveIndex()), + beanDeployment.getBeanArchiveIndex(), beanDeployment.getObserverAndProducerMethods()), false, new HashSet<>()); } @@ -519,14 +519,17 @@ static class SubclassSkipPredicate implements Predicate { private final BiFunction assignableFromFun; private final IndexView beanArchiveIndex; + private final Set producersAndObservers; private ClassInfo clazz; private ClassInfo originalClazz; private List regularMethods; private Set bridgeMethods = new HashSet<>(); - public SubclassSkipPredicate(BiFunction assignableFromFun, IndexView beanArchiveIndex) { + public SubclassSkipPredicate(BiFunction assignableFromFun, IndexView beanArchiveIndex, + Set producersAndObservers) { this.assignableFromFun = assignableFromFun; this.beanArchiveIndex = beanArchiveIndex; + this.producersAndObservers = producersAndObservers; } void startProcessing(ClassInfo clazz, ClassInfo originalClazz) { @@ -558,6 +561,10 @@ public boolean test(MethodInfo method) { // Skip non-bridge synthetic methods return true; } + if (Modifier.isPrivate(method.flags()) && !producersAndObservers.contains(method)) { + // Skip a private method that is not and observer or producer + return true; + } if (method.hasAnnotation(DotNames.POST_CONSTRUCT) || method.hasAnnotation(DotNames.PRE_DESTROY)) { // @PreDestroy and @PostConstruct methods declared on the bean are NOT candidates for around invoke interception return true; diff --git a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/SubclassSkipPredicateTest.java b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/SubclassSkipPredicateTest.java index 463818dd2ef76..683231f1d8b4b 100644 --- a/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/SubclassSkipPredicateTest.java +++ b/independent-projects/arc/processor/src/test/java/io/quarkus/arc/processor/SubclassSkipPredicateTest.java @@ -5,6 +5,7 @@ import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; +import java.util.Collections; import java.util.List; import java.util.stream.Collectors; @@ -24,7 +25,8 @@ public class SubclassSkipPredicateTest { public void testPredicate() throws IOException { IndexView index = Basics.index(Base.class, Submarine.class, Long.class, Number.class); AssignabilityCheck assignabilityCheck = new AssignabilityCheck(index, null); - SubclassSkipPredicate predicate = new SubclassSkipPredicate(assignabilityCheck::isAssignableFrom, null); + SubclassSkipPredicate predicate = new SubclassSkipPredicate(assignabilityCheck::isAssignableFrom, null, + Collections.emptySet()); ClassInfo submarineClass = index.getClassByName(DotName.createSimple(Submarine.class.getName())); predicate.startProcessing(submarineClass, submarineClass);