HHH-15566 Improve efficiency of CallbackRegistryImpl

hibernate-core/src/main/java/org/hibernate/event/service/internal/EventListenerRegistryImpl.java

@@ -43,7 +43,7 @@ import org.hibernate.event.service.spi.EventListenerGroup;
 import org.hibernate.event.service.spi.EventListenerRegistrationException;
 import org.hibernate.event.service.spi.EventListenerRegistry;
 import org.hibernate.event.spi.EventType;
-import org.hibernate.jpa.event.internal.CallbackRegistryImplementor;
+import org.hibernate.jpa.event.spi.CallbackRegistry;
 
 import static org.hibernate.event.spi.EventType.AUTO_FLUSH;
 import static org.hibernate.event.spi.EventType.CLEAR;
@@ -201,14 +201,14 @@ public class EventListenerRegistryImpl implements EventListenerRegistry {
 	// Builder
 
 	public static class Builder {
-		private final CallbackRegistryImplementor callbackRegistry;
+		private final CallbackRegistry callbackRegistry;
 		private final boolean jpaBootstrap;
 
 		private final Map<EventType<?>,EventListenerGroup<?>> listenerGroupMap = new TreeMap<>(
 				Comparator.comparing( EventType::ordinal )
 		);
 
-		public Builder(CallbackRegistryImplementor callbackRegistry, boolean jpaBootstrap) {
+		public Builder(CallbackRegistry callbackRegistry, boolean jpaBootstrap) {
 			this.callbackRegistry = callbackRegistry;
 			this.jpaBootstrap = jpaBootstrap;
 

hibernate-core/src/main/java/org/hibernate/event/spi/EventEngine.java

@@ -20,8 +20,8 @@ import org.hibernate.event.service.internal.EventListenerRegistryImpl;
 import org.hibernate.event.service.spi.EventListenerGroup;
 import org.hibernate.event.service.spi.EventListenerRegistry;
 import org.hibernate.internal.util.collections.CollectionHelper;
-import org.hibernate.jpa.event.internal.CallbackRegistryImplementor;
 import org.hibernate.jpa.event.internal.CallbacksFactory;
+import org.hibernate.jpa.event.spi.CallbackRegistry;
 import org.hibernate.service.spi.Stoppable;
 
 /**
@@ -34,7 +34,7 @@ public class EventEngine {
 	private final Map<String,EventType<?>> registeredEventTypes;
 	private final EventListenerRegistry listenerRegistry;
 
-	private final CallbackRegistryImplementor callbackRegistry;
+	private final CallbackRegistry callbackRegistry;
 
 	public EventEngine(
 			MetadataImplementor mappings,
@@ -151,7 +151,7 @@ public class EventEngine {
 		return listenerRegistry;
 	}
 
-	public CallbackRegistryImplementor getCallbackRegistry() {
+	public CallbackRegistry getCallbackRegistry() {
 		return callbackRegistry;
 	}
 

hibernate-core/src/main/java/org/hibernate/internal/util/collections/MapBackedClassValue.java

@@ -0,0 +1,66 @@
+/*
+ * Hibernate, Relational Persistence for Idiomatic Java
+ *
+ * License: GNU Lesser General Public License (LGPL), version 2.1 or later
+ * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
+ */
+package org.hibernate.internal.util.collections;
+
+import java.util.Map;
+
+/**
+ * For efficient lookup based on Class types as key,
+ * a ClassValue should be used; however it requires
+ * lazy association of values; this helper wraps
+ * a plain HashMap but optimises lookups via the ClassValue.
+ * N.B. there is a cost in memory and in terms of weak references,
+ * so let's use this only where proven that a simple Map lookup
+ * is otherwise too costly.
+ * @param <V> the type of the values stored in the Maps.
+ * @author Sanne Grinovero
+ * @since 6.2
+ */
+public final class MapBackedClassValue<V> {
+
+	private volatile Map<Class<?>, V> map;
+
+	private final ClassValue<V> classValue = new ClassValue<>() {
+		@Override
+		protected V computeValue(final Class<?> type) {
+			final Map<Class<?>, V> m = map;
+			if ( m == null ) {
+				throw new IllegalStateException( "This MapBackedClassValue has been disposed" );
+			}
+			else {
+				return map.get( type );
+			}
+		}
+	};
+
+	public MapBackedClassValue(final Map<Class<?>, V> map) {
+		//Defensive copy, and implicit null check.
+		//Choose the Map.copyOf implementation as it has a compact layout;
+		//it doesn't have great get() performance but it's acceptable since we're performing that at most
+		//once per key before caching it via the ClassValue.
+		this.map = Map.copyOf( map );
+	}
+
+	public V get(Class<?> key) {
+		return classValue.get( key );
+	}
+
+	/**
+	 * Use this to wipe the backing map, but N.B.
+	 * we won't be clearing the ClassValue: this is useful
+	 * only to avoid classloader leaks since the Map
+	 * may hold references to user classes.
+	 * Since ClassValue is also possibly caching state,
+	 * it might be possible to retrieve some values after this
+	 * but shouldn't be relied on.
+	 * ClassValue doesn't leak references to classes.
+	 */
+	public void dispose() {
+		this.map = null;
+	}
+
+}

hibernate-core/src/main/java/org/hibernate/jpa/event/internal/CallbackRegistryImpl.java

@@ -7,10 +7,14 @@
 package org.hibernate.jpa.event.internal;
 
 import java.util.HashMap;
+import java.util.Map;
+
 import jakarta.persistence.PersistenceException;
 
 import org.hibernate.internal.util.collections.ArrayHelper;
+import org.hibernate.internal.util.collections.MapBackedClassValue;
 import org.hibernate.jpa.event.spi.Callback;
+import org.hibernate.jpa.event.spi.CallbackRegistry;
 import org.hibernate.jpa.event.spi.CallbackType;
 
 /**
@@ -18,37 +22,39 @@ import org.hibernate.jpa.event.spi.CallbackType;
  *
  * @author <a href="mailto:kabir.khan@jboss.org">Kabir Khan</a>
  * @author Steve Ebersole
+ * @author Sanne Grinovero
  */
-final class CallbackRegistryImpl implements CallbackRegistryImplementor {
+final class CallbackRegistryImpl implements CallbackRegistry {
-	private final HashMap<Class<?>, Callback[]> preCreates = new HashMap<>();
-	private final HashMap<Class<?>, Callback[]> postCreates = new HashMap<>();
-	private final HashMap<Class<?>, Callback[]> preRemoves = new HashMap<>();
-	private final HashMap<Class<?>, Callback[]> postRemoves = new HashMap<>();
-	private final HashMap<Class<?>, Callback[]> preUpdates = new HashMap<>();
-	private final HashMap<Class<?>, Callback[]> postUpdates = new HashMap<>();
-	private final HashMap<Class<?>, Callback[]> postLoads = new HashMap<>();
 
-	@Override
+	private final MapBackedClassValue<Callback[]> preCreates;
-	public boolean hasRegisteredCallbacks(Class<?> entityClass, CallbackType callbackType) {
+	private final MapBackedClassValue<Callback[]> postCreates;
-		final HashMap<Class<?>, Callback[]> map = determineAppropriateCallbackMap( callbackType );
+	private final MapBackedClassValue<Callback[]> preRemoves;
-		return notEmpty( map.get( entityClass ) );
+	private final MapBackedClassValue<Callback[]> postRemoves;
+	private final MapBackedClassValue<Callback[]> preUpdates;
+	private final MapBackedClassValue<Callback[]> postUpdates;
+	private final MapBackedClassValue<Callback[]> postLoads;
+
+	public CallbackRegistryImpl(
+			Map<Class<?>, Callback[]> preCreates,
+			Map<Class<?>, Callback[]> postCreates,
+			Map<Class<?>, Callback[]> preRemoves,
+			Map<Class<?>, Callback[]> postRemoves,
+			Map<Class<?>, Callback[]> preUpdates,
+			Map<Class<?>, Callback[]> postUpdates,
+			Map<Class<?>, Callback[]> postLoads) {
+		this.preCreates = new MapBackedClassValue<>( preCreates );
+		this.postCreates = new MapBackedClassValue<>( postCreates );
+		this.preRemoves = new MapBackedClassValue<>( preRemoves );
+		this.postRemoves = new MapBackedClassValue<>( postRemoves );
+		this.preUpdates = new MapBackedClassValue<>( preUpdates );
+		this.postUpdates = new MapBackedClassValue<>( postUpdates );
+		this.postLoads = new MapBackedClassValue<>( postLoads );
 	}
 
 	@Override
-	public void registerCallbacks(Class<?> entityClass, Callback[] callbacks) {
+	public boolean hasRegisteredCallbacks(Class<?> entityClass, CallbackType callbackType) {
-		if ( callbacks == null || callbacks.length == 0 ) {
+		final MapBackedClassValue<Callback[]> map = determineAppropriateCallbackMap( callbackType );
-			return;
+		return notEmpty( map.get( entityClass ) );
-		}
-
-		for ( Callback callback : callbacks ) {
-			final HashMap<Class<?>, Callback[]> map = determineAppropriateCallbackMap( callback.getCallbackType() );
-			Callback[] entityCallbacks = map.get( entityClass );
-			if ( entityCallbacks == null ) {
-				entityCallbacks = new Callback[0];
-			}
-			entityCallbacks = ArrayHelper.join( entityCallbacks, callback );
-			map.put( entityClass, entityCallbacks );
-		}
 	}
 
 	@Override
@@ -90,6 +96,17 @@ final class CallbackRegistryImpl implements CallbackRegistryImplementor {
 		return callback( postLoads.get( bean.getClass() ), bean );
 	}
 
+	@Override
+	public void release() {
+		this.preCreates.dispose();
+		this.postCreates.dispose();
+		this.preRemoves.dispose();
+		this.postRemoves.dispose();
+		this.preUpdates.dispose();
+		this.postUpdates.dispose();
+		this.postLoads.dispose();
+	}
+
 	private boolean callback(Callback[] callbacks, Object bean) {
 		if ( callbacks != null && callbacks.length != 0 ) {
 			for ( Callback callback : callbacks ) {
@@ -102,7 +119,7 @@ final class CallbackRegistryImpl implements CallbackRegistryImplementor {
 		}
 	}
 
-	private HashMap<Class<?>, Callback[]> determineAppropriateCallbackMap(CallbackType callbackType) {
+	private MapBackedClassValue<Callback[]> determineAppropriateCallbackMap(CallbackType callbackType) {
 		if ( callbackType == CallbackType.PRE_PERSIST ) {
 			return preCreates;
 		}
@@ -134,16 +151,66 @@ final class CallbackRegistryImpl implements CallbackRegistryImplementor {
 		throw new PersistenceException( "Unrecognized JPA callback type [" + callbackType + "]" );
 	}
 
-	public void release() {
+	public static class Builder {
-		preCreates.clear();
+		private final Map<Class<?>, Callback[]> preCreates = new HashMap<>();
-		postCreates.clear();
+		private final Map<Class<?>, Callback[]> postCreates = new HashMap<>();
+		private final Map<Class<?>, Callback[]> preRemoves = new HashMap<>();
+		private final Map<Class<?>, Callback[]> postRemoves = new HashMap<>();
+		private final Map<Class<?>, Callback[]> preUpdates = new HashMap<>();
+		private final Map<Class<?>, Callback[]> postUpdates = new HashMap<>();
+		private final Map<Class<?>, Callback[]> postLoads = new HashMap<>();
 
-		preRemoves.clear();
+		public void registerCallbacks(Class<?> entityClass, Callback[] callbacks) {
-		postRemoves.clear();
+			if ( callbacks == null || callbacks.length == 0 ) {
+				return;
+			}
 
-		preUpdates.clear();
+			for ( Callback callback : callbacks ) {
-		postUpdates.clear();
+				final Map<Class<?>, Callback[]> map = determineAppropriateCallbackMap( callback.getCallbackType() );
+				Callback[] entityCallbacks = map.get( entityClass );
+				if ( entityCallbacks == null ) {
+					entityCallbacks = new Callback[0];
+				}
+				entityCallbacks = ArrayHelper.join( entityCallbacks, callback );
+				map.put( entityClass, entityCallbacks );
+			}
+		}
+
+		private Map<Class<?>, Callback[]> determineAppropriateCallbackMap(CallbackType callbackType) {
+			if ( callbackType == CallbackType.PRE_PERSIST ) {
+				return preCreates;
+			}
+
+			if ( callbackType == CallbackType.POST_PERSIST ) {
+				return postCreates;
+			}
+
+			if ( callbackType == CallbackType.PRE_REMOVE ) {
+				return preRemoves;
+			}
+
+			if ( callbackType == CallbackType.POST_REMOVE ) {
+				return postRemoves;
+			}
+
+			if ( callbackType == CallbackType.PRE_UPDATE ) {
+				return preUpdates;
+			}
+
+			if ( callbackType == CallbackType.POST_UPDATE ) {
+				return postUpdates;
+			}
+
+			if ( callbackType == CallbackType.POST_LOAD ) {
+				return postLoads;
+			}
+
+			throw new PersistenceException( "Unrecognized JPA callback type [" + callbackType + "]" );
+		}
+
+		protected CallbackRegistryImpl build() {
+			return new CallbackRegistryImpl( preCreates, postCreates, preRemoves, postRemoves, preUpdates, postUpdates, postLoads );
+		}
 
-		postLoads.clear();
 	}
 }

hibernate-core/src/main/java/org/hibernate/jpa/event/internal/CallbackRegistryImplementor.java

@@ -1,15 +0,0 @@
-/*
- * Hibernate, Relational Persistence for Idiomatic Java
- *
- * License: GNU Lesser General Public License (LGPL), version 2.1 or later.
- * See the lgpl.txt file in the root directory or <http://www.gnu.org/licenses/lgpl-2.1.html>.
- */
-package org.hibernate.jpa.event.internal;
-
-import org.hibernate.jpa.event.spi.CallbackRegistrar;
-
-public interface CallbackRegistryImplementor extends CallbackRegistrar {
-
-	void release();
-
-}

hibernate-core/src/main/java/org/hibernate/jpa/event/internal/CallbacksFactory.java

@@ -9,13 +9,13 @@ package org.hibernate.jpa.event.internal;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashSet;
-import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
 
 import org.hibernate.boot.spi.SessionFactoryOptions;
 import org.hibernate.jpa.event.spi.Callback;
 import org.hibernate.jpa.event.spi.CallbackDefinition;
+import org.hibernate.jpa.event.spi.CallbackRegistry;
 import org.hibernate.mapping.PersistentClass;
 import org.hibernate.mapping.Property;
 import org.hibernate.resource.beans.spi.ManagedBeanRegistry;
@@ -31,13 +31,12 @@ import org.jboss.logging.Logger;
 public final class CallbacksFactory {
 	private static final Logger log = Logger.getLogger( CallbacksFactory.class );
 
-	public static CallbackRegistryImplementor buildCallbackRegistry(SessionFactoryOptions options,
+	public static CallbackRegistry buildCallbackRegistry(SessionFactoryOptions options, ServiceRegistry serviceRegistry, Collection<PersistentClass> entityBindings) {
-			ServiceRegistry serviceRegistry, Collection<PersistentClass> entityBindings) {
 		if ( !jpaCallBacksEnabled( options ) ) {
 			return new EmptyCallbackRegistryImpl();
 		}
 		ManagedBeanRegistry beanRegistry = serviceRegistry.getService( ManagedBeanRegistry.class );
-		CallbackRegistryImplementor registry = new CallbackRegistryImpl();
+		CallbackRegistryImpl.Builder registryBuilder = new CallbackRegistryImpl.Builder();
 		Set<Class<?>> entityClasses = new HashSet<>();
 
 		for ( PersistentClass persistentClass : entityBindings ) {
@@ -63,16 +62,16 @@ public final class CallbacksFactory {
 				continue;
 			}
 
-			registry.registerCallbacks( persistentClass.getMappedClass(),
+			registryBuilder.registerCallbacks( persistentClass.getMappedClass(),
 					buildCallbacks( persistentClass.getCallbackDefinitions(), beanRegistry ) );
 
 			for ( Property property : persistentClass.getDeclaredProperties() ) {
-				registry.registerCallbacks( persistentClass.getMappedClass(),
+				registryBuilder.registerCallbacks( persistentClass.getMappedClass(),
 						buildCallbacks( property.getCallbackDefinitions(), beanRegistry ) );
 			}
 		}
 
-		return registry;
+		return registryBuilder.build();
 	}
 
 	private static Callback[] buildCallbacks(List<CallbackDefinition> callbackDefinitions,

hibernate-core/src/main/java/org/hibernate/jpa/event/internal/EmptyCallbackRegistryImpl.java

@@ -6,10 +6,10 @@
  */
 package org.hibernate.jpa.event.internal;
 
-import org.hibernate.jpa.event.spi.Callback;
+import org.hibernate.jpa.event.spi.CallbackRegistry;
 import org.hibernate.jpa.event.spi.CallbackType;
 
-final class EmptyCallbackRegistryImpl implements CallbackRegistryImplementor {
+final class EmptyCallbackRegistryImpl implements CallbackRegistry {
 
 	@Override
 	public boolean hasRegisteredCallbacks(final Class<?> entityClass, final CallbackType callbackType) {
@@ -56,9 +56,4 @@ final class EmptyCallbackRegistryImpl implements CallbackRegistryImplementor {
 		//no-op
 	}
 
-	@Override
-	public void registerCallbacks(Class<?> entityClass, Callback[] callbacks) {
-		//no-op
-	}
-
 }

hibernate-core/src/main/java/org/hibernate/jpa/event/spi/CallbackRegistrar.java

@@ -1,19 +0,0 @@
-/*
- * Hibernate, Relational Persistence for Idiomatic Java
- *
- * License: GNU Lesser General Public License (LGPL), version 2.1 or later
- * See the lgpl.txt file in the root directory or http://www.gnu.org/licenses/lgpl-2.1.html
- */
-package org.hibernate.jpa.event.spi;
-
-public interface CallbackRegistrar extends CallbackRegistry {
-
-	/**
-	 * Register the callback against the given entity.
-	 *
-	 * @param entityClass The entity Class to register the Callbacks against
-	 * @param callbacks The Callbacks to register against the given entity Class
-	 */
-	void registerCallbacks(Class<?> entityClass, Callback[] callbacks);
-
-}

hibernate-core/src/main/java/org/hibernate/jpa/event/spi/CallbackRegistry.java

@@ -6,14 +6,12 @@
  */
 package org.hibernate.jpa.event.spi;
 
-import java.io.Serializable;
-
 /**
  * Registry of Callbacks by entity and type
  *
  * @author Steve Ebersole
  */
-public interface CallbackRegistry extends Serializable {
+public interface CallbackRegistry {
 	/**
 	 * Do we have any registered callbacks of the given type for the given entity?
 	 *
@@ -35,4 +33,12 @@ public interface CallbackRegistry extends Serializable {
 	void postRemove(Object entity);
 
 	boolean postLoad(Object entity);
+
+	/**
+	 * Signals that the CallbackRegistry will no longer be used.
+	 * In particular it is important to release references to class types
+	 * to avoid classloader leaks.
+	 */
+	void release();
+
 }