Merge pull request #49579 from gsmet/hibernate-proxy-improvements

Hibernate ORM - Assorted improvements to proxy generation

Author: yrodiere

extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateEntityEnhancer.java

@@ -36,7 +36,7 @@ public final class HibernateEntityEnhancer implements BiFunction<String, ClassVi
             ClassFileVersion.JAVA_V17);
 
     //Choose this set to include Jakarta annotations, basic Java types such as String and Map, Hibernate annotations, and Panache supertypes:
-    private static final CoreTypePool CORE_POOL = new CoreTypePool(
+    static final CoreTypePool CORE_TYPE_POOL = new CoreTypePool(
             "java.",
             "jakarta.",
             "org.hibernate.bytecode.enhance.spi.",
@@ -64,6 +64,7 @@ public HibernateEnhancingClassVisitor(String className, ClassVisitor outputClass
             //Careful: the ASM API version needs to match the ASM version of Gizmo, not the one from Byte Buddy.
             //Most often these match - but occasionally they will diverge which is acceptable as Byte Buddy is shading ASM.
             super(Gizmo.ASM_API_VERSION, new QuarkusClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS));
+
             this.className = className;
             this.outputClassVisitor = outputClassVisitor;
             this.enhancerHolder = enhancerHolder;
@@ -103,7 +104,7 @@ public Enhancer getEnhancer() {
                 synchronized (this) {
                     if (actualEnhancer == null) {
                         EnhancerClassLocator enhancerClassLocator = ModelTypePool
-                                .buildModelTypePool(QuarkusClassFileLocator.INSTANCE, CORE_POOL);
+                                .buildModelTypePool(QuarkusClassFileLocator.INSTANCE, CORE_TYPE_POOL);
                         actualEnhancer = PROVIDER.getEnhancer(QuarkusEnhancementContext.INSTANCE, enhancerClassLocator);
                     }
                 }

extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/HibernateOrmProcessor.java

@@ -29,6 +29,11 @@
 import java.util.Properties;
 import java.util.Set;
 import java.util.TreeSet;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentLinkedDeque;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Future;
 import java.util.function.BiConsumer;
 import java.util.stream.Collectors;
 
@@ -111,6 +116,7 @@
 import io.quarkus.hibernate.orm.PersistenceUnit;
 import io.quarkus.hibernate.orm.deployment.integration.HibernateOrmIntegrationRuntimeConfiguredBuildItem;
 import io.quarkus.hibernate.orm.deployment.integration.HibernateOrmIntegrationStaticConfiguredBuildItem;
+import io.quarkus.hibernate.orm.deployment.integration.QuarkusClassFileLocator;
 import io.quarkus.hibernate.orm.deployment.spi.AdditionalJpaModelBuildItem;
 import io.quarkus.hibernate.orm.deployment.spi.DatabaseKindDialectBuildItem;
 import io.quarkus.hibernate.orm.dev.HibernateOrmDevIntegrator;
@@ -144,6 +150,8 @@
 import net.bytebuddy.dynamic.ClassFileLocator;
 import net.bytebuddy.dynamic.DynamicType;
 import net.bytebuddy.pool.TypePool;
+import net.bytebuddy.pool.TypePool.CacheProvider;
+import net.bytebuddy.pool.TypePool.Default.ReaderMode;
 
 /**
  * Simulacrum of JPA bootstrap.
@@ -462,7 +470,8 @@ public BytecodeRecorderConstantDefinitionBuildItem pregenProxies(
             List<PersistenceUnitDescriptorBuildItem> persistenceUnitDescriptorBuildItems,
             List<AdditionalJpaModelBuildItem> additionalJpaModelBuildItems,
             BuildProducer<GeneratedClassBuildItem> generatedClassBuildItemBuildProducer,
-            LiveReloadBuildItem liveReloadBuildItem) {
+            LiveReloadBuildItem liveReloadBuildItem,
+            ExecutorService buildExecutor) throws ExecutionException, InterruptedException {
         Set<String> managedClassAndPackageNames = new HashSet<>(jpaModel.getEntityClassNames());
         for (PersistenceUnitDescriptorBuildItem pud : persistenceUnitDescriptorBuildItems) {
             // Note: getManagedClassNames() can also return *package* names
@@ -476,9 +485,9 @@ public BytecodeRecorderConstantDefinitionBuildItem pregenProxies(
             managedClassAndPackageNames.add(additionalJpaModelBuildItem.getClassName());
         }
 
-        PreGeneratedProxies proxyDefinitions = generatedProxies(managedClassAndPackageNames,
+        PreGeneratedProxies proxyDefinitions = generateProxies(managedClassAndPackageNames,
                 indexBuildItem.getIndex(), transformedClassesBuildItem,
-                generatedClassBuildItemBuildProducer, liveReloadBuildItem);
+                generatedClassBuildItemBuildProducer, liveReloadBuildItem, buildExecutor);
 
         // Make proxies available through a constant;
         // this is a hack to avoid introducing circular dependencies between build steps.
@@ -1369,53 +1378,65 @@ private static MultiTenancyStrategy getMultiTenancyStrategy(Optional<String> mul
         return multiTenancyStrategy;
     }
 
-    private PreGeneratedProxies generatedProxies(Set<String> managedClassAndPackageNames, IndexView combinedIndex,
+    private PreGeneratedProxies generateProxies(Set<String> managedClassAndPackageNames, IndexView combinedIndex,
             TransformedClassesBuildItem transformedClassesBuildItem,
             BuildProducer<GeneratedClassBuildItem> generatedClassBuildItemBuildProducer,
-            LiveReloadBuildItem liveReloadBuildItem) {
+            LiveReloadBuildItem liveReloadBuildItem,
+            ExecutorService buildExecutor) throws ExecutionException, InterruptedException {
         ProxyCache proxyCache = liveReloadBuildItem.getContextObject(ProxyCache.class);
         if (proxyCache == null) {
             proxyCache = new ProxyCache();
             liveReloadBuildItem.setContextObject(ProxyCache.class, proxyCache);
         }
-        Set<String> changedClasses = Collections.emptySet();
+        Set<String> changedClasses = Set.of();
         if (liveReloadBuildItem.getChangeInformation() != null) {
             changedClasses = liveReloadBuildItem.getChangeInformation().getChangedClasses();
         } else {
             //we don't have class change info, invalidate the cache
             proxyCache.cache.clear();
         }
         //create a map of entity to proxy type
-        PreGeneratedProxies preGeneratedProxies = new PreGeneratedProxies();
         TypePool transformedClassesTypePool = createTransformedClassesTypePool(transformedClassesBuildItem,
                 managedClassAndPackageNames);
+
+        PreGeneratedProxies preGeneratedProxies = new PreGeneratedProxies();
+
         try (ProxyBuildingHelper proxyHelper = new ProxyBuildingHelper(transformedClassesTypePool)) {
+            final ConcurrentLinkedDeque<Future<CachedProxy>> generatedProxyQueue = new ConcurrentLinkedDeque<>();
+            Set<String> proxyInterfaceNames = Set.of(ClassNames.HIBERNATE_PROXY.toString());
+
             for (String managedClassOrPackageName : managedClassAndPackageNames) {
-                CachedProxy result;
                 if (proxyCache.cache.containsKey(managedClassOrPackageName)
                         && !isModified(managedClassOrPackageName, changedClasses, combinedIndex)) {
-                    result = proxyCache.cache.get(managedClassOrPackageName);
+                    CachedProxy proxy = proxyCache.cache.get(managedClassOrPackageName);
+                    generatedProxyQueue.add(CompletableFuture.completedFuture(proxy));
                 } else {
-                    Set<String> proxyInterfaceNames = new TreeSet<>();
-                    proxyInterfaceNames.add(ClassNames.HIBERNATE_PROXY.toString()); //always added
-                    if (!proxyHelper.isProxiable(managedClassOrPackageName)) {
-                        // we need to make sure the actual class is proxiable
+                    if (!proxyHelper.isProxiable(combinedIndex.getClassByName(managedClassOrPackageName))) {
+                        // we need to make sure we have a class and not a package and that it is proxiable
                         continue;
                     }
-                    final String mappedClass = managedClassOrPackageName;
-                    DynamicType.Unloaded<?> unloaded = proxyHelper.buildUnloadedProxy(mappedClass, proxyInterfaceNames);
-                    result = new CachedProxy(unloaded, proxyInterfaceNames);
-                    proxyCache.cache.put(managedClassOrPackageName, result);
+                    // we now are sure we have a proper class and not a package, let's avoid the confusion
+                    String managedClass = managedClassOrPackageName;
+                    generatedProxyQueue.add(buildExecutor.submit(() -> {
+                        DynamicType.Unloaded<?> unloaded = proxyHelper.buildUnloadedProxy(managedClass,
+                                proxyInterfaceNames);
+                        return new CachedProxy(managedClass, unloaded, proxyInterfaceNames);
+                    }));
                 }
-                for (Entry<TypeDescription, byte[]> i : result.proxyDef.getAllTypes().entrySet()) {
+            }
+
+            for (Future<CachedProxy> proxyFuture : generatedProxyQueue) {
+                CachedProxy proxy = proxyFuture.get();
+                proxyCache.cache.put(proxy.managedClassName, proxy);
+                for (Entry<TypeDescription, byte[]> i : proxy.proxyDef.getAllTypes().entrySet()) {
                     generatedClassBuildItemBuildProducer
                             .produce(new GeneratedClassBuildItem(true, i.getKey().getName(), i.getValue()));
                 }
-                preGeneratedProxies.getProxies().put(managedClassOrPackageName,
-                        new PreGeneratedProxies.ProxyClassDetailsHolder(result.proxyDef.getTypeDescription().getName(),
-                                result.interfaces));
+                preGeneratedProxies.getProxies().put(proxy.managedClassName, new PreGeneratedProxies.ProxyClassDetailsHolder(
+                        proxy.proxyDef.getTypeDescription().getName(), proxy.interfaces));
             }
         }
+
         return preGeneratedProxies;
     }
 
@@ -1435,9 +1456,15 @@ private TypePool createTransformedClassesTypePool(TransformedClassesBuildItem tr
                 }
             }
         }
-        return TypePool.Default.of(new ClassFileLocator.Compound(
+
+        ClassFileLocator classFileLocator = new ClassFileLocator.Compound(
                 new ClassFileLocator.Simple(transformedClasses),
-                ClassFileLocator.ForClassLoader.of(Thread.currentThread().getContextClassLoader())));
+                QuarkusClassFileLocator.INSTANCE);
+
+        // we can reuse the core TypePool but we may not reuse the full enhancer TypePool
+        // or PublicFieldWithProxyAndLazyLoadingAndInheritanceTest will fail
+        return new TypePool.Default(new CacheProvider.Simple(), classFileLocator, ReaderMode.FAST,
+                HibernateEntityEnhancer.CORE_TYPE_POOL);
     }
 
     private boolean isModified(String entity, Set<String> changedClasses, IndexView index) {
@@ -1455,7 +1482,7 @@ private boolean isModified(String entity, Set<String> changedClasses, IndexView
             }
         }
         DotName superName = clazz.superName();
-        if (superName != null) {
+        if (superName != null && !DotName.OBJECT_NAME.equals(superName)) {
             return isModified(superName.toString(), changedClasses, index);
         }
         return false;
@@ -1467,10 +1494,12 @@ private static final class ProxyCache {
     }
 
     static final class CachedProxy {
+        final String managedClassName;
         final DynamicType.Unloaded<?> proxyDef;
         final Set<String> interfaces;
 
-        CachedProxy(DynamicType.Unloaded<?> proxyDef, Set<String> interfaces) {
+        CachedProxy(String managedClassName, DynamicType.Unloaded<?> proxyDef, Set<String> interfaces) {
+            this.managedClassName = managedClassName;
             this.proxyDef = proxyDef;
             this.interfaces = interfaces;
         }

extensions/hibernate-orm/deployment/src/main/java/io/quarkus/hibernate/orm/deployment/ProxyBuildingHelper.java

@@ -6,11 +6,11 @@
 
 import org.hibernate.bytecode.internal.bytebuddy.BytecodeProviderImpl;
 import org.hibernate.proxy.pojo.bytebuddy.ByteBuddyProxyHelper;
+import org.jboss.jandex.ClassInfo;
 
 import net.bytebuddy.ClassFileVersion;
 import net.bytebuddy.description.method.MethodDescription;
 import net.bytebuddy.description.type.TypeDefinition;
-import net.bytebuddy.description.type.TypeDescription;
 import net.bytebuddy.dynamic.DynamicType;
 import net.bytebuddy.matcher.ElementMatcher;
 import net.bytebuddy.matcher.ElementMatchers;
@@ -52,17 +52,12 @@ private ByteBuddyProxyHelper getByteBuddyProxyHelper() {
         return this.byteBuddyProxyHelper;
     }
 
-    public boolean isProxiable(String managedClassOrPackageName) {
-        TypePool.Resolution mappedClassResolution = typePool.describe(managedClassOrPackageName);
-        if (!mappedClassResolution.isResolved()) {
-            // Probably a package name - consider it's not proxiable.
-            return false;
-        }
-
-        TypeDescription mappedClass = mappedClassResolution.resolve();
-
-        return !mappedClass.isFinal()
-                && !mappedClass.getDeclaredMethods().filter(NO_ARG_CONSTRUCTOR).isEmpty();
+    public boolean isProxiable(ClassInfo classInfo) {
+        return classInfo != null
+                && !classInfo.isInterface()
+                && !classInfo.isAbstract()
+                && !classInfo.isFinal()
+                && classInfo.hasNoArgsConstructor();
     }
 
     @Override