From f5e2a5758cec9e1bcf7dfc96bf2cdb72fa1869fa Mon Sep 17 00:00:00 2001 From: Guillaume Smet Date: Wed, 1 Jul 2020 12:44:39 +0200 Subject: [PATCH] Revert "Simplify runtime class transformation" This reverts commit 18f1ca2792940386cc50b0efa70c15c25d307505. --- .../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, 159 insertions(+), 42 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 3f0aa90194fca..edad767ad912a 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,16 +36,12 @@ 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(String className, byte[] data, String fileName, boolean eager) { - this.className = className; + public TransformedClass(byte[] data, String fileName) { this.data = data; this.fileName = fileName; - this.eager = eager; } public byte[] getData() { @@ -56,14 +52,6 @@ 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 b561d6ce92e7b..cdbeb0c057285 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,7 +45,6 @@ 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()); @@ -55,9 +54,6 @@ 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<>(); @@ -104,8 +100,8 @@ public TransformedClassesBuildItem.TransformedClass call() throws Exception { visitor = i.apply(className, visitor); } cr.accept(visitor, 0); - return new TransformedClassesBuildItem.TransformedClass(className, writer.toByteArray(), - classFileName, eager.contains(className)); + return new TransformedClassesBuildItem.TransformedClass(writer.toByteArray(), + classFileName); } 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 94abf8a01034c..db4c96997a999 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,6 +32,7 @@ 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; @@ -40,7 +41,6 @@ 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, TransformedClassesBuildItem.class, ApplicationClassNameBuildItem.class, + GeneratedResourceBuildItem.class, BytecodeTransformerBuildItem.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, TransformedClassesBuildItem.class, ApplicationClassNameBuildItem.class, + GeneratedResourceBuildItem.class, BytecodeTransformerBuildItem.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 ffc480bd96fe1..6fed3f94104a7 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,15 +9,20 @@ 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; @@ -27,11 +32,12 @@ 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; @@ -48,27 +54,30 @@ public StartupActionImpl(CuratedApplication curatedApplication, BuildResult buil this.curatedApplication = curatedApplication; this.buildResult = buildResult; Set eagerClasses = new HashSet<>(); - Map bytecodeTransformers = extractTransformers(eagerClasses); + Map> transformerPredicates = new HashMap<>(); + Map>> bytecodeTransformers = extractTransformers( + eagerClasses, transformerPredicates); 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), + baseClassLoader.reset(extractGeneratedResources(false), bytecodeTransformers, transformerPredicates, deploymentClassLoader); runtimeClassLoader = curatedApplication.createRuntimeClassLoader(baseClassLoader, - deploymentClassLoader, resources); + bytecodeTransformers, transformerPredicates, + deploymentClassLoader, extractGeneratedResources(true)); } else { + Map resources = new HashMap<>(); resources.putAll(extractGeneratedResources(false)); - baseClassLoader.reset(resources, deploymentClassLoader); + resources.putAll(extractGeneratedResources(true)); + baseClassLoader.reset(resources, bytecodeTransformers, transformerPredicates, deploymentClassLoader); runtimeClassLoader = baseClassLoader; } this.runtimeClassLoader = runtimeClassLoader; + handleEagerClasses(runtimeClassLoader, eagerClasses); } private void handleEagerClasses(QuarkusClassLoader runtimeClassLoader, Set eagerClasses) { @@ -253,18 +262,40 @@ public ClassLoader getClassLoader() { return runtimeClassLoader; } - 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()); - } + 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()); + } + }); } - return ret; + return bytecodeTransformers; } 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 3461793f16258..00ea7573c5ec9 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"), resp1); + Assertions.assertTrue(resp1.startsWith("OK")); 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 f6caf80f85e59..90a4fccab71fc 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,8 +19,11 @@ 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. @@ -265,16 +268,19 @@ 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()) { @@ -283,6 +289,7 @@ 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 0e810e0c220c5..f239fac8263b7 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,7 +1,10 @@ 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; @@ -19,9 +22,14 @@ 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 @@ -67,6 +75,8 @@ 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; @@ -94,6 +104,8 @@ 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; @@ -123,13 +135,17 @@ 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; } } @@ -350,6 +366,26 @@ 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))); @@ -422,6 +458,30 @@ 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); @@ -453,6 +513,8 @@ 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; @@ -550,6 +612,29 @@ 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 a40d9fbf1d251..fe2b7c931c8ef 100644 --- a/independent-projects/bootstrap/maven-resolver/pom.xml +++ b/independent-projects/bootstrap/maven-resolver/pom.xml @@ -18,6 +18,10 @@ 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 47cecc2bb5eb8..43e72b4a058b2 100644 --- a/independent-projects/bootstrap/pom.xml +++ b/independent-projects/bootstrap/pom.xml @@ -41,6 +41,7 @@ 1.0 3.9 27.0.1-jre + 7.3.1 1.2.6 1.0.4 20.1.0 @@ -56,6 +57,11 @@ + + org.ow2.asm + asm + ${asm.version} + com.google.guava guava