Speedup Flyway by preventing it from doing classpath scanning

geoand · geoand · commit b1a64d0ae4e2 · 2020-05-22T20:54:03.000+03:00
This PR essentially takes the substitution we had for native mode and turns it into a class transformer. This way even in JVM mode our Flyway integration take advantage of the fact that the migration script locations are known at build time, thus no classpath scanning is needed. This solution isn't ideal from a maintenance perspective, but the transformation is relatively straightforward and our test suite for Flyway pretty extensive, so we should easily be able to adapt to future Flyway updates (ideally we would be able to remove this if /when flyway/flyway#2822 is done) Relates to: #9428
diff --git a/extensions/flyway/deployment/src/main/java/io/quarkus/flyway/FlywayProcessor.java b/extensions/flyway/deployment/src/main/java/io/quarkus/flyway/FlywayProcessor.java
@@ -40,6 +40,7 @@
 import io.quarkus.deployment.annotations.BuildStep;
 import io.quarkus.deployment.annotations.ExecutionTime;
 import io.quarkus.deployment.annotations.Record;
+import io.quarkus.deployment.builditem.BytecodeTransformerBuildItem;
 import io.quarkus.deployment.builditem.CapabilityBuildItem;
 import io.quarkus.deployment.builditem.FeatureBuildItem;
 import io.quarkus.deployment.builditem.ServiceStartBuildItem;
@@ -65,6 +66,13 @@ CapabilityBuildItem capability() {
         return new CapabilityBuildItem(Capabilities.FLYWAY);
     }
 
+    @BuildStep
+    void scannerTransformer(BuildProducer<BytecodeTransformerBuildItem> transformers) {
+        transformers
+                .produce(new BytecodeTransformerBuildItem(true, ScannerTransformer.FLYWAY_SCANNER_CLASS_NAME,
+                        new ScannerTransformer()));
+    }
+
     @Record(STATIC_INIT)
     @BuildStep
     void build(BuildProducer<FeatureBuildItem> featureProducer,
@@ -197,7 +205,9 @@ private Set<String> getApplicationMigrationsFromPath(final String location, fina
         try (final Stream<Path> pathStream = Files.walk(Paths.get(path.toURI()))) {
             return pathStream.filter(Files::isRegularFile)
                     .map(it -> Paths.get(location, it.getFileName().toString()).toString())
-                    .peek(it -> LOGGER.debug("Discovered: " + it))
+                    // we don't want windows paths here since the paths are going to be used as classpath paths anyway
+                    .map(it -> it.replace('\\', '/'))
+                    .peek(it -> LOGGER.debugf("Discovered path: %s", it))
                     .collect(Collectors.toSet());
         }
     }
diff --git a/extensions/flyway/deployment/src/main/java/io/quarkus/flyway/ScannerTransformer.java b/extensions/flyway/deployment/src/main/java/io/quarkus/flyway/ScannerTransformer.java
@@ -0,0 +1,121 @@
+package io.quarkus.flyway;
+
+import static org.objectweb.asm.Opcodes.ALOAD;
+import static org.objectweb.asm.Opcodes.ASTORE;
+import static org.objectweb.asm.Opcodes.DUP;
+import static org.objectweb.asm.Opcodes.GETFIELD;
+import static org.objectweb.asm.Opcodes.INVOKEINTERFACE;
+import static org.objectweb.asm.Opcodes.INVOKESPECIAL;
+import static org.objectweb.asm.Opcodes.NEW;
+import static org.objectweb.asm.Opcodes.POP;
+import static org.objectweb.asm.Opcodes.PUTFIELD;
+import static org.objectweb.asm.Opcodes.RETURN;
+
+import java.util.function.BiFunction;
+
+import org.flywaydb.core.internal.scanner.Scanner;
+import org.flywaydb.core.internal.scanner.classpath.ResourceAndClassScanner;
+import org.objectweb.asm.ClassVisitor;
+import org.objectweb.asm.MethodVisitor;
+
+import io.quarkus.flyway.runtime.QuarkusPathLocationScanner;
+import io.quarkus.gizmo.Gizmo;
+
+/**
+ * Transforms {@link Scanner} in a way to take advantage of our build time knowledge
+ * This should be removed completely if https://github.com/flyway/flyway/issues/2822
+ * is implemented
+ */
+class ScannerTransformer implements BiFunction<String, ClassVisitor, ClassVisitor> {
+
+    static final String FLYWAY_SCANNER_CLASS_NAME = Scanner.class.getName();
+    private static final String FLYWAY_SCANNER_INTERNAL_CLASS_NAME = FLYWAY_SCANNER_CLASS_NAME.replace('.', '/');
+
+    private static final String FLYWAY_RESOURCE_AND_CLASS_SCANNER_CLASS_NAME = ResourceAndClassScanner.class.getName();
+    private static final String FLYWAY_RESOURCE_AND_CLASS_SCANNER_INTERNAL_CLASS_NAME = FLYWAY_RESOURCE_AND_CLASS_SCANNER_CLASS_NAME
+            .replace('.', '/');
+
+    private static final String QUARKUS_RESOURCE_AND_CLASS_SCANNER_CLASS_NAME = QuarkusPathLocationScanner.class.getName();
+    private static final String QUARKUS_RESOURCE_AND_CLASS_SCANNER_INTERNAL_CLASS_NAME = QUARKUS_RESOURCE_AND_CLASS_SCANNER_CLASS_NAME
+            .replace('.', '/');
+
+    private static final String CTOR_METHOD_NAME = "<init>";
+
+    @Override
+    public ClassVisitor apply(String s, ClassVisitor cv) {
+        return new ScannerVisitor(cv);
+    }
+
+    private static final class ScannerVisitor extends ClassVisitor {
+
+        public ScannerVisitor(ClassVisitor cv) {
+            super(Gizmo.ASM_API_VERSION, cv);
+        }
+
+        @Override
+        public MethodVisitor visitMethod(int access, String name, String descriptor, String signature, String[] exceptions) {
+            MethodVisitor mv = super.visitMethod(access, name, descriptor, signature, exceptions);
+            if (name.equals(CTOR_METHOD_NAME)) {
+                return new ConstructorTransformer(mv);
+            }
+            return mv;
+        }
+
+        /**
+         * Replaces the constructor of the {@link Scanner} with:
+         *
+         * <pre>
+         * public ScannerSubstitutions(Class<?> implementedInterface, Collection<Location> locations, ClassLoader classLoader,
+         *         Charset encoding, ResourceNameCache resourceNameCache, LocationScannerCache locationScannerCache) {
+         *     ResourceAndClassScanner quarkusScanner = new QuarkusPathLocationScanner(locations);
+         *     resources.addAll(quarkusScanner.scanForResources());
+         *     classes.addAll(quarkusScanner.scanForClasses());
+         * }
+         * </pre>
+         */
+        private static class ConstructorTransformer extends MethodVisitor {
+
+            public ConstructorTransformer(MethodVisitor mv) {
+                super(Gizmo.ASM_API_VERSION, mv);
+            }
+
+            @Override
+            public void visitCode() {
+                super.visitVarInsn(ALOAD, 0);
+                super.visitMethodInsn(INVOKESPECIAL, "java/lang/Object", CTOR_METHOD_NAME, "()V", false);
+                super.visitVarInsn(ALOAD, 0);
+                super.visitTypeInsn(NEW, "java/util/ArrayList");
+                super.visitInsn(DUP);
+                super.visitMethodInsn(INVOKESPECIAL, "java/util/ArrayList", CTOR_METHOD_NAME, "()V", false);
+                super.visitFieldInsn(PUTFIELD, FLYWAY_SCANNER_INTERNAL_CLASS_NAME, "resources", "Ljava/util/List;");
+                super.visitVarInsn(ALOAD, 0);
+                super.visitTypeInsn(NEW, "java/util/ArrayList");
+                super.visitInsn(DUP);
+                super.visitMethodInsn(INVOKESPECIAL, "java/util/ArrayList", CTOR_METHOD_NAME, "()V", false);
+                super.visitFieldInsn(PUTFIELD, FLYWAY_SCANNER_INTERNAL_CLASS_NAME, "classes", "Ljava/util/List;");
+                super.visitTypeInsn(NEW, QUARKUS_RESOURCE_AND_CLASS_SCANNER_INTERNAL_CLASS_NAME);
+                super.visitInsn(DUP);
+                super.visitVarInsn(ALOAD, 2);
+                super.visitMethodInsn(INVOKESPECIAL, QUARKUS_RESOURCE_AND_CLASS_SCANNER_INTERNAL_CLASS_NAME,
+                        CTOR_METHOD_NAME, "(Ljava/util/Collection;)V", false);
+                super.visitVarInsn(ASTORE, 7);
+                super.visitVarInsn(ALOAD, 0);
+                super.visitFieldInsn(GETFIELD, FLYWAY_SCANNER_INTERNAL_CLASS_NAME, "resources", "Ljava/util/List;");
+                super.visitVarInsn(ALOAD, 7);
+                super.visitMethodInsn(INVOKEINTERFACE, FLYWAY_RESOURCE_AND_CLASS_SCANNER_INTERNAL_CLASS_NAME,
+                        "scanForResources", "()Ljava/util/Collection;", true);
+                super.visitMethodInsn(INVOKEINTERFACE, "java/util/List", "addAll", "(Ljava/util/Collection;)Z", true);
+                super.visitInsn(POP);
+                super.visitVarInsn(ALOAD, 0);
+                super.visitFieldInsn(GETFIELD, FLYWAY_SCANNER_INTERNAL_CLASS_NAME, "classes", "Ljava/util/List;");
+                super.visitVarInsn(ALOAD, 7);
+                super.visitMethodInsn(INVOKEINTERFACE, FLYWAY_RESOURCE_AND_CLASS_SCANNER_INTERNAL_CLASS_NAME,
+                        "scanForClasses", "()Ljava/util/Collection;", true);
+                super.visitMethodInsn(INVOKEINTERFACE, "java/util/List", "addAll", "(Ljava/util/Collection;)Z", true);
+                super.visitInsn(POP);
+                super.visitInsn(RETURN);
+                super.visitMaxs(3, 8);
+            }
+        }
+    }
+}
diff --git a/extensions/flyway/deployment/src/test/java/io/quarkus/flyway/test/FlywayExtensionMigrateAtStartNamedDataSourceTest.java b/extensions/flyway/deployment/src/test/java/io/quarkus/flyway/test/FlywayExtensionMigrateAtStartNamedDataSourceTest.java
@@ -1,10 +1,12 @@
 package io.quarkus.flyway.test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 import javax.inject.Inject;
 
 import org.flywaydb.core.Flyway;
+import org.flywaydb.core.api.MigrationInfo;
 import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.shrinkwrap.api.spec.JavaArchive;
 import org.junit.jupiter.api.DisplayName;
@@ -32,7 +34,12 @@ public class FlywayExtensionMigrateAtStartNamedDataSourceTest {
     @Test
     @DisplayName("Migrates at start for datasource named 'users' correctly")
     public void testFlywayConfigInjection() {
-        String currentVersion = flywayUsers.info().current().getVersion().toString();
+        MigrationInfo migrationInfo = flywayUsers.info().current();
+        assertNotNull(migrationInfo, "No Flyway migration was executed");
+
+        String currentVersion = migrationInfo
+                .getVersion()
+                .toString();
         // Expected to be 1.0.0 as migration runs at start
         assertEquals("1.0.0", currentVersion);
     }
diff --git a/extensions/flyway/deployment/src/test/java/io/quarkus/flyway/test/FlywayExtensionMigrateAtStartTest.java b/extensions/flyway/deployment/src/test/java/io/quarkus/flyway/test/FlywayExtensionMigrateAtStartTest.java
@@ -1,10 +1,12 @@
 package io.quarkus.flyway.test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 import javax.inject.Inject;
 
 import org.flywaydb.core.Flyway;
+import org.flywaydb.core.api.MigrationInfo;
 import org.jboss.shrinkwrap.api.ShrinkWrap;
 import org.jboss.shrinkwrap.api.spec.JavaArchive;
 import org.junit.jupiter.api.DisplayName;
@@ -27,7 +29,12 @@ public class FlywayExtensionMigrateAtStartTest {
     @Test
     @DisplayName("Migrates at start correctly")
     public void testFlywayConfigInjection() {
-        String currentVersion = flyway.info().current().getVersion().toString();
+        MigrationInfo migrationInfo = flyway.info().current();
+        assertNotNull(migrationInfo, "No Flyway migration was executed");
+
+        String currentVersion = migrationInfo
+                .getVersion()
+                .toString();
         // Expected to be 1.0.0 as migration runs at start
         assertEquals("1.0.0", currentVersion);
     }
diff --git a/extensions/flyway/deployment/src/test/resources/migrate-at-start-config-named-datasource.properties b/extensions/flyway/deployment/src/test/resources/migrate-at-start-config-named-datasource.properties
@@ -1,3 +1,5 @@
+quarkus.log.category."org.flywaydb.core".level=DEBUG
+quarkus.log.category."io.quarkus.flyway".level=DEBUG
 quarkus.datasource.users.db-kind=h2
 quarkus.datasource.users.username=sa
 quarkus.datasource.users.password=sa
diff --git a/extensions/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/FlywayRecorder.java b/extensions/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/FlywayRecorder.java
@@ -7,18 +7,21 @@
 import javax.sql.DataSource;
 
 import org.flywaydb.core.Flyway;
+import org.jboss.logging.Logger;
 
 import io.quarkus.agroal.runtime.DataSources;
 import io.quarkus.arc.Arc;
-import io.quarkus.flyway.runtime.graal.QuarkusPathLocationScanner;
 import io.quarkus.runtime.annotations.Recorder;
 
 @Recorder
 public class FlywayRecorder {
 
+    private static final Logger log = Logger.getLogger(FlywayRecorder.class);
+
     private final List<FlywayContainer> flywayContainers = new ArrayList<>(2);
 
     public void setApplicationMigrationFiles(List<String> migrationFiles) {
+        log.debugv("Setting the following application migration files: {0}", migrationFiles);
         QuarkusPathLocationScanner.setApplicationMigrationFiles(migrationFiles);
     }
 
diff --git a/extensions/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/QuarkusPathLocationScanner.java b/extensions/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/QuarkusPathLocationScanner.java
@@ -1,4 +1,4 @@
-package io.quarkus.flyway.runtime.graal;
+package io.quarkus.flyway.runtime;
 
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
@@ -12,6 +12,11 @@
 import org.flywaydb.core.internal.scanner.classpath.ResourceAndClassScanner;
 import org.jboss.logging.Logger;
 
+/**
+ * This class is used in order to prevent Flyway from doing classpath scanning which is both slow
+ * and won't work in native mode
+ */
+@SuppressWarnings("rawtypes")
 public final class QuarkusPathLocationScanner implements ResourceAndClassScanner {
     private static final Logger LOGGER = Logger.getLogger(QuarkusPathLocationScanner.class);
     private static final String LOCATION_SEPARATOR = "/";
@@ -20,6 +25,8 @@ public final class QuarkusPathLocationScanner implements ResourceAndClassScanner
     private final Collection<LoadableResource> scannedResources;
 
     public QuarkusPathLocationScanner(Collection<Location> locations) {
+        LOGGER.debugv("Locations: {0}", locations);
+
         this.scannedResources = new ArrayList<>();
         ClassLoader classLoader = Thread.currentThread().getContextClassLoader();
 
@@ -50,6 +57,9 @@ private boolean canHandleMigrationFile(Collection<Location> locations, String mi
 
             if (migrationFile.startsWith(locationPath)) {
                 return true;
+            } else {
+                LOGGER.debugf("Migration file '%s' will be ignored because it does not start with '%s'", migrationFile,
+                        locationPath);
             }
         }
 
diff --git a/extensions/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/graal/ScannerSubstitutions.java b/extensions/flyway/runtime/src/main/java/io/quarkus/flyway/runtime/graal/ScannerSubstitutions.java

Original file line number	Diff line number	Diff line change
`@@ -1,3 +1,5 @@`
	`1`	`+quarkus.log.category."org.flywaydb.core".level=DEBUG`
	`2`	`+quarkus.log.category."io.quarkus.flyway".level=DEBUG`
`1`	`3`	`quarkus.datasource.users.db-kind=h2`
`2`	`4`	`quarkus.datasource.users.username=sa`
`3`	`5`	`quarkus.datasource.users.password=sa`