Skip to content

Commit

Permalink
ArC - load removed beans lazily
Browse files Browse the repository at this point in the history
- only if Instance#get(), InjectableInstance#getHandle() and ArcContainer.instance() is called
- added InjectableInstance.orElse() and InjectableInstance.orNull()
- resolves quarkusio#18345
  • Loading branch information
mkouba committed Sep 21, 2022
1 parent 5a1eef0 commit d59ae7b
Show file tree
Hide file tree
Showing 11 changed files with 175 additions and 73 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ AdditionalBeanBuildItem quarkusApplication(CombinedIndexBuildItem combinedIndex)
List<String> quarkusApplications = new ArrayList<>();
for (ClassInfo quarkusApplication : combinedIndex.getIndex()
.getAllKnownImplementors(DotName.createSimple(QuarkusApplication.class.getName()))) {
if (quarkusApplication.classAnnotation(DotNames.DECORATOR) == null) {
if (quarkusApplication.declaredAnnotation(DotNames.DECORATOR) == null) {
quarkusApplications.add(quarkusApplication.name().toString());
}
}
Expand Down Expand Up @@ -470,7 +470,7 @@ public ObserverRegistrationPhaseBuildItem registerSyntheticObservers(BeanRegistr
} else {
declaringClass = injectionPoint.getTarget().asMethod().declaringClass();
}
if (declaringClass.classAnnotation(DotNames.KOTLIN_METADATA_ANNOTATION) != null) {
if (declaringClass.declaredAnnotation(DotNames.KOTLIN_METADATA_ANNOTATION) != null) {
validationErrors.produce(new ValidationErrorBuildItem(
new DefinitionException(
"kotlin.collections.List cannot be used together with the @All qualifier, please use MutableList or java.util.List instead: "
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
package io.quarkus.arc.test.unused;

import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.util.logging.Formatter;
import java.util.logging.LogRecord;

import javax.enterprise.context.ApplicationScoped;
import javax.enterprise.inject.UnsatisfiedResolutionException;
import javax.enterprise.inject.spi.CDI;

import org.jboss.logmanager.formatters.PatternFormatter;
Expand All @@ -31,7 +32,7 @@ public class ArcLookupProblemDetectedTest {
Formatter fmt = new PatternFormatter("%m");
String message = fmt.format(warning);
assertTrue(message.contains(
"Stack frame: io.quarkus.arc.test.unused.ArcLookupProblemDetectedTest.testWarning"),
"Stack frame: io.quarkus.arc.test.unused.ArcLookupProblemDetectedTest"),
message);
assertTrue(message.contains(
"Required type: class io.quarkus.arc.test.unused.ArcLookupProblemDetectedTest$Alpha"),
Expand All @@ -41,7 +42,7 @@ public class ArcLookupProblemDetectedTest {
@Test
public void testWarning() {
// Note that the warning is only displayed once, subsequent calls use a cached result
assertFalse(CDI.current().select(Alpha.class).isResolvable());
assertThrows(UnsatisfiedResolutionException.class, () -> CDI.current().select(Alpha.class).get());
}

// unused bean, will be removed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
import java.lang.annotation.Annotation;
import java.lang.reflect.InvocationTargetException;
import java.util.Arrays;
import java.util.function.Supplier;

import org.jboss.logging.Logger;

import io.quarkus.arc.ArcContainer;
import io.quarkus.arc.InjectableInstance;
import io.quarkus.arc.InstanceHandle;
import io.quarkus.arc.ManagedContext;

Expand All @@ -23,8 +23,8 @@ public BeanContainerImpl(ArcContainer container) {

@Override
public <T> Factory<T> instanceFactory(Class<T> type, Annotation... qualifiers) {
Supplier<InstanceHandle<T>> handleSupplier = container.instanceSupplier(type, qualifiers);
if (handleSupplier == null) {
InjectableInstance<T> instance = container.select(type, qualifiers);
if (!instance.isResolvable()) {
LOGGER.debugf(
"No matching bean found for type %s and qualifiers %s. The bean might have been marked as unused and removed during build.",
type, Arrays.toString(qualifiers));
Expand All @@ -33,7 +33,7 @@ public <T> Factory<T> instanceFactory(Class<T> type, Annotation... qualifiers) {
return new Factory<T>() {
@Override
public Instance<T> create() {
InstanceHandle<T> handle = handleSupplier.get();
InstanceHandle<T> handle = instance.getHandle();
return new Instance<T>() {
@Override
public T get() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,8 @@ public void accept(Closeable closeable) {
shutdownContext.addShutdownTask(new ShutdownContext.CloseRunnable(closeable));
}
};
CurrentIdentityAssociation currentIdentityAssociation = Arc.container().instance(CurrentIdentityAssociation.class)
.get();
CurrentIdentityAssociation currentIdentityAssociation = Arc.container().select(CurrentIdentityAssociation.class)
.orNull();
ClassLoader tccl = Thread.currentThread().getContextClassLoader();
if (contextFactory == null) {
contextFactory = new RequestContextFactory() {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,18 @@
package io.quarkus.undertow.deployment;

import java.util.List;

import org.jboss.jandex.AnnotationTarget.Kind;
import org.jboss.jandex.AnnotationValue;
import org.jboss.jandex.ClassInfo;
import org.jboss.jandex.DotName;
import org.jboss.jandex.Type;

import io.quarkus.arc.deployment.AnnotationsTransformerBuildItem;
import io.quarkus.arc.deployment.BeanDefiningAnnotationBuildItem;
import io.quarkus.arc.processor.Annotations;
import io.quarkus.arc.processor.AnnotationsTransformer;
import io.quarkus.arc.processor.DotNames;
import io.quarkus.deployment.annotations.BuildProducer;
import io.quarkus.deployment.annotations.BuildStep;

Expand All @@ -12,4 +24,34 @@ void beanDefiningAnnotations(BuildProducer<BeanDefiningAnnotationBuildItem> anno
annotations.produce(new BeanDefiningAnnotationBuildItem(UndertowBuildStep.WEB_SERVLET));
annotations.produce(new BeanDefiningAnnotationBuildItem(UndertowBuildStep.WEB_LISTENER));
}

@BuildStep
AnnotationsTransformerBuildItem addTypedAnnotationToComponents() {
// We need to add @Typed to servlets, filters and listeners because BeanContainer#instanceFactory()
// does not rely on non-standard behavior of ArcContainer#instanceSupplier() anymore
List<DotName> annotations = List.of(UndertowBuildStep.WEB_FILTER, UndertowBuildStep.WEB_SERVLET,
UndertowBuildStep.WEB_LISTENER);
return new AnnotationsTransformerBuildItem(new AnnotationsTransformer() {

@Override
public boolean appliesTo(Kind kind) {
return kind == Kind.CLASS;
}

@Override
public void transform(TransformationContext context) {
if (Annotations.containsAny(context.getAnnotations(), annotations)) {
// E.g. @Typed(TestServlet.class) @WebServlet(urlPatterns = "/test") class TestServlet
ClassInfo clazz = context.getTarget().asClass();
context.transform().add(DotNames.TYPED,
AnnotationValue.createArrayValue("value",
new AnnotationValue[] { AnnotationValue.createClassValue("value",
Type.create(clazz.name(), org.jboss.jandex.Type.Kind.CLASS)) }))
.done();
}
}

});
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,7 @@ public void visit(int version, int access, String name, String signature,
mv.visitInsn(Opcodes.RETURN);
mv.visitMaxs(1, 1);
mv.visitEnd();
LOGGER.debugf("Added a no-args constructor to bean class: ", className);
LOGGER.debugf("Added a no-args constructor to bean class: %s", className);
}
};
}
Expand All @@ -935,7 +935,7 @@ public MethodVisitor visitMethod(int access, String name, String descriptor, Str
if (name.equals(Methods.INIT)) {
access = access & (~Opcodes.ACC_PRIVATE);
LOGGER.debugf(
"Changed visibility of a private no-args constructor to package-private: ",
"Changed visibility of a private no-args constructor to package-private: %s",
className);
}
return super.visitMethod(access, name, descriptor, signature, exceptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import static java.util.stream.Collectors.toList;
import static org.objectweb.asm.Opcodes.ACC_PRIVATE;
import static org.objectweb.asm.Opcodes.ACC_PUBLIC;
import static org.objectweb.asm.Opcodes.ACC_STATIC;

import io.quarkus.arc.Arc;
import io.quarkus.arc.Components;
Expand All @@ -15,6 +16,7 @@
import io.quarkus.gizmo.ClassCreator;
import io.quarkus.gizmo.ClassOutput;
import io.quarkus.gizmo.FieldDescriptor;
import io.quarkus.gizmo.FunctionCreator;
import io.quarkus.gizmo.MethodCreator;
import io.quarkus.gizmo.MethodDescriptor;
import io.quarkus.gizmo.ResultHandle;
Expand Down Expand Up @@ -122,12 +124,18 @@ Collection<Resource> generate(String name, BeanDeployment beanDeployment, Map<Be
beanIdToBeanHandle);

// Break removed beans processing into multiple addRemovedBeans() methods
ResultHandle removedBeansHandle = getComponents.newInstance(MethodDescriptor.ofConstructor(ArrayList.class));
ResultHandle typeCacheHandle = getComponents.newInstance(MethodDescriptor.ofConstructor(HashMap.class));
FunctionCreator removedBeansSupplier = getComponents.createFunction(Supplier.class);
BytecodeCreator removedBeansSupplierBytecode = removedBeansSupplier.getBytecode();
ResultHandle removedBeansList = removedBeansSupplierBytecode
.newInstance(MethodDescriptor.ofConstructor(ArrayList.class));
ResultHandle typeCacheHandle = removedBeansSupplierBytecode.newInstance(MethodDescriptor.ofConstructor(HashMap.class));
if (detectUnusedFalsePositives) {
processRemovedBeans(componentsProvider, getComponents, removedBeansHandle, typeCacheHandle, beanDeployment,
// Generate static addRemovedBeans() methods
processRemovedBeans(componentsProvider, removedBeansSupplierBytecode, removedBeansList, typeCacheHandle,
beanDeployment,
classOutput);
}
removedBeansSupplierBytecode.returnValue(removedBeansList);

// All qualifiers
ResultHandle qualifiers = getComponents.newInstance(MethodDescriptor.ofConstructor(HashSet.class));
Expand All @@ -150,8 +158,8 @@ Collection<Resource> generate(String name, BeanDeployment beanDeployment, Map<Be

ResultHandle componentsHandle = getComponents.newInstance(
MethodDescriptor.ofConstructor(Components.class, Collection.class, Collection.class, Collection.class,
Map.class, Collection.class, Map.class, Set.class),
beansHandle, observersHandle, contextsHandle, transitiveBindingsHandle, removedBeansHandle,
Map.class, Supplier.class, Map.class, Set.class),
beansHandle, observersHandle, contextsHandle, transitiveBindingsHandle, removedBeansSupplier.getInstance(),
qualifiersNonbindingMembers, qualifiers);
getComponents.returnValue(componentsHandle);

Expand Down Expand Up @@ -266,10 +274,10 @@ private void processObservers(ClassCreator componentsProvider, MethodCreator get
}
}

private void processRemovedBeans(ClassCreator componentsProvider, MethodCreator getComponents,
private void processRemovedBeans(ClassCreator componentsProvider, BytecodeCreator targetMethod,
ResultHandle removedBeansHandle, ResultHandle typeCacheHandle, BeanDeployment beanDeployment,
ClassOutput classOutput) {
try (RemovedBeanAdder removedBeanAdder = new RemovedBeanAdder(componentsProvider, getComponents, removedBeansHandle,
try (RemovedBeanAdder removedBeanAdder = new RemovedBeanAdder(componentsProvider, targetMethod, removedBeansHandle,
typeCacheHandle, classOutput)) {
for (BeanInfo remnovedBean : beanDeployment.getRemovedBeans()) {
removedBeanAdder.addComponent(remnovedBean);
Expand Down Expand Up @@ -388,10 +396,10 @@ MethodCreator newAddMethod() {

@Override
void invokeAddMethod() {
getComponentsMethod.invokeVirtualMethod(
targetMethod.invokeVirtualMethod(
MethodDescriptor.ofMethod(componentsProvider.getClassName(),
addMethod.getMethodDescriptor().getName(), void.class, Map.class, List.class),
getComponentsMethod.getThis(), beanIdToBeanHandle, observersHandle);
targetMethod.getThis(), beanIdToBeanHandle, observersHandle);
}

@Override
Expand Down Expand Up @@ -435,9 +443,9 @@ class RemovedBeanAdder extends ComponentAdder<BeanInfo> {

private final MapTypeCache typeCache;

public RemovedBeanAdder(ClassCreator componentsProvider, MethodCreator getComponentsMethod,
public RemovedBeanAdder(ClassCreator componentsProvider, BytecodeCreator targetMethod,
ResultHandle removedBeansHandle, ResultHandle typeCacheHandle, ClassOutput classOutput) {
super(getComponentsMethod, componentsProvider);
super(targetMethod, componentsProvider);
this.removedBeansHandle = removedBeansHandle;
this.typeCacheHandle = typeCacheHandle;
this.sharedQualifers = new HashMap<>();
Expand All @@ -457,7 +465,7 @@ MethodCreator newAddMethod() {
// private void addRemovedBeans1(List removedBeans, List typeCache)
MethodCreator addMethod = componentsProvider
.getMethodCreator(ADD_REMOVED_BEANS + group++, void.class, List.class, Map.class)
.setModifiers(ACC_PRIVATE);
.setModifiers(ACC_STATIC);
// Get the TCCL - we will use it later
ResultHandle currentThread = addMethod
.invokeStaticMethod(MethodDescriptors.THREAD_CURRENT_THREAD);
Expand All @@ -470,10 +478,11 @@ MethodCreator newAddMethod() {

@Override
void invokeAddMethod() {
getComponentsMethod.invokeVirtualMethod(
// Static methods are invoked from within the generated supplier
targetMethod.invokeStaticMethod(
MethodDescriptor.ofMethod(componentsProvider.getClassName(),
addMethod.getMethodDescriptor().getName(), void.class, List.class, Map.class),
getComponentsMethod.getThis(), removedBeansHandle, typeCacheHandle);
removedBeansHandle, typeCacheHandle);
}

@Override
Expand Down Expand Up @@ -613,10 +622,10 @@ MethodCreator newAddMethod() {

@Override
void invokeAddMethod() {
getComponentsMethod.invokeVirtualMethod(
targetMethod.invokeVirtualMethod(
MethodDescriptor.ofMethod(componentsProvider.getClassName(),
addMethod.getMethodDescriptor().getName(), void.class, Map.class),
getComponentsMethod.getThis(), beanIdToBeanHandle);
targetMethod.getThis(), beanIdToBeanHandle);
}

@Override
Expand Down Expand Up @@ -708,12 +717,12 @@ static abstract class ComponentAdder<T extends InjectionTargetInfo> implements A
protected int group;
private int componentsAdded;
protected MethodCreator addMethod;
protected final MethodCreator getComponentsMethod;
protected final BytecodeCreator targetMethod;
protected final ClassCreator componentsProvider;

public ComponentAdder(MethodCreator getComponentsMethod, ClassCreator componentsProvider) {
public ComponentAdder(BytecodeCreator getComponentsMethod, ClassCreator componentsProvider) {
this.group = 1;
this.getComponentsMethod = getComponentsMethod;
this.targetMethod = getComponentsMethod;
this.componentsProvider = componentsProvider;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@
import java.util.Collection;
import java.util.Map;
import java.util.Set;
import java.util.function.Supplier;

public final class Components {

private final Collection<InjectableBean<?>> beans;
private final Collection<RemovedBean> removedBeans;
private final Supplier<Collection<RemovedBean>> removedBeans;
private final Collection<InjectableObserverMethod<?>> observers;
private final Collection<InjectableContext> contexts;
private final Map<Class<? extends Annotation>, Set<Annotation>> transitiveInterceptorBindings;
Expand All @@ -18,7 +19,8 @@ public final class Components {
public Components(Collection<InjectableBean<?>> beans, Collection<InjectableObserverMethod<?>> observers,
Collection<InjectableContext> contexts,
Map<Class<? extends Annotation>, Set<Annotation>> transitiveInterceptorBindings,
Collection<RemovedBean> removedBeans, Map<String, Set<String>> qualifierNonbindingMembers, Set<String> qualifiers) {
Supplier<Collection<RemovedBean>> removedBeans, Map<String, Set<String>> qualifierNonbindingMembers,
Set<String> qualifiers) {
this.beans = beans;
this.observers = observers;
this.contexts = contexts;
Expand All @@ -44,7 +46,7 @@ public Map<Class<? extends Annotation>, Set<Annotation>> getTransitiveIntercepto
return transitiveInterceptorBindings;
}

public Collection<RemovedBean> getRemovedBeans() {
public Supplier<Collection<RemovedBean>> getRemovedBeans() {
return removedBeans;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,26 @@ public interface InjectableInstance<T> extends Instance<T> {
@Override
Iterator<T> iterator();

/**
* If there is exactly one bean that matches the required type and qualifiers, returns the instance, otherwise returns
* {@code other}.
*
* @param other
* @return the bean instance or the other value
*/
default T orElse(T other) {
return isResolvable() ? get() : other;
}

/**
* If there is exactly one bean that matches the required type and qualifiers, returns the instance, otherwise returns
* {@code null}.
*
* @param other
* @return the bean instance or {@code null}
*/
default T orNull() {
return orElse(null);
}

}
Loading

0 comments on commit d59ae7b

Please sign in to comment.