Improve generated bytecode from recorder

Mostly targeted at improving RESTEasy Reactive hot reload time and memory usage. Decompiled bytecode for a 100 endpoint app goes from 28k lines to 8k lines, and hot reload speed is significantly improved. Runtime memory usage is also improved, as some objects can now be shared. - Primitive improvements and simplification of buggy int/boolean handling - Cast once when the object is returned from the value array - Allow for value based recording to 'intern' objects as part of the recording process. - Allow for constructor recording for objects that can't be annotated.
quarkusio · Sep 29, 2021 · 6f9191f · 6f9191f
1 parent c3321e2
commit 6f9191f
Show file tree

Hide file tree

Showing 21 changed files with 314 additions and 131 deletions.
diff --git a/core/deployment/src/main/java/io/quarkus/deployment/ExtensionLoader.java b/core/deployment/src/main/java/io/quarkus/deployment/ExtensionLoader.java
@@ -491,6 +491,7 @@ private static Consumer<BuildChainBuilder> loadStepsFromClass(Class<?> clazz,
             final Parameter[] methodParameters = method.getParameters();
             final Record recordAnnotation = method.getAnnotation(Record.class);
             final boolean isRecorder = recordAnnotation != null;
+            final boolean identityComparison = isRecorder ? recordAnnotation.useIdentityComparisonForParameters() : true;
             if (isRecorder) {
                 boolean recorderFound = false;
                 for (Class<?> p : method.getParameterTypes()) {
@@ -810,7 +811,7 @@ public void execute(final BuildContext bc) {
                                 BytecodeRecorderImpl bri = isRecorder
                                         ? new BytecodeRecorderImpl(recordAnnotation.value() == ExecutionTime.STATIC_INIT,
                                                 clazz.getSimpleName(), method.getName(),
-                                                Integer.toString(method.toString().hashCode()))
+                                                Integer.toString(method.toString().hashCode()), identityComparison)
                                         : null;
                                 for (int i = 0; i < methodArgs.length; i++) {
                                     methodArgs[i] = methodParamFns.get(i).apply(bc, bri);

diff --git a/core/deployment/src/main/java/io/quarkus/deployment/annotations/Record.java b/core/deployment/src/main/java/io/quarkus/deployment/annotations/Record.java
@@ -53,4 +53,15 @@
      */
     boolean optional() default false;
 
+    /**
+     * If this is set to false then parameters are considered equal based on equals/hashCode, instead of identity.
+     *
+     * This is an advanced option, it is only useful if you are recording lots of objects that you expect to be the same
+     * but have different identities. This allows multiple objects at deployment time to be interned into a single
+     * object at runtime.
+     *
+     * This is an advanced option, most recorders don't want this.
+     */
+    boolean useIdentityComparisonForParameters() default true;
+
 }
diff --git a/...loyment/src/main/java/io/quarkus/deployment/builditem/RecordableConstructorBuildItem.java b/...loyment/src/main/java/io/quarkus/deployment/builditem/RecordableConstructorBuildItem.java
@@ -0,0 +1,22 @@
+package io.quarkus.deployment.builditem;
+
+import io.quarkus.builder.item.MultiBuildItem;
+
+/**
+ * Indicates that the given class should be instantiated with the constructor with the most parameters
+ * when the object is bytecode recorded.
+ *
+ * An alternative to {@link RecordableConstructorBuildItem} for when the objects cannot be annotated
+ */
+public final class RecordableConstructorBuildItem extends MultiBuildItem {
+
+    private final Class<?> clazz;
+
+    public RecordableConstructorBuildItem(Class<?> clazz) {
+        this.clazz = clazz;
+    }
+
+    public Class<?> getClazz() {
+        return clazz;
+    }
+}
diff --git a/core/deployment/src/main/java/io/quarkus/deployment/recording/BytecodeRecorderImpl.java b/core/deployment/src/main/java/io/quarkus/deployment/recording/BytecodeRecorderImpl.java
diff --git a/core/deployment/src/main/java/io/quarkus/deployment/steps/MainClassBuildStep.java b/core/deployment/src/main/java/io/quarkus/deployment/steps/MainClassBuildStep.java
@@ -47,6 +47,7 @@
 import io.quarkus.deployment.builditem.MainClassBuildItem;
 import io.quarkus.deployment.builditem.ObjectSubstitutionBuildItem;
 import io.quarkus.deployment.builditem.QuarkusApplicationClassBuildItem;
+import io.quarkus.deployment.builditem.RecordableConstructorBuildItem;
 import io.quarkus.deployment.builditem.StaticBytecodeRecorderBuildItem;
 import io.quarkus.deployment.builditem.SystemPropertyBuildItem;
 import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
@@ -115,6 +116,7 @@ void build(List<StaticBytecodeRecorderBuildItem> staticInitTasks,
             List<FeatureBuildItem> features,
             BuildProducer<ApplicationClassNameBuildItem> appClassNameProducer,
             List<BytecodeRecorderObjectLoaderBuildItem> loaders,
+            List<RecordableConstructorBuildItem> recordableConstructorBuildItems,
             BuildProducer<GeneratedClassBuildItem> generatedClass,
             LaunchModeBuildItem launchMode,
             LiveReloadBuildItem liveReloadBuildItem,
@@ -174,7 +176,8 @@ void build(List<StaticBytecodeRecorderBuildItem> staticInitTasks,
         TryBlock tryBlock = mv.tryBlock();
         tryBlock.invokeStaticMethod(CONFIGURE_STEP_TIME_START);
         for (StaticBytecodeRecorderBuildItem holder : staticInitTasks) {
-            writeRecordedBytecode(holder.getBytecodeRecorder(), null, substitutions, loaders, gizmoOutput, startupContext,
+            writeRecordedBytecode(holder.getBytecodeRecorder(), null, substitutions, recordableConstructorBuildItems, loaders,
+                    gizmoOutput, startupContext,
                     tryBlock);
         }
         tryBlock.returnValue(null);
@@ -269,6 +272,7 @@ void build(List<StaticBytecodeRecorderBuildItem> staticInitTasks,
         tryBlock.invokeStaticMethod(CONFIGURE_STEP_TIME_START);
         for (MainBytecodeRecorderBuildItem holder : mainMethod) {
             writeRecordedBytecode(holder.getBytecodeRecorder(), holder.getGeneratedStartupContextClassName(), substitutions,
+                    recordableConstructorBuildItems,
                     loaders, gizmoOutput, startupContext, tryBlock);
         }
 
@@ -437,6 +441,7 @@ private void generateMainForQuarkusApplication(String quarkusApplicationClassNam
 
     private void writeRecordedBytecode(BytecodeRecorderImpl recorder, String fallbackGeneratedStartupTaskClassName,
             List<ObjectSubstitutionBuildItem> substitutions,
+            List<RecordableConstructorBuildItem> recordableConstructorBuildItems,
             List<BytecodeRecorderObjectLoaderBuildItem> loaders, GeneratedClassGizmoAdaptor gizmoOutput,
             ResultHandle startupContext, BytecodeCreator bytecodeCreator) {
 
@@ -452,6 +457,9 @@ private void writeRecordedBytecode(BytecodeRecorderImpl recorder, String fallbac
             for (BytecodeRecorderObjectLoaderBuildItem item : loaders) {
                 recorder.registerObjectLoader(item.getObjectLoader());
             }
+            for (var item : recordableConstructorBuildItems) {
+                recorder.markClassAsConstructorRecordable(item.getClazz());
+            }
             recorder.writeBytecode(gizmoOutput);
         }
 

diff --git a/core/deployment/src/test/java/io/quarkus/deployment/recording/BytecodeRecorderTestCase.java b/core/deployment/src/test/java/io/quarkus/deployment/recording/BytecodeRecorderTestCase.java
@@ -280,7 +280,7 @@ public void testNewInstance() throws Exception {
             recorder.add(instance);
             recorder.add(instance);
             recorder.result(instance);
-        }, new TestJavaBean(null, 2));
+        }, new TestJavaBean(null, 2, 2));
     }
 
     @Test

diff --git a/core/deployment/src/test/java/io/quarkus/deployment/recording/TestJavaBean.java b/core/deployment/src/test/java/io/quarkus/deployment/recording/TestJavaBean.java
@@ -8,6 +8,12 @@ public class TestJavaBean {
     public TestJavaBean() {
     }
 
+    public TestJavaBean(String sval, int ival, Integer boxedIval) {
+        this.sval = sval;
+        this.ival = ival;
+        this.boxedIval = boxedIval;
+    }
+
     public TestJavaBean(String sval, int ival) {
         this.sval = sval;
         this.ival = ival;
@@ -21,6 +27,7 @@ public TestJavaBean(String sval, int ival, Supplier<String> supplier) {
 
     private String sval;
     private int ival;
+    private Integer boxedIval = 0;
     private Supplier<String> supplier;
 
     public String getSval() {
@@ -40,6 +47,15 @@ public void setIval(int ival) {
         this.ival = ival;
     }
 
+    public Integer getBoxedIval() {
+        return boxedIval;
+    }
+
+    public TestJavaBean setBoxedIval(Integer boxedIval) {
+        this.boxedIval = boxedIval;
+        return this;
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o)
@@ -48,7 +64,7 @@ public boolean equals(Object o) {
             return false;
         TestJavaBean that = (TestJavaBean) o;
         boolean matchesSimple = ival == that.ival &&
-                Objects.equals(sval, that.sval);
+                Objects.equals(sval, that.sval) && Objects.equals(boxedIval, that.boxedIval);
         if (!matchesSimple) {
             return false;
         }
@@ -81,6 +97,7 @@ public String toString() {
         return "TestJavaBean{" +
                 "sval='" + sval + '\'' +
                 ", ival=" + ival +
+                ", boxedIval=" + boxedIval +
                 '}';
     }
 }
diff --git a/core/deployment/src/test/java/io/quarkus/deployment/recording/TestRecorder.java b/core/deployment/src/test/java/io/quarkus/deployment/recording/TestRecorder.java
@@ -69,6 +69,7 @@ public void bean(NonSerializable bean) {
 
     public void add(RuntimeValue<TestJavaBean> bean) {
         bean.getValue().setIval(bean.getValue().getIval() + 1);
+        bean.getValue().setBoxedIval(bean.getValue().getBoxedIval() + 1);
     }
 
     public void bean(TestConstructorBean bean) {

diff --git a/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/unused/UnusedExclusionTest.java b/extensions/arc/deployment/src/test/java/io/quarkus/arc/test/unused/UnusedExclusionTest.java
@@ -53,7 +53,7 @@ public void execute(BuildContext context) {
                         BeanContainer beanContainer = context.consume(BeanContainerBuildItem.class).getValue();
                         BytecodeRecorderImpl bytecodeRecorder = new BytecodeRecorderImpl(true,
                                 TestRecorder.class.getSimpleName(),
-                                "test", "" + TestRecorder.class.hashCode());
+                                "test", "" + TestRecorder.class.hashCode(), true);
                         // We need to use reflection due to some class loading problems
                         Object recorderProxy = bytecodeRecorder.getRecordingProxy(TestRecorder.class);
                         try {

diff --git a/...c/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java b/...c/main/java/io/quarkus/resteasy/reactive/server/deployment/ResteasyReactiveProcessor.java
@@ -62,6 +62,8 @@
 import org.jboss.resteasy.reactive.server.model.Features;
 import org.jboss.resteasy.reactive.server.model.HandlerChainCustomizer;
 import org.jboss.resteasy.reactive.server.model.ParamConverterProviders;
+import org.jboss.resteasy.reactive.server.model.ServerMethodParameter;
+import org.jboss.resteasy.reactive.server.model.ServerResourceMethod;
 import org.jboss.resteasy.reactive.server.processor.scanning.MethodScanner;
 import org.jboss.resteasy.reactive.spi.BeanFactory;
 
@@ -86,6 +88,7 @@
 import io.quarkus.deployment.builditem.FeatureBuildItem;
 import io.quarkus.deployment.builditem.GeneratedClassBuildItem;
 import io.quarkus.deployment.builditem.LaunchModeBuildItem;
+import io.quarkus.deployment.builditem.RecordableConstructorBuildItem;
 import io.quarkus.deployment.builditem.ShutdownContextBuildItem;
 import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
 import io.quarkus.deployment.builditem.nativeimage.ReflectiveHierarchyBuildItem;
@@ -126,6 +129,7 @@
 import io.quarkus.resteasy.reactive.spi.MessageBodyReaderOverrideBuildItem;
 import io.quarkus.resteasy.reactive.spi.MessageBodyWriterBuildItem;
 import io.quarkus.resteasy.reactive.spi.MessageBodyWriterOverrideBuildItem;
+import io.quarkus.runtime.LaunchMode;
 import io.quarkus.runtime.RuntimeValue;
 import io.quarkus.security.AuthenticationCompletionException;
 import io.quarkus.security.AuthenticationFailedException;
@@ -170,6 +174,12 @@ MinNettyAllocatorMaxOrderBuildItem setMinimalNettyMaxOrderSize() {
         return new MinNettyAllocatorMaxOrderBuildItem(3);
     }
 
+    @BuildStep
+    void recordableConstructor(BuildProducer<RecordableConstructorBuildItem> ctors) {
+        ctors.produce(new RecordableConstructorBuildItem(ServerResourceMethod.class));
+        ctors.produce(new RecordableConstructorBuildItem(ServerMethodParameter.class));
+    }
+
     @BuildStep
     void vertxIntegration(BuildProducer<MessageBodyWriterBuildItem> writerBuildItemBuildProducer) {
         writerBuildItemBuildProducer.produce(new MessageBodyWriterBuildItem(ServerVertxBufferMessageBodyWriter.class.getName(),
@@ -258,8 +268,12 @@ public void unremoveableBeans(Optional<ResourceScanningResultBuildItem> resource
 
     @SuppressWarnings("unchecked")
     @BuildStep
-    @Record(ExecutionTime.STATIC_INIT)
-    public void setupEndpoints(Capabilities capabilities, BeanArchiveIndexBuildItem beanArchiveIndexBuildItem,
+    //note useIdentityComparisonForParameters=false
+    //resteasy can generate lots of small collections with similar values as part of its metadata gathering
+    //this allows multiple objects to be compressed into a single object at runtime
+    //saving memory and reducing reload time
+    @Record(value = ExecutionTime.STATIC_INIT, useIdentityComparisonForParameters = false)
+    public void setupEndpoints(BeanArchiveIndexBuildItem beanArchiveIndexBuildItem,
             BeanContainerBuildItem beanContainerBuildItem,
             ResteasyReactiveConfig config,
             Optional<ResourceScanningResultBuildItem> resourceScanningResultBuildItem,
@@ -579,6 +593,7 @@ private boolean hasAnnotation(MethodInfo method, short paramPosition, DotName an
                     .setGlobalHandlerCustomers(
                             new ArrayList<>(Collections.singletonList(new SecurityContextOverrideHandler.Customizer()))) //TODO: should be pluggable
                     .setResourceClasses(resourceClasses)
+                    .setDevelopmentMode(launchModeBuildItem.getLaunchMode() == LaunchMode.DEVELOPMENT)
                     .setLocatableResourceClasses(subResourceClasses)
                     .setParamConverterProviders(paramConverterProviders);
             quarkusRestDeploymentInfoBuildItemBuildProducer

diff --git a/...ommon/runtime/src/main/java/org/jboss/resteasy/reactive/common/model/MethodParameter.java b/...ommon/runtime/src/main/java/org/jboss/resteasy/reactive/common/model/MethodParameter.java
@@ -1,5 +1,7 @@
 package org.jboss.resteasy.reactive.common.model;
 
+import java.util.Objects;
+
 public class MethodParameter {
     public String name;
     public String type;
@@ -117,4 +119,25 @@ public String toString() {
                 ", type='" + type + '\'' +
                 '}';
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o)
+            return true;
+        if (o == null || getClass() != o.getClass())
+            return false;
+        MethodParameter that = (MethodParameter) o;
+        return encoded == that.encoded && single == that.single && optional == that.optional
+                && isObtainedAsCollection == that.isObtainedAsCollection && Objects.equals(name, that.name)
+                && Objects.equals(type, that.type) && Objects.equals(declaredType, that.declaredType)
+                && Objects.equals(declaredUnresolvedType, that.declaredUnresolvedType)
+                && Objects.equals(signature, that.signature) && parameterType == that.parameterType
+                && Objects.equals(defaultValue, that.defaultValue);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(name, type, declaredType, declaredUnresolvedType, signature, parameterType, encoded, single,
+                defaultValue, optional, isObtainedAsCollection);
+    }
 }
diff --git a/...common/runtime/src/main/java/org/jboss/resteasy/reactive/common/model/ResourceMethod.java b/...common/runtime/src/main/java/org/jboss/resteasy/reactive/common/model/ResourceMethod.java
@@ -65,6 +65,31 @@ public class ResourceMethod {
     private boolean isMultipart;
     private List<ResourceMethod> subResourceMethods;
 
+    public ResourceMethod() {
+    }
+
+    public ResourceMethod(String httpMethod, String path, String[] produces, String sseElementType, String[] consumes,
+            Set<String> nameBindingNames, String name, String returnType, String simpleReturnType, MethodParameter[] parameters,
+            boolean blocking, boolean suspended, boolean isSse, boolean isFormParamRequired, boolean isMultipart,
+            List<ResourceMethod> subResourceMethods) {
+        this.httpMethod = httpMethod;
+        this.path = path;
+        this.produces = produces;
+        this.sseElementType = sseElementType;
+        this.consumes = consumes;
+        this.nameBindingNames = nameBindingNames;
+        this.name = name;
+        this.returnType = returnType;
+        this.simpleReturnType = simpleReturnType;
+        this.parameters = parameters;
+        this.blocking = blocking;
+        this.suspended = suspended;
+        this.isSse = isSse;
+        this.isFormParamRequired = isFormParamRequired;
+        this.isMultipart = isMultipart;
+        this.subResourceMethods = subResourceMethods;
+    }
+
     public boolean isResourceLocator() {
         return httpMethod == null;
     }

diff --git a/independent-projects/resteasy-reactive/pom.xml b/independent-projects/resteasy-reactive/pom.xml
@@ -281,6 +281,12 @@
     <build>
         <pluginManagement>
             <plugins>
+                <plugin>
+                    <artifactId>maven-compiler-plugin</artifactId>
+                    <configuration>
+                        <parameters>true</parameters>
+                    </configuration>
+                </plugin>
                 <plugin>
                     <artifactId>maven-javadoc-plugin</artifactId>
                     <configuration>

diff --git a/.../server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/DeploymentInfo.java b/.../server/runtime/src/main/java/org/jboss/resteasy/reactive/server/core/DeploymentInfo.java
@@ -32,6 +32,7 @@ public class DeploymentInfo {
     private Function<Object, Object> clientProxyUnwrapper;
     private String applicationPath;
     private List<HandlerChainCustomizer> globalHandlerCustomers = new ArrayList<>();
+    private boolean developmentMode;
 
     public ResourceInterceptors getInterceptors() {
         return interceptors;
@@ -167,4 +168,13 @@ public DeploymentInfo setGlobalHandlerCustomers(List<HandlerChainCustomizer> glo
         this.globalHandlerCustomers = globalHandlerCustomers;
         return this;
     }
+
+    public boolean isDevelopmentMode() {
+        return developmentMode;
+    }
+
+    public DeploymentInfo setDevelopmentMode(boolean developmentMode) {
+        this.developmentMode = developmentMode;
+        return this;
+    }
 }
diff --git a/...boss/resteasy/reactive/server/core/parameters/converters/GeneratedParameterConverter.java b/...boss/resteasy/reactive/server/core/parameters/converters/GeneratedParameterConverter.java
@@ -1,5 +1,7 @@
 package org.jboss.resteasy.reactive.server.core.parameters.converters;
 
+import java.util.Objects;
+
 public class GeneratedParameterConverter implements ParameterConverterSupplier {
 
     private String className;
@@ -24,4 +26,19 @@ public GeneratedParameterConverter setClassName(String className) {
         this.className = className;
         return this;
     }
+
+    @Override
+    public boolean equals(Object o) {
+        if (this == o)
+            return true;
+        if (o == null || getClass() != o.getClass())
+            return false;
+        GeneratedParameterConverter that = (GeneratedParameterConverter) o;
+        return Objects.equals(className, that.className);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(className);
+    }
 }