HHH-16218 Natural id cache is extremely slow for entities with compound natural id

hibernate-core/src/main/java/org/hibernate/cache/internal/DefaultCacheKeysFactory.java

@@ -69,8 +69,16 @@ public class DefaultCacheKeysFactory implements CacheKeysFactory {
 		}
 	}
 
-	public static Object staticCreateNaturalIdKey(Object naturalIdValues, EntityPersister persister, SharedSessionContractImplementor session) {
+	public static Object staticCreateNaturalIdKey(
-		return new NaturalIdCacheKey( naturalIdValues,  persister, session );
+			Object naturalIdValues,
+			EntityPersister persister,
+			SharedSessionContractImplementor session) {
+		NaturalIdCacheKey.NaturalIdCacheKeyBuilder builder = new NaturalIdCacheKey.NaturalIdCacheKeyBuilder(
+				naturalIdValues,
+				persister,
+				session
+		);
+		return builder.build();
 	}
 
 	public static Object staticGetEntityId(Object cacheKey) {

hibernate-core/src/main/java/org/hibernate/cache/internal/NaturalIdCacheKey.java

@@ -9,9 +9,12 @@ package org.hibernate.cache.internal;
 import java.io.IOException;
 import java.io.ObjectInputStream;
 import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Objects;
 
 import org.hibernate.Internal;
+import org.hibernate.cache.MutableCacheKeyBuilder;
 import org.hibernate.engine.spi.SharedSessionContractImplementor;
 import org.hibernate.internal.util.ValueHolder;
 import org.hibernate.persister.entity.EntityPersister;
@@ -33,12 +36,51 @@ public class NaturalIdCacheKey implements Serializable {
 	// "transient" is important here -- NaturalIdCacheKey needs to be Serializable
 	private transient ValueHolder<String> toString;
 
-	public NaturalIdCacheKey(Object naturalIdValues, EntityPersister persister, SharedSessionContractImplementor session) {
+	public static class NaturalIdCacheKeyBuilder implements MutableCacheKeyBuilder {
+
+		private final String entityName;
+		private final String tenantIdentifier;
+
+		private final List<Object> values;
+		private int hashCode;
+
+		public NaturalIdCacheKeyBuilder(
+				Object naturalIdValues,
+				EntityPersister persister,
+				String entityName,
+				SharedSessionContractImplementor session) {
+			this.entityName = entityName;
+			this.tenantIdentifier = session.getTenantIdentifier();
+			values = new ArrayList<>();
+			persister.getNaturalIdMapping().addToCacheKey( this, naturalIdValues, session );
+		}
+
+		public NaturalIdCacheKeyBuilder(
+				Object naturalIdValues,
+				EntityPersister persister,
+				SharedSessionContractImplementor session) {
 			this( naturalIdValues, persister, persister.getRootEntityName(), session );
 		}
 
-	public NaturalIdCacheKey(Object naturalIdValues, EntityPersister persister, String entityName, SharedSessionContractImplementor session) {
+		@Override
-		this( persister.getNaturalIdMapping().disassemble( naturalIdValues, session ), entityName, session.getTenantIdentifier(), persister.getNaturalIdMapping().calculateHashCode( naturalIdValues, session ) );
+		public void addValue(Object value) {
+			values.add( value );
+		}
+
+		@Override
+		public void addHashCode(int hashCode) {
+			this.hashCode = 37 * this.hashCode + hashCode;
+		}
+
+		@Override
+		public NaturalIdCacheKey build() {
+			return new NaturalIdCacheKey(
+					values.toArray( new Object[0] ),
+					entityName,
+					tenantIdentifier,
+					hashCode
+			);
+		}
 	}
 
 	@Internal

hibernate-core/src/main/java/org/hibernate/cache/internal/SimpleCacheKeysFactory.java

@@ -32,9 +32,18 @@ public class SimpleCacheKeysFactory implements CacheKeysFactory {
 	}
 
 	@Override
-	public Object createNaturalIdKey(Object naturalIdValues, EntityPersister persister, SharedSessionContractImplementor session) {
+	public Object createNaturalIdKey(
+			Object naturalIdValues,
+			EntityPersister persister,
+			SharedSessionContractImplementor session) {
 		// natural ids always need to be wrapped
-		return new NaturalIdCacheKey(naturalIdValues, persister, null, session);
+		NaturalIdCacheKey.NaturalIdCacheKeyBuilder builder = new NaturalIdCacheKey.NaturalIdCacheKeyBuilder(
+				naturalIdValues,
+				persister,
+				null,
+				session
+		);
+		return builder.build();
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/engine/internal/NaturalIdResolutionsImpl.java

@@ -770,7 +770,7 @@ public class NaturalIdResolutionsImpl implements NaturalIdResolutions, Serializa
 			final int prime = 31;
 			int hashCodeCalculation = 1;
 			hashCodeCalculation = prime * hashCodeCalculation + entityDescriptor.hashCode();
-			hashCodeCalculation = prime * hashCodeCalculation + entityDescriptor.getNaturalIdMapping().calculateHashCode( naturalIdValue, persistenceContext.getSession() );
+			hashCodeCalculation = prime * hashCodeCalculation + entityDescriptor.getNaturalIdMapping().calculateHashCode( naturalIdValue );
 
 			this.hashCode = hashCodeCalculation;
 		}

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NaturalIdMapping.java

@@ -112,7 +112,7 @@ public interface NaturalIdMapping extends VirtualModelPart {
 	 * @param value The natural-id value
 	 * @return The hash-code
 	 */
-	int calculateHashCode(Object value, SharedSessionContractImplementor session);
+	int calculateHashCode(Object value);
 
 	/**
 	 * Make a loader capable of loading a single entity by natural-id

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/CompoundNaturalIdMapping.java

@@ -172,9 +172,19 @@ public class CompoundNaturalIdMapping extends AbstractNaturalIdMapping implement
 	}
 
 	@Override
-	public int calculateHashCode(Object value, SharedSessionContractImplementor session) {
+	public int calculateHashCode(Object value) {
-		Object o = disassemble( value, session ) ;
+		if ( value == null ) {
-		return Arrays.hashCode((Object[]) o);
+			return 0;
+		}
+		Object[] values = (Object[]) value;
+		int hashcode = 0;
+		for ( int i = 0; i < attributes.size(); i++ ) {
+			final Object o = values[i];
+			if ( o != null ) {
+				hashcode = 27 * hashcode + ( (JavaType) attributes.get( i ).getExpressibleJavaType() ).extractHashCode( o );
+			}
+		}
+		return hashcode;
 	}
 
 	@Override

hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/SimpleNaturalIdMapping.java

@@ -136,7 +136,7 @@ public class SimpleNaturalIdMapping extends AbstractNaturalIdMapping implements
 	}
 
 	@Override
-	public int calculateHashCode(Object value, SharedSessionContractImplementor session) {
+	public int calculateHashCode(Object value) {
 		//noinspection unchecked
 		return value == null ? 0 : ( (JavaType<Object>) getJavaType() ).extractHashCode( value );
 	}

hibernate-core/src/test/java/org/hibernate/orm/test/caching/mocked/NaturalIdCacheKeyTest.java

@@ -46,7 +46,7 @@ public class NaturalIdCacheKeyTest {
         when( entityPersister.getRootEntityName() ).thenReturn( "EntityName" );
         when( entityPersister.getNaturalIdMapping() ).thenReturn( naturalIdMapping );
         when( naturalIdMapping.disassemble( any(), eq( sessionImplementor ) ) ).thenAnswer( invocation -> invocation.getArguments()[0] );
-        when( naturalIdMapping.calculateHashCode( any(), eq( sessionImplementor ) ) ).thenAnswer( invocation -> invocation.getArguments()[0].hashCode() );
+        when( naturalIdMapping.calculateHashCode( any() ) ).thenAnswer( invocation -> invocation.getArguments()[0].hashCode() );
 
 
         final NaturalIdCacheKey key = (NaturalIdCacheKey) DefaultCacheKeysFactory.staticCreateNaturalIdKey( new Object[] {"a", "b", "c"}, entityPersister, sessionImplementor );

hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/compound/CompoundNaturalIdCacheTest.java

@@ -6,120 +6,102 @@
  */
 package org.hibernate.orm.test.mapping.naturalid.compound;
 
+import org.hibernate.annotations.NaturalId;
+import org.hibernate.annotations.NaturalIdCache;
+import org.hibernate.cfg.AvailableSettings;
+import org.hibernate.engine.internal.NaturalIdResolutionsImpl;
+import org.hibernate.stat.NaturalIdStatistics;
+import org.hibernate.stat.spi.StatisticsImplementor;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.ServiceRegistry;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.hibernate.testing.orm.junit.Setting;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
 import jakarta.persistence.Entity;
 import jakarta.persistence.GeneratedValue;
 import jakarta.persistence.Id;
-import org.hibernate.Session;
-import org.hibernate.Transaction;
-import org.hibernate.annotations.NaturalId;
-import org.hibernate.cfg.AvailableSettings;
-import org.hibernate.cfg.Configuration;
-import org.hibernate.query.criteria.HibernateCriteriaBuilder;
-import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
-import org.hibernate.testing.TestForIssue;
-import org.junit.Test;
-import org.junit.jupiter.api.Timeout;
 
-import java.util.function.Function;
+import static org.assertj.core.api.AssertionsForClassTypes.assertThat;
-
+import static org.hibernate.testing.cache.CachingRegionFactory.DEFAULT_ACCESSTYPE;
-import static org.junit.jupiter.api.Assertions.assertTrue;
 
 /**
  * @author Sylvain Dusart
  */
 @TestForIssue(jiraKey = "HHH-16218")
-public class CompoundNaturalIdCacheTest extends BaseCoreFunctionalTestCase {
+@DomainModel(
-
+		annotatedClasses = {
-  @Override
+				CompoundNaturalIdCacheTest.EntityWithSimpleNaturalId.class,
-  protected Class[] getAnnotatedClasses() {
+				CompoundNaturalIdCacheTest.EntityWithCompoundNaturalId.class
-    return new Class[]{
-        EntityWithSimpleNaturalId.class,
-        EntityWithCompoundNaturalId.class
-    };
 		}
+)
+@ServiceRegistry(
+		settings = {
+				@Setting(name = DEFAULT_ACCESSTYPE, value = "nonstrict-read-write"),
+				@Setting(name = AvailableSettings.USE_QUERY_CACHE, value = "true"),
+				@Setting(name = AvailableSettings.SHOW_SQL, value = "false"),
+		}
+)
+@SessionFactory(generateStatistics = true)
+public class CompoundNaturalIdCacheTest {
 
-  @Override
+	private static final int OBJECT_NUMBER = 3;
-  protected void configure(Configuration configuration) {
-    super.configure(configuration);
 
-    configuration.setProperty(AvailableSettings.STATEMENT_BATCH_SIZE, "5000");
+	@BeforeAll
-    configuration.setProperty( AvailableSettings.SHOW_SQL, Boolean.FALSE.toString() );
+	public void setUp(SessionFactoryScope scope) {
-    configuration.setProperty(AvailableSettings.GENERATE_STATISTICS, "false");
+		scope.inTransaction(
+				session -> {
+					for ( int i = 0; i < OBJECT_NUMBER; i++ ) {
+						EntityWithCompoundNaturalId compoundNaturalIdEntity = new EntityWithCompoundNaturalId();
+						final String str = String.valueOf( i );
+						compoundNaturalIdEntity.setFirstname( str );
+						compoundNaturalIdEntity.setLastname( str );
+						session.persist( compoundNaturalIdEntity );
+
+						EntityWithSimpleNaturalId withSimpleNaturalIdEntity = new EntityWithSimpleNaturalId();
+						withSimpleNaturalIdEntity.setName( str );
+						session.persist( withSimpleNaturalIdEntity );
+						NaturalIdResolutionsImpl naturalIdResolutions = (NaturalIdResolutionsImpl) session.getPersistenceContext()
+								.getNaturalIdResolutions();
+					}
+				}
+		);
 	}
 
 	@Test
-  @Timeout(30)
+	public void createThenLoadTest(SessionFactoryScope scope) {
-  public void createThenLoadTest() {
+		StatisticsImplementor statistics = scope.getSessionFactory().getStatistics();
-    Session s = openSession();
+		statistics.clear();
-    Transaction tx = s.beginTransaction();
+		NaturalIdStatistics naturalIdStatistics = statistics.getNaturalIdStatistics( EntityWithCompoundNaturalId.class.getName() );
 
-    int objectsNb = 20000;
+		loadEntityWithCompoundNaturalId( "0", "0", scope );
+		assertThat( naturalIdStatistics.getCacheHitCount() ).isEqualTo( 1 );
 
-    log.info("Starting creations");
+		loadEntityWithCompoundNaturalId( "1", "1", scope );
+		assertThat( naturalIdStatistics.getCacheHitCount() ).isEqualTo( 2 );
 
-    var creationDurationForCompoundNaturalId = createEntities(i -> {
+		loadEntityWithCompoundNaturalId( "2", "2", scope );
-      var entity = new EntityWithCompoundNaturalId();
+		assertThat( naturalIdStatistics.getCacheHitCount() ).isEqualTo( 3 );
-      final var str = String.valueOf(i);
-      entity.setFirstname(str);
-      entity.setLastname(str);
-      return entity;
-    }, objectsNb);
 
-    log.info("Persisted " + objectsNb + ' ' + EntityWithCompoundNaturalId.class.getSimpleName() +
-               " objects, duration=" + creationDurationForCompoundNaturalId + "ms");
-
-    var creationDurationForSimpleNaturalId = createEntities(i -> {
-      var entity = new EntityWithSimpleNaturalId();
-      entity.setName(String.valueOf(i));
-      return entity;
-    }, objectsNb);
-
-    log.info("Persisted " + objectsNb + ' ' + EntityWithSimpleNaturalId.class.getSimpleName() +
-            " objects, duration=" + creationDurationForSimpleNaturalId + "ms");
-
-    tx.commit();
-    s.close();
-
-    int maxResults = 20000;
-    var loadDurationForCompoundNaturalId = loadEntities(EntityWithCompoundNaturalId.class, maxResults);
-    var loadDurationForSimpleNaturalId = loadEntities(EntityWithSimpleNaturalId.class, maxResults);
-
-    s.close();
-
-    assertTrue(loadDurationForCompoundNaturalId <= 5 * loadDurationForSimpleNaturalId,
-        "it should not be soo long to load entities with compound naturalId");
 	}
 
-  private long createEntities(final Function<Integer, Object> creator, final int objectsNb) {
+	private void loadEntityWithCompoundNaturalId(String firstname, String lastname, SessionFactoryScope scope) {
-    var start = System.currentTimeMillis();
+		scope.inSession(
-
+				session -> {
-    for (int i = 0; i < objectsNb; i++) {
+					session.byNaturalId( EntityWithCompoundNaturalId.class )
-      session.persist(creator.apply(i));
+							.using( "firstname", firstname )
+							.using( "lastname", lastname )
+							.load();
+				}
+		);
 	}
 
-    return System.currentTimeMillis() - start;
+	@Entity(name = "EntityWithSimpleNaturalId")
-  }
+	@NaturalIdCache
-
-  private long loadEntities(final Class<?> clazz, final int maxResults) {
-    var s = openSession();
-
-    var start = System.currentTimeMillis();
-    log.info("Loading at most " + maxResults + "  instances of " + clazz);
-
-    final HibernateCriteriaBuilder cb = s.getCriteriaBuilder();
-    final var query = cb.createQuery(clazz);
-    query.from(clazz);
-    var objects = s.createQuery(query).setMaxResults(maxResults).list();
-
-    var duration = System.currentTimeMillis() - start;
-    log.info("Loaded " + objects.size() + " instances of " + clazz + ", duration=" + duration + "ms");
-
-    s.close();
-
-    return duration;
-  }
-
-  @Entity
 	public static class EntityWithSimpleNaturalId {
 
 		@Id
@@ -146,7 +128,8 @@ public class CompoundNaturalIdCacheTest extends BaseCoreFunctionalTestCase {
 		}
 	}
 
-  @Entity
+	@Entity(name = "EntityWithCompoundNaturalId")
+	@NaturalIdCache
 	public static class EntityWithCompoundNaturalId {
 
 		@Id

hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/compound/CompoundNaturalIdTest.java

@@ -0,0 +1,161 @@
+/*
+ * 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.orm.test.mapping.naturalid.compound;
+
+import org.hibernate.annotations.NaturalId;
+import org.hibernate.cfg.AvailableSettings;
+import org.hibernate.engine.spi.NaturalIdResolutions;
+import org.hibernate.query.criteria.HibernateCriteriaBuilder;
+import org.hibernate.query.criteria.JpaCriteriaQuery;
+import org.hibernate.query.criteria.JpaRoot;
+
+import org.hibernate.testing.TestForIssue;
+import org.hibernate.testing.orm.junit.DomainModel;
+import org.hibernate.testing.orm.junit.ServiceRegistry;
+import org.hibernate.testing.orm.junit.SessionFactory;
+import org.hibernate.testing.orm.junit.SessionFactoryScope;
+import org.hibernate.testing.orm.junit.Setting;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.GeneratedValue;
+import jakarta.persistence.Id;
+
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * @author Sylvain Dusart
+ */
+@TestForIssue(jiraKey = "HHH-16218")
+@DomainModel(
+		annotatedClasses = {
+				CompoundNaturalIdTest.EntityWithSimpleNaturalId.class,
+				CompoundNaturalIdTest.EntityWithCompoundNaturalId.class
+		}
+)
+@ServiceRegistry(
+		settings = {
+				@Setting(name = AvailableSettings.STATEMENT_BATCH_SIZE, value = "500"),
+				@Setting(name = AvailableSettings.SHOW_SQL, value = "false"),
+		}
+)
+@SessionFactory
+public class CompoundNaturalIdTest {
+
+	private static final int OBJECT_NUMBER = 2000;
+	private static final int MAX_RESULTS = 2000;
+
+	@BeforeAll
+	public void setUp(SessionFactoryScope scope) {
+		scope.inTransaction(
+				session -> {
+					for ( int i = 0; i < OBJECT_NUMBER; i++ ) {
+						EntityWithCompoundNaturalId compoundNaturalIdEntity = new EntityWithCompoundNaturalId();
+						final String str = String.valueOf( i );
+						compoundNaturalIdEntity.setFirstname( str );
+						compoundNaturalIdEntity.setLastname( str );
+						session.persist( compoundNaturalIdEntity );
+
+						EntityWithSimpleNaturalId withSimpleNaturalIdEntity = new EntityWithSimpleNaturalId();
+						withSimpleNaturalIdEntity.setName( str );
+						session.persist( withSimpleNaturalIdEntity );
+					}
+				}
+		);
+	}
+
+	@Test
+	public void createThenLoadTest(SessionFactoryScope scope) {
+		long loadDurationForCompoundNaturalId = loadEntities( EntityWithCompoundNaturalId.class, MAX_RESULTS, scope );
+		long loadDurationForSimpleNaturalId = loadEntities( EntityWithSimpleNaturalId.class, MAX_RESULTS, scope );
+
+		assertTrue(
+				loadDurationForCompoundNaturalId <= 5 * loadDurationForSimpleNaturalId,
+				"it should not be soo long to load entities with compound naturalId"
+		);
+	}
+
+	private long loadEntities(final Class<?> clazz, final int maxResults, SessionFactoryScope scope) {
+		long start = System.currentTimeMillis();
+		scope.inSession(
+				session -> {
+					final HibernateCriteriaBuilder cb = session.getCriteriaBuilder();
+					JpaCriteriaQuery<?> query = cb.createQuery( clazz );
+					query.from( clazz );
+					session.createQuery( query ).setMaxResults( maxResults ).list();
+				}
+		);
+		long duration = System.currentTimeMillis() - start;
+		return duration;
+	}
+
+	@Entity(name = "EntityWithSimpleNaturalId")
+	public static class EntityWithSimpleNaturalId {
+
+		@Id
+		@GeneratedValue
+		private Long id;
+
+		@NaturalId
+		private String name;
+
+		public Long getId() {
+			return id;
+		}
+
+		public void setId(final Long id) {
+			this.id = id;
+		}
+
+		public String getName() {
+			return name;
+		}
+
+		public void setName(final String name) {
+			this.name = name;
+		}
+	}
+
+	@Entity(name = "EntityWithCompoundNaturalId")
+	public static class EntityWithCompoundNaturalId {
+
+		@Id
+		@GeneratedValue
+		private Long id;
+
+		@NaturalId
+		private String firstname;
+
+		@NaturalId
+		private String lastname;
+
+		public Long getId() {
+			return id;
+		}
+
+		public void setId(final Long id) {
+			this.id = id;
+		}
+
+		public String getFirstname() {
+			return firstname;
+		}
+
+		public void setFirstname(final String firstname) {
+			this.firstname = firstname;
+		}
+
+		public String getLastname() {
+			return lastname;
+		}
+
+		public void setLastname(final String lastname) {
+			this.lastname = lastname;
+		}
+	}
+}