Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request scope performance improvements #12752

Merged
merged 2 commits into from
Nov 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
@RequestScoped
public class CurrentVertxRequest {

public RoutingContext current;
private RoutingContext current;

@Produces
@RequestScoped
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@
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;
Expand All @@ -32,8 +30,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;
import org.jboss.jandex.FieldInfo;
Expand Down Expand Up @@ -278,39 +274,18 @@ void implementDelegate(ClassCreator clientProxy, ProviderType providerType, Fiel
}

ResultHandle beanHandle = creator.readInstanceField(beanField, creator.getThis());
ResultHandle contextHandle;

if (BuiltinScope.APPLICATION.is(bean.getScope())) {
// Application context stored in a field and is always active
contextHandle = creator.readInstanceField(
FieldDescriptor.of(clientProxy.getClassName(), CONTEXT_FIELD, InjectableContext.class), creator.getThis());
// Application context is stored in a field and is always active
creator.returnValue(creator.invokeStaticMethod(MethodDescriptors.CLIENT_PROXIES_GET_APP_SCOPED_DELEGATE,
creator.readInstanceField(
FieldDescriptor.of(clientProxy.getClassName(), CONTEXT_FIELD, InjectableContext.class),
creator.getThis()),
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.invokeStaticMethod(MethodDescriptors.CLIENT_PROXIES_GET_DELEGATE,
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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import io.quarkus.arc.InjectableContext;
import io.quarkus.arc.InjectableInterceptor;
import io.quarkus.arc.InjectableReferenceProvider;
import io.quarkus.arc.impl.ClientProxies;
import io.quarkus.arc.impl.CreationalContextImpl;
import io.quarkus.arc.impl.FixedValueSupplier;
import io.quarkus.arc.impl.InjectableReferenceProviders;
Expand Down Expand Up @@ -242,6 +243,12 @@ public final class MethodDescriptors {
public static final MethodDescriptor REMOVED_BEAN_IMPL = MethodDescriptor.ofConstructor(RemovedBeanImpl.class, Kind.class,
String.class, Set.class, Set.class);

public static final MethodDescriptor CLIENT_PROXIES_GET_APP_SCOPED_DELEGATE = MethodDescriptor.ofMethod(ClientProxies.class,
"getApplicationScopedDelegate", Object.class, InjectableContext.class, InjectableBean.class);

public static final MethodDescriptor CLIENT_PROXIES_GET_DELEGATE = MethodDescriptor.ofMethod(ClientProxies.class,
"getDelegate", Object.class, InjectableBean.class);

private MethodDescriptors() {
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import java.lang.annotation.Annotation;
import java.lang.reflect.Type;
import java.util.Collection;
import java.util.Set;
import java.util.concurrent.ExecutorService;
import java.util.function.Supplier;
Expand All @@ -28,6 +29,13 @@ public interface ArcContainer {
*/
InjectableContext getActiveContext(Class<? extends Annotation> scopeType);

/**
*
* @param scopeType
* @return the matching context objects, never null
*/
Collection<InjectableContext> getContexts(Class<? extends Annotation> scopeType);

/**
*
* @return the set of all supported scopes
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
package io.quarkus.arc;

import java.util.Map;
import java.util.function.Function;
import javax.enterprise.context.NormalScope;
import javax.enterprise.context.spi.AlterableContext;
import javax.enterprise.context.spi.Contextual;
import javax.enterprise.context.spi.CreationalContext;

/**
* A context implementing this interface allows to capture and view its state via {@link ContextState}.
* It also allows user to destroy all contextual instances within this context.
*
* @author Martin Kouba
* A context implementing this interface makes it possible to capture and view its state via the {@link ContextState}.
*
* It also allows users to destroy all contextual instances within this context.
*/
public interface InjectableContext extends AlterableContext {

Expand All @@ -22,6 +24,30 @@ public interface InjectableContext extends AlterableContext {
*/
ContextState getState();

/**
* If the context is active then return an existing instance of certain contextual type or create a new instance, otherwise
* return a null value.
*
* This allows for the {@link #isActive()} check and the actual creation to happen in a single method, which gives a
* performance benefit by performing fewer thread local operations.
*
* @param <T> the type of contextual type
* @param contextual the contextual type
* @param creationalContextFunction the creational context function
* @return the contextual instance, or a null value
*/
default <T> T getIfActive(Contextual<T> contextual,
Function<Contextual<T>, CreationalContext<T>> creationalContextFunction) {
if (!isActive()) {
return null;
}
T result = get(contextual);
if (result != null) {
return result;
}
return get(contextual, creationalContextFunction.apply(contextual));
}

/**
* Destroy all contextual instances from the given state.
* <p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.lang.reflect.TypeVariable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -78,8 +79,7 @@ public class ArcContainerImpl implements ArcContainer {
private final List<InjectableObserverMethod<?>> observers;
private final Map<Class<? extends Annotation>, Set<Annotation>> transitiveInterceptorBindings;

// List of "ambiguous" contexts that could share a scope
private final List<InjectableContext> contexts;
private final Map<Class<? extends Annotation>, Collection<InjectableContext>> contexts;
private final ManagedContext requestContext;
private final InjectableContext applicationContext;
private final InjectableContext singletonContext;
Expand All @@ -106,8 +106,10 @@ public ArcContainerImpl() {
applicationContext = new ApplicationContext();
singletonContext = new SingletonContext();
requestContext = new RequestContext();
contexts = new ArrayList<>();
contexts.add(requestContext);
contexts = new HashMap<>();
putContext(requestContext);
putContext(applicationContext);
putContext(singletonContext);

for (ComponentsProvider componentsProvider : ServiceLoader.load(ComponentsProvider.class)) {
Components components = componentsProvider.getComponents();
Expand All @@ -130,7 +132,7 @@ public ArcContainerImpl() {
throw new IllegalStateException(
"Failed to register a context - built-in singleton context is always active: " + context);
}
contexts.add(context);
putContext(context);
}
for (Entry<Class<? extends Annotation>, Set<Annotation>> entry : components.getTransitiveInterceptorBindings()
.entrySet()) {
Expand All @@ -153,6 +155,17 @@ public ArcContainerImpl() {
instance = InstanceImpl.of(Object.class, Collections.emptySet());
}

private void putContext(InjectableContext context) {
Collection<InjectableContext> values = contexts.get(context.getScope());
if (values == null) {
contexts.put(context.getScope(), Collections.singleton(context));
} else {
List<InjectableContext> multi = new LinkedList<>(values);
multi.add(context);
contexts.put(context.getScope(), Collections.unmodifiableList(multi));
}
}

private void addBuiltInBeans() {
// BeanManager, Event<?>, Instance<?>
beans.add(new BeanManagerBean());
Expand Down Expand Up @@ -181,27 +194,31 @@ public InjectableContext getActiveContext(Class<? extends Annotation> scopeType)
} else if (Singleton.class.equals(scopeType)) {
return singletonContext;
}
List<InjectableContext> active = new ArrayList<>();
for (InjectableContext context : contexts) {
if (scopeType.equals(context.getScope()) && context.isActive()) {
active.add(context);
Collection<InjectableContext> contextsForScope = contexts.get(scopeType);
InjectableContext selected = null;
if (contextsForScope != null) {
for (InjectableContext context : contextsForScope) {
if (context.isActive()) {
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
public Collection<InjectableContext> getContexts(Class<? extends Annotation> scopeType) {
requireRunning();
return contexts.getOrDefault(scopeType, Collections.emptyList());
}

@Override
public Set<Class<? extends Annotation>> getScopes() {
Set<Class<? extends Annotation>> scopes = contexts.stream().map(InjectableContext::getScope)
.collect(Collectors.toSet());
scopes.add(ApplicationScoped.class);
scopes.add(Singleton.class);
return scopes;
return contexts.keySet().stream().collect(Collectors.toSet());
}

@Override
Expand Down Expand Up @@ -464,16 +481,24 @@ boolean isScope(Class<? extends Annotation> annotationType) {
if (annotationType.isAnnotationPresent(Scope.class) || annotationType.isAnnotationPresent(NormalScope.class)) {
return true;
}
return contexts.stream().map(InjectableContext::getScope).filter(annotationType::equals).findAny().isPresent();
for (Class<? extends Annotation> scopeType : contexts.keySet()) {
if (scopeType.equals(annotationType)) {
return true;
}
}
return false;
}

boolean isNormalScope(Class<? extends Annotation> annotationType) {
if (annotationType.isAnnotationPresent(NormalScope.class)) {
return true;
}
for (InjectableContext context : contexts) {
if (context.getScope().equals(annotationType) && context.isNormal()) {
return true;
Collection<InjectableContext> injectableContexts = contexts.get(annotationType);
if (injectableContexts != null) {
for (InjectableContext context : injectableContexts) {
if (context.isNormal()) {
return true;
}
}
}
return false;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package io.quarkus.arc.impl;

import io.quarkus.arc.Arc;
import io.quarkus.arc.InjectableBean;
import io.quarkus.arc.InjectableContext;
import javax.enterprise.context.ContextNotActiveException;
import javax.enterprise.context.spi.Contextual;

public final class ClientProxies {

private ClientProxies() {
}

public static <T> T getApplicationScopedDelegate(InjectableContext applicationContext, InjectableBean<T> bean) {
T result = applicationContext.get(bean);
if (result == null) {
result = applicationContext.get(bean, newCreationalContext(bean));
}
return result;
}

public static <T> T getDelegate(InjectableBean<T> bean) {
Iterable<InjectableContext> contexts = Arc.container().getContexts(bean.getScope());
T result = null;
InjectableContext selectedContext = null;
for (InjectableContext context : contexts) {
if (result != null) {
if (context.isActive()) {
throw new IllegalArgumentException(
"More than one context object for the given scope: " + selectedContext + " " + context);
}
} else {
result = context.getIfActive(bean, ClientProxies::newCreationalContext);
if (result != null) {
selectedContext = context;
}
}
}
if (result == null) {
throw new ContextNotActiveException();
}
return result;
}

private static <T> CreationalContextImpl<T> newCreationalContext(Contextual<T> contextual) {
return new CreationalContextImpl<>(contextual);
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.function.Function;
import javax.enterprise.context.spi.Contextual;
import javax.enterprise.context.spi.CreationalContext;

Expand All @@ -16,7 +17,7 @@
*
* @param <T>
*/
public class CreationalContextImpl<T> implements CreationalContext<T> {
public class CreationalContextImpl<T> implements CreationalContext<T>, Function<Contextual<T>, CreationalContext<T>> {

private final Contextual<T> contextual;
private final CreationalContextImpl<?> parent;
Expand Down Expand Up @@ -86,6 +87,11 @@ public <C> CreationalContextImpl<C> child(Contextual<C> contextual) {
return new CreationalContextImpl<>(contextual, this);
}

@Override
public CreationalContext<T> apply(Contextual<T> contextual) {
return this;
}

public static <T> CreationalContextImpl<T> unwrap(CreationalContext<T> ctx) {
if (ctx instanceof CreationalContextImpl) {
return (CreationalContextImpl<T>) ctx;
Expand Down
Loading