From 18f1ca2792940386cc50b0efa70c15c25d307505 Mon Sep 17 00:00:00 2001 From: Stuart Douglas Date: Thu, 18 Jun 2020 16:32:38 +1000 Subject: [PATCH] Simplify runtime class transformation The tranformation now uses the same code as the production app, which removes complexity and the ASM dependency from the bootstrap. --- .../TransformedClassesBuildItem.java | 14 ++- .../steps/ClassTransformingBuildStep.java | 8 +- .../runner/bootstrap/AugmentActionImpl.java | 6 +- .../runner/bootstrap/StartupActionImpl.java | 67 ++++----------- .../VerticleWithClassNameDeploymentTest.java | 2 +- .../bootstrap/app/CuratedApplication.java | 9 +- .../classloading/QuarkusClassLoader.java | 85 ------------------- .../bootstrap/maven-resolver/pom.xml | 4 - independent-projects/bootstrap/pom.xml | 6 -- 9 files changed, 42 insertions(+), 159 deletions(-) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/builditem/TransformedClassesBuildItem.java b/core/deployment/src/main/java/io/quarkus/deployment/builditem/TransformedClassesBuildItem.java index edad767ad912a..3f0aa90194fca 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/builditem/TransformedClassesBuildItem.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/builditem/TransformedClassesBuildItem.java @@ -36,12 +36,16 @@ public Map> getTransformedFilesByJar() { public static class TransformedClass { + private final String className; private final byte[] data; private final String fileName; + private final boolean eager; - public TransformedClass(byte[] data, String fileName) { + public TransformedClass(String className, byte[] data, String fileName, boolean eager) { + this.className = className; this.data = data; this.fileName = fileName; + this.eager = eager; } public byte[] getData() { @@ -52,6 +56,14 @@ public String getFileName() { return fileName; } + public String getClassName() { + return className; + } + + public boolean isEager() { + return eager; + } + @Override public boolean equals(Object o) { if (this == o) diff --git a/core/deployment/src/main/java/io/quarkus/deployment/steps/ClassTransformingBuildStep.java b/core/deployment/src/main/java/io/quarkus/deployment/steps/ClassTransformingBuildStep.java index cdbeb0c057285..b561d6ce92e7b 100644 --- a/core/deployment/src/main/java/io/quarkus/deployment/steps/ClassTransformingBuildStep.java +++ b/core/deployment/src/main/java/io/quarkus/deployment/steps/ClassTransformingBuildStep.java @@ -45,6 +45,7 @@ TransformedClassesBuildItem handleClassTransformation(List noConstScanning = new HashSet<>(); Map> constScanning = new HashMap<>(); + Set eager = new HashSet<>(); for (BytecodeTransformerBuildItem i : bytecodeTransformerBuildItems) { bytecodeTransformers.computeIfAbsent(i.getClassToTransform(), (h) -> new ArrayList<>()) .add(i.getVisitorFunction()); @@ -54,6 +55,9 @@ TransformedClassesBuildItem handleClassTransformation(List new HashSet<>()) .addAll(i.getRequireConstPoolEntry()); } + if (i.isEager()) { + eager.add(i.getClassToTransform()); + } } QuarkusClassLoader cl = (QuarkusClassLoader) Thread.currentThread().getContextClassLoader(); Map transformedToArchive = new ConcurrentHashMap<>(); @@ -100,8 +104,8 @@ public TransformedClassesBuildItem.TransformedClass call() throws Exception { visitor = i.apply(className, visitor); } cr.accept(visitor, 0); - return new TransformedClassesBuildItem.TransformedClass(writer.toByteArray(), - classFileName); + return new TransformedClassesBuildItem.TransformedClass(className, writer.toByteArray(), + classFileName, eager.contains(className)); } finally { Thread.currentThread().setContextClassLoader(old); } diff --git a/core/deployment/src/main/java/io/quarkus/runner/bootstrap/AugmentActionImpl.java b/core/deployment/src/main/java/io/quarkus/runner/bootstrap/AugmentActionImpl.java index db4c96997a999..94abf8a01034c 100644 --- a/core/deployment/src/main/java/io/quarkus/runner/bootstrap/AugmentActionImpl.java +++ b/core/deployment/src/main/java/io/quarkus/runner/bootstrap/AugmentActionImpl.java @@ -32,7 +32,6 @@ import io.quarkus.deployment.ExtensionLoader; import io.quarkus.deployment.QuarkusAugmentor; import io.quarkus.deployment.builditem.ApplicationClassNameBuildItem; -import io.quarkus.deployment.builditem.BytecodeTransformerBuildItem; import io.quarkus.deployment.builditem.ConfigDescriptionBuildItem; import io.quarkus.deployment.builditem.GeneratedClassBuildItem; import io.quarkus.deployment.builditem.GeneratedResourceBuildItem; @@ -41,6 +40,7 @@ import io.quarkus.deployment.builditem.MainClassBuildItem; import io.quarkus.deployment.builditem.RawCommandLineArgumentsBuildItem; import io.quarkus.deployment.builditem.ShutdownContextBuildItem; +import io.quarkus.deployment.builditem.TransformedClassesBuildItem; import io.quarkus.deployment.pkg.builditem.ArtifactResultBuildItem; import io.quarkus.deployment.pkg.builditem.JarBuildItem; import io.quarkus.deployment.pkg.builditem.NativeImageBuildItem; @@ -125,7 +125,7 @@ public StartupActionImpl createInitialRuntimeApplication() { } ClassLoader classLoader = curatedApplication.createDeploymentClassLoader(); BuildResult result = runAugment(true, Collections.emptySet(), classLoader, GeneratedClassBuildItem.class, - GeneratedResourceBuildItem.class, BytecodeTransformerBuildItem.class, ApplicationClassNameBuildItem.class, + GeneratedResourceBuildItem.class, TransformedClassesBuildItem.class, ApplicationClassNameBuildItem.class, MainClassBuildItem.class); return new StartupActionImpl(curatedApplication, result, classLoader); } @@ -137,7 +137,7 @@ public StartupActionImpl reloadExistingApplication(boolean hasStartedSuccessfull } ClassLoader classLoader = curatedApplication.createDeploymentClassLoader(); BuildResult result = runAugment(!hasStartedSuccessfully, changedResources, classLoader, GeneratedClassBuildItem.class, - GeneratedResourceBuildItem.class, BytecodeTransformerBuildItem.class, ApplicationClassNameBuildItem.class, + GeneratedResourceBuildItem.class, TransformedClassesBuildItem.class, ApplicationClassNameBuildItem.class, MainClassBuildItem.class); return new StartupActionImpl(curatedApplication, result, classLoader); } diff --git a/core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java b/core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java index 1fcd84153c0ba..d103d405e5980 100644 --- a/core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java +++ b/core/deployment/src/main/java/io/quarkus/runner/bootstrap/StartupActionImpl.java @@ -9,20 +9,15 @@ import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.StandardOpenOption; -import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; -import java.util.function.BiFunction; import java.util.function.Consumer; -import java.util.function.Predicate; import org.jboss.logging.Logger; -import org.objectweb.asm.ClassVisitor; import io.quarkus.bootstrap.BootstrapDebug; import io.quarkus.bootstrap.app.CuratedApplication; @@ -32,12 +27,11 @@ import io.quarkus.bootstrap.classloading.QuarkusClassLoader; import io.quarkus.builder.BuildResult; import io.quarkus.deployment.builditem.ApplicationClassNameBuildItem; -import io.quarkus.deployment.builditem.BytecodeTransformerBuildItem; import io.quarkus.deployment.builditem.GeneratedClassBuildItem; import io.quarkus.deployment.builditem.GeneratedResourceBuildItem; import io.quarkus.deployment.builditem.MainClassBuildItem; +import io.quarkus.deployment.builditem.TransformedClassesBuildItem; import io.quarkus.deployment.configuration.RunTimeConfigurationGenerator; -import io.quarkus.deployment.index.ConstPoolScanner; import io.quarkus.dev.appstate.ApplicationStateNotification; import io.quarkus.runtime.Quarkus; @@ -54,30 +48,27 @@ public StartupActionImpl(CuratedApplication curatedApplication, BuildResult buil this.curatedApplication = curatedApplication; this.buildResult = buildResult; Set eagerClasses = new HashSet<>(); - Map> transformerPredicates = new HashMap<>(); - Map>> bytecodeTransformers = extractTransformers( - eagerClasses, transformerPredicates); + Map bytecodeTransformers = extractTransformers(eagerClasses); QuarkusClassLoader baseClassLoader = curatedApplication.getBaseRuntimeClassLoader(); QuarkusClassLoader runtimeClassLoader; //so we have some differences between dev and test mode here. //test mode only has a single class loader, while dev uses a disposable runtime class loader //that is discarded between restarts + Map resources = new HashMap<>(); + resources.putAll(extractGeneratedResources(true)); + resources.putAll(bytecodeTransformers); if (curatedApplication.getQuarkusBootstrap().getMode() == QuarkusBootstrap.Mode.DEV) { - baseClassLoader.reset(extractGeneratedResources(false), bytecodeTransformers, transformerPredicates, + baseClassLoader.reset(extractGeneratedResources(false), deploymentClassLoader); runtimeClassLoader = curatedApplication.createRuntimeClassLoader(baseClassLoader, - bytecodeTransformers, transformerPredicates, - deploymentClassLoader, extractGeneratedResources(true)); + deploymentClassLoader, resources); } else { - Map resources = new HashMap<>(); resources.putAll(extractGeneratedResources(false)); - resources.putAll(extractGeneratedResources(true)); - baseClassLoader.reset(resources, bytecodeTransformers, transformerPredicates, deploymentClassLoader); + baseClassLoader.reset(resources, deploymentClassLoader); runtimeClassLoader = baseClassLoader; } this.runtimeClassLoader = runtimeClassLoader; - handleEagerClasses(runtimeClassLoader, eagerClasses); } private void handleEagerClasses(QuarkusClassLoader runtimeClassLoader, Set eagerClasses) { @@ -262,40 +253,18 @@ public ClassLoader getClassLoader() { return runtimeClassLoader; } - private Map>> extractTransformers(Set eagerClasses, - Map> transformerPredicates) { - Map>> bytecodeTransformers = new HashMap<>(); - Set noConstScanning = new HashSet<>(); - Map> constScanning = new HashMap<>(); - List transformers = buildResult.consumeMulti(BytecodeTransformerBuildItem.class); - for (BytecodeTransformerBuildItem i : transformers) { - List> list = bytecodeTransformers.get(i.getClassToTransform()); - if (list == null) { - bytecodeTransformers.put(i.getClassToTransform(), list = new ArrayList<>()); - } - list.add(i.getVisitorFunction()); - if (i.isEager()) { - eagerClasses.add(i.getClassToTransform()); - } - if (i.getRequireConstPoolEntry() == null || i.getRequireConstPoolEntry().isEmpty()) { - noConstScanning.add(i.getClassToTransform()); - } else { - constScanning.computeIfAbsent(i.getClassToTransform(), (s) -> new HashSet<>()) - .addAll(i.getRequireConstPoolEntry()); - } - } - for (String i : noConstScanning) { - constScanning.remove(i); - } - for (Map.Entry> entry : constScanning.entrySet()) { - transformerPredicates.put(entry.getKey(), new Predicate() { - @Override - public boolean test(byte[] bytes) { - return ConstPoolScanner.constPoolEntryPresent(bytes, entry.getValue()); + private Map extractTransformers(Set eagerClasses) { + Map ret = new HashMap<>(); + TransformedClassesBuildItem transformers = buildResult.consume(TransformedClassesBuildItem.class); + for (Set i : transformers.getTransformedClassesByJar().values()) { + for (TransformedClassesBuildItem.TransformedClass clazz : i) { + ret.put(clazz.getFileName(), clazz.getData()); + if (clazz.isEager()) { + eagerClasses.add(clazz.getClassName()); } - }); + } } - return bytecodeTransformers; + return ret; } private Map extractGeneratedResources(boolean applicationClasses) { diff --git a/extensions/vertx-core/deployment/src/test/java/io/quarkus/vertx/core/deployment/VerticleWithClassNameDeploymentTest.java b/extensions/vertx-core/deployment/src/test/java/io/quarkus/vertx/core/deployment/VerticleWithClassNameDeploymentTest.java index 00ea7573c5ec9..3461793f16258 100644 --- a/extensions/vertx-core/deployment/src/test/java/io/quarkus/vertx/core/deployment/VerticleWithClassNameDeploymentTest.java +++ b/extensions/vertx-core/deployment/src/test/java/io/quarkus/vertx/core/deployment/VerticleWithClassNameDeploymentTest.java @@ -33,7 +33,7 @@ public class VerticleWithClassNameDeploymentTest { public void testDeploymentOfVerticleUsingClassName() { String resp1 = RestAssured.get("http://localhost:8080").asString(); String resp2 = RestAssured.get("http://localhost:8080").asString(); - Assertions.assertTrue(resp1.startsWith("OK")); + Assertions.assertTrue(resp1.startsWith("OK"), resp1); Assertions.assertTrue(resp2.startsWith("OK")); Assertions.assertNotEquals(resp1, resp2); } diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java index 90a4fccab71fc..f6caf80f85e59 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/app/CuratedApplication.java @@ -19,11 +19,8 @@ import java.util.Map; import java.util.Set; import java.util.function.BiConsumer; -import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; -import java.util.function.Predicate; -import org.objectweb.asm.ClassVisitor; /** * The result of the curate step that is done by QuarkusBootstrap. @@ -268,19 +265,16 @@ public QuarkusClassLoader createDeploymentClassLoader() { } public QuarkusClassLoader createRuntimeClassLoader(QuarkusClassLoader loader, - Map>> bytecodeTransformers, - Map> transformerPredicates, ClassLoader deploymentClassLoader, Map resources) { QuarkusClassLoader.Builder builder = QuarkusClassLoader.builder("Quarkus Runtime ClassLoader", loader, false) .setAggregateParentResources(true); - builder.setTransformerPredicates(transformerPredicates); builder.setTransformerClassLoader(deploymentClassLoader); + builder.addElement(new MemoryClassPathElement(resources)); for (Path root : quarkusBootstrap.getApplicationRoot()) { builder.addElement(ClassPathElement.fromPath(root)); } - builder.addElement(new MemoryClassPathElement(resources)); for (AdditionalDependency i : getQuarkusBootstrap().getAdditionalApplicationArchives()) { if (i.isHotReloadable()) { @@ -289,7 +283,6 @@ public QuarkusClassLoader createRuntimeClassLoader(QuarkusClassLoader loader, } } } - builder.setBytecodeTransformers(bytecodeTransformers); return builder.build(); } diff --git a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java index f239fac8263b7..0e810e0c220c5 100644 --- a/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java +++ b/independent-projects/bootstrap/core/src/main/java/io/quarkus/bootstrap/classloading/QuarkusClassLoader.java @@ -1,10 +1,7 @@ package io.quarkus.bootstrap.classloading; -import io.quarkus.bootstrap.BootstrapDebug; import java.io.ByteArrayInputStream; import java.io.Closeable; -import java.io.File; -import java.io.FileOutputStream; import java.io.IOException; import java.io.InputStream; import java.lang.reflect.InvocationTargetException; @@ -22,14 +19,9 @@ import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; -import java.util.function.BiFunction; -import java.util.function.Predicate; import java.util.jar.Attributes; import java.util.jar.Manifest; import org.jboss.logging.Logger; -import org.objectweb.asm.ClassReader; -import org.objectweb.asm.ClassVisitor; -import org.objectweb.asm.ClassWriter; /** * The ClassLoader used for non production Quarkus applications (i.e. dev and test mode). Production @@ -75,8 +67,6 @@ public class QuarkusClassLoader extends ClassLoader implements Closeable { */ private volatile MemoryClassPathElement resettableElement; - private volatile Map>> bytecodeTransformers; - private volatile Map> transformerPredicates; private volatile ClassLoader transformerClassLoader; private volatile ClassLoaderState state; @@ -104,8 +94,6 @@ private QuarkusClassLoader(Builder builder) { super(PLATFORM_CLASS_LOADER); this.name = builder.name; this.elements = builder.elements; - this.bytecodeTransformers = builder.bytecodeTransformers; - this.transformerPredicates = builder.transformerPredicates; this.bannedElements = builder.bannedElements; this.parentFirstElements = builder.parentFirstElements; this.lesserPriorityElements = builder.lesserPriorityElements; @@ -135,17 +123,13 @@ private boolean parentFirst(String name, ClassLoaderState state) { } public void reset(Map resources, - Map>> bytecodeTransformers, - Map> transformerPredicates, ClassLoader transformerClassLoader) { if (resettableElement == null) { throw new IllegalStateException("Classloader is not resettable"); } this.transformerClassLoader = transformerClassLoader; synchronized (this) { - this.transformerPredicates = transformerPredicates; resettableElement.reset(resources); - this.bytecodeTransformers = bytecodeTransformers; state = null; } } @@ -366,26 +350,6 @@ protected Class loadClass(String name, boolean resolve) throws ClassNotFoundE ClassPathResource classPathElementResource = classPathElement.getResource(resourceName); if (classPathElementResource != null) { //can happen if the class loader was closed byte[] data = classPathElementResource.getData(); - List> transformers = bytecodeTransformers.get(name); - Map> constPoolEntries = transformerPredicates; - if (transformers != null) { - data = handleTransform(name, data, transformers, constPoolEntries); - - if (BootstrapDebug.DEBUG_TRANSFORMED_CLASSES_DIR != null) { - File debugPath = new File(BootstrapDebug.DEBUG_TRANSFORMED_CLASSES_DIR); - if (!debugPath.exists()) { - debugPath.mkdir(); - } - File classFile = new File(debugPath, name.replace(".", "/") + ".class"); - classFile.getParentFile().mkdirs(); - try (FileOutputStream classWriter = new FileOutputStream(classFile)) { - classWriter.write(data); - } catch (Exception e) { - log.errorf(e, "Failed to write transformed class %s", name); - } - log.infof("Wrote transformed class to %s", classFile.getAbsolutePath()); - } - } definePackage(name, classPathElement); return defineClass(name, data, 0, data.length, protectionDomains.computeIfAbsent(classPathElement, (ce) -> ce.getProtectionDomain(this))); @@ -458,30 +422,6 @@ public List getElementsWithResource(String name, boolean local return ret; } - private byte[] handleTransform(String name, byte[] bytes, - List> transformers, - Map> transformerPredicates) { - if (transformerPredicates.containsKey(name)) { - if (!transformerPredicates.get(name).test(bytes)) { - return bytes; - } - } - ClassReader cr = new ClassReader(bytes); - ClassWriter writer = new ClassWriter(cr, ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS) { - - @Override - protected ClassLoader getClassLoader() { - return transformerClassLoader; - } - }; - ClassVisitor visitor = writer; - for (BiFunction i : transformers) { - visitor = i.apply(name, visitor); - } - cr.accept(visitor, 0); - return writer.toByteArray(); - } - @SuppressWarnings("unused") public Class visibleDefineClass(String name, byte[] b, int off, int len) throws ClassFormatError { return super.defineClass(name, b, off, len); @@ -513,8 +453,6 @@ public static class Builder { final List bannedElements = new ArrayList<>(); final List parentFirstElements = new ArrayList<>(); final List lesserPriorityElements = new ArrayList<>(); - Map> transformerPredicates = Collections.emptyMap(); - Map>> bytecodeTransformers = Collections.emptyMap(); final boolean parentFirst; MemoryClassPathElement resettableElement; private volatile ClassLoader transformerClassLoader; @@ -612,29 +550,6 @@ public Builder addLesserPriorityElement(ClassPathElement element) { return this; } - /** - * Sets any bytecode transformers that should be applied to this Class Loader - * - * @param bytecodeTransformers - */ - public void setBytecodeTransformers( - Map>> bytecodeTransformers) { - if (bytecodeTransformers == null) { - this.bytecodeTransformers = Collections.emptyMap(); - } else { - this.bytecodeTransformers = bytecodeTransformers; - } - } - - /** - * Sets const pool entries that need to be present for a class to be transformed. This optimisation - * allows for transformation to be skipped if the scanner detects that no transformation is required. - */ - public Builder setTransformerPredicates(Map> transformerPredicates) { - this.transformerPredicates = transformerPredicates; - return this; - } - /** * If this is true then a getResources call will always include the parent resources. * diff --git a/independent-projects/bootstrap/maven-resolver/pom.xml b/independent-projects/bootstrap/maven-resolver/pom.xml index fe2b7c931c8ef..a40d9fbf1d251 100644 --- a/independent-projects/bootstrap/maven-resolver/pom.xml +++ b/independent-projects/bootstrap/maven-resolver/pom.xml @@ -18,10 +18,6 @@ io.quarkus quarkus-bootstrap-app-model - - org.ow2.asm - asm - org.apache.maven maven-embedder diff --git a/independent-projects/bootstrap/pom.xml b/independent-projects/bootstrap/pom.xml index 5bf074fc18a13..3c782a8b76a2b 100644 --- a/independent-projects/bootstrap/pom.xml +++ b/independent-projects/bootstrap/pom.xml @@ -41,7 +41,6 @@ 1.0 3.9 27.0.1-jre - 7.3.1 1.2.6 1.0.4 20.1.0 @@ -56,11 +55,6 @@ - - org.ow2.asm - asm - ${asm.version} - com.google.guava guava