From b972eb40b724c3223822066f217934865b0b5bb6 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Fri, 16 Oct 2020 13:31:55 +1100 Subject: [PATCH] Request scope performance improvements Previously this required 3 ThreadLocal operations: - isActive() - create(Contextual) - create(Contextual, CreationalContext) This PR provides a single operation so that these can be accomplished with only a single ThreadLocal op --- .../arc/processor/ClientProxyGenerator.java | 34 +++---------- .../java/io/quarkus/arc/ArcContainer.java | 8 +++ .../io/quarkus/arc/InjectableContext.java | 23 +++++++++ .../io/quarkus/arc/impl/ArcContainerImpl.java | 49 ++++++++++++++++--- .../io/quarkus/arc/impl/RequestContext.java | 21 ++++++++ 5 files changed, 101 insertions(+), 34 deletions(-) diff --git a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ClientProxyGenerator.java b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ClientProxyGenerator.java index f38229202c908..8f78d6d4492e6 100644 --- a/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ClientProxyGenerator.java +++ b/independent-projects/arc/processor/src/main/java/io/quarkus/arc/processor/ClientProxyGenerator.java @@ -5,14 +5,13 @@ import static org.objectweb.asm.Opcodes.ACC_PRIVATE; import static org.objectweb.asm.Opcodes.ACC_VOLATILE; +import io.quarkus.arc.ArcContainer; import io.quarkus.arc.ClientProxy; import io.quarkus.arc.InjectableBean; import io.quarkus.arc.InjectableContext; -import io.quarkus.arc.impl.CreationalContextImpl; import io.quarkus.arc.impl.Mockable; import io.quarkus.arc.processor.BeanGenerator.ProviderType; import io.quarkus.arc.processor.ResourceOutput.Resource; -import io.quarkus.gizmo.AssignableResultHandle; import io.quarkus.gizmo.BytecodeCreator; import io.quarkus.gizmo.ClassCreator; import io.quarkus.gizmo.DescriptorUtils; @@ -32,7 +31,6 @@ import java.util.Set; import java.util.function.Consumer; import java.util.function.Predicate; -import javax.enterprise.context.ContextNotActiveException; import javax.enterprise.context.spi.Contextual; import org.jboss.jandex.ClassInfo; import org.jboss.jandex.DotName; @@ -284,33 +282,17 @@ void implementDelegate(ClassCreator clientProxy, ProviderType providerType, Fiel // Application context stored in a field and is always active contextHandle = creator.readInstanceField( FieldDescriptor.of(clientProxy.getClassName(), CONTEXT_FIELD, InjectableContext.class), creator.getThis()); + + creator.returnValue(creator.invokeInterfaceMethod( + MethodDescriptor.ofMethod(InjectableContext.class, "getOrCreate", Object.class, Contextual.class), + contextHandle, beanHandle)); } else { // Arc.container() ResultHandle container = creator.invokeStaticMethod(MethodDescriptors.ARC_CONTAINER); - // bean.getScope() - ResultHandle scope = creator - .invokeInterfaceMethod(MethodDescriptor.ofMethod(InjectableBean.class, "getScope", Class.class), - beanHandle); - // getContext() - contextHandle = creator.invokeInterfaceMethod(MethodDescriptors.ARC_CONTAINER_GET_ACTIVE_CONTEXT, - container, scope); - - BytecodeCreator inactiveBranch = creator.ifNull(contextHandle).trueBranch(); - ResultHandle exception = inactiveBranch.newInstance( - MethodDescriptor.ofConstructor(ContextNotActiveException.class, String.class), - inactiveBranch.invokeVirtualMethod(MethodDescriptors.OBJECT_TO_STRING, scope)); - inactiveBranch.throwException(exception); + creator.returnValue(creator.invokeInterfaceMethod( + MethodDescriptor.ofMethod(ArcContainer.class, "normalScopedInstance", Object.class, InjectableBean.class), + container, beanHandle)); } - - AssignableResultHandle ret = creator.createVariable(Object.class); - creator.assign(ret, creator.invokeInterfaceMethod(MethodDescriptors.CONTEXT_GET_IF_PRESENT, contextHandle, beanHandle)); - BytecodeCreator isNullBranch = creator.ifNull(ret).trueBranch(); - // Create a new contextual instance - new CreationalContextImpl<>() - ResultHandle creationContext = isNullBranch - .newInstance(MethodDescriptor.ofConstructor(CreationalContextImpl.class, Contextual.class), beanHandle); - isNullBranch.assign(ret, - isNullBranch.invokeInterfaceMethod(MethodDescriptors.CONTEXT_GET, contextHandle, beanHandle, creationContext)); - creator.returnValue(ret); } void implementGetContextualInstance(ClassCreator clientProxy, ProviderType providerType) { diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/ArcContainer.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/ArcContainer.java index 4bb1fcebff563..e01373784caae 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/ArcContainer.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/ArcContainer.java @@ -98,6 +98,14 @@ public interface ArcContainer { */ InstanceHandle instance(InjectableBean bean); + /** + * Returns an instance of a normal scoped bean + * + * @param bean + * @return a new bean instance + */ + T normalScopedInstance(InjectableBean bean); + /** * Instances of dependent scoped beans obtained with the returned injectable instance must be explicitly destroyed, either * via the {@link Instance#destroy(Object)} method invoked upon the same injectable instance or with diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/InjectableContext.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/InjectableContext.java index bab63d5ef0c1f..e675156d73495 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/InjectableContext.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/InjectableContext.java @@ -1,8 +1,10 @@ package io.quarkus.arc; +import io.quarkus.arc.impl.CreationalContextImpl; import java.util.Map; import javax.enterprise.context.NormalScope; import javax.enterprise.context.spi.AlterableContext; +import javax.enterprise.context.spi.Contextual; /** * A context implementing this interface allows to capture and view its state via {@link ContextState}. @@ -22,6 +24,27 @@ public interface InjectableContext extends AlterableContext { */ ContextState getState(); + /** + * Attempts to get or create a new isntance of the given contextual. If the scope is not active this returns null. + * + * This allows for the isActive check and the actual creation to happen in a single method, which gives a performance + * benefit by performing fewer thread local operations. + * + * @param contextual The bean + * @param The type of bean + * @return + */ + default T getOrCreate(Contextual contextual) { + if (!isActive()) { + return null; + } + T result = get(contextual); + if (result != null) { + return result; + } + return get(contextual, new CreationalContextImpl<>(contextual)); + } + /** * Destroy all contextual instances from the given state. *

diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java index ada43c262b847..d4002d557023a 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/ArcContainerImpl.java @@ -39,6 +39,7 @@ import java.util.stream.Collectors; import javax.enterprise.context.ApplicationScoped; import javax.enterprise.context.BeforeDestroyed; +import javax.enterprise.context.ContextNotActiveException; import javax.enterprise.context.Dependent; import javax.enterprise.context.Destroyed; import javax.enterprise.context.Initialized; @@ -181,18 +182,17 @@ public InjectableContext getActiveContext(Class scopeType) } else if (Singleton.class.equals(scopeType)) { return singletonContext; } - List active = new ArrayList<>(); + InjectableContext selected = null; for (InjectableContext context : contexts) { if (scopeType.equals(context.getScope()) && context.isActive()) { - active.add(context); + if (selected != null) { + throw new IllegalArgumentException( + "More than one context object for the given scope: " + selected + " " + context); + } + selected = context; } } - if (active.isEmpty()) { - return null; - } else if (active.size() == 1) { - return active.get(0); - } - throw new IllegalArgumentException("More than one context object for the given scope: " + active); + return selected; } @Override @@ -263,6 +263,39 @@ public InstanceHandle instance(InjectableBean bean) { return (InstanceHandle) beanInstanceHandle(bean, null); } + @Override + public T normalScopedInstance(InjectableBean bean) { + requireRunning(); + Class scopeType = bean.getScope(); + // Application/Singleton context is always active + if (ApplicationScoped.class.equals(scopeType)) { + return applicationContext.getOrCreate(bean); + } else if (Singleton.class.equals(scopeType)) { + return applicationContext.getOrCreate(bean); + } + T result = null; + InjectableContext selectedContext = null; + for (InjectableContext context : contexts) { + if (scopeType.equals(context.getScope())) { + if (result != null) { + if (context.isActive()) { + throw new IllegalArgumentException( + "More than one context object for the given scope: " + selectedContext + " " + context); + } + } else { + result = context.getOrCreate(bean); + if (result != null) { + selectedContext = context; + } + } + } + } + if (result == null) { + throw new ContextNotActiveException(); + } + return result; + } + @Override public InjectableInstance select(Class type, Annotation... qualifiers) { return instance.select(type, qualifiers); diff --git a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/RequestContext.java b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/RequestContext.java index ee38bdc9ef476..9985c37e945e6 100644 --- a/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/RequestContext.java +++ b/independent-projects/arc/runtime/src/main/java/io/quarkus/arc/impl/RequestContext.java @@ -49,6 +49,27 @@ public Class getScope() { return RequestScoped.class; } + @Override + public T getOrCreate(Contextual contextual) { + if (contextual == null) { + throw new IllegalArgumentException("Contextual parameter must not be null"); + } + Map, ContextInstanceHandle> ctx = currentContext.get(); + if (ctx == null) { + // Thread local not set - context is not active! + return null; + } + ContextInstanceHandle instance = (ContextInstanceHandle) ctx.get(contextual); + if (instance == null) { + CreationalContext creationalContext = new CreationalContextImpl<>(contextual); + // Bean instance does not exist - create one if we have CreationalContext + instance = new ContextInstanceHandleImpl((InjectableBean) contextual, + contextual.create(creationalContext), creationalContext); + ctx.put(contextual, instance); + } + return instance.get(); + } + @SuppressWarnings("unchecked") @Override public T get(Contextual contextual, CreationalContext creationalContext) {