From 1826da69a0e6b1c39f44b047840ef42ce77b976d Mon Sep 17 00:00:00 2001 From: Sylvain Dusart Date: Wed, 22 Feb 2023 17:45:05 +0100 Subject: [PATCH] HHH-16218 Natural id cache is extremely slow for entities with compound natural id --- .../cache/internal/NaturalIdCacheKey.java | 2 +- .../internal/NaturalIdResolutionsImpl.java | 2 +- .../metamodel/mapping/NaturalIdMapping.java | 2 +- .../internal/CompoundNaturalIdMapping.java | 5 +- .../internal/SimpleNaturalIdMapping.java | 2 +- .../caching/mocked/NaturalIdCacheKeyTest.java | 2 +- .../compound/CompoundNaturalIdCacheTest.java | 186 ++++++++++++++++++ 7 files changed, 194 insertions(+), 7 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/compound/CompoundNaturalIdCacheTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/cache/internal/NaturalIdCacheKey.java b/hibernate-core/src/main/java/org/hibernate/cache/internal/NaturalIdCacheKey.java index 89abfd5838..3b77f592bc 100644 --- a/hibernate-core/src/main/java/org/hibernate/cache/internal/NaturalIdCacheKey.java +++ b/hibernate-core/src/main/java/org/hibernate/cache/internal/NaturalIdCacheKey.java @@ -38,7 +38,7 @@ public class NaturalIdCacheKey implements Serializable { } public NaturalIdCacheKey(Object naturalIdValues, EntityPersister persister, String entityName, SharedSessionContractImplementor session) { - this( persister.getNaturalIdMapping().disassemble( naturalIdValues, session ), entityName, session.getTenantIdentifier(), persister.getNaturalIdMapping().calculateHashCode( naturalIdValues ) ); + this( persister.getNaturalIdMapping().disassemble( naturalIdValues, session ), entityName, session.getTenantIdentifier(), persister.getNaturalIdMapping().calculateHashCode( naturalIdValues, session ) ); } @Internal diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/NaturalIdResolutionsImpl.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/NaturalIdResolutionsImpl.java index 6f0f5c2540..9243d0b88e 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/NaturalIdResolutionsImpl.java +++ b/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 ); + hashCodeCalculation = prime * hashCodeCalculation + entityDescriptor.getNaturalIdMapping().calculateHashCode( naturalIdValue, persistenceContext.getSession() ); this.hashCode = hashCodeCalculation; } diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NaturalIdMapping.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NaturalIdMapping.java index b6b394a5c2..20fe44f34a 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/NaturalIdMapping.java +++ b/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); + int calculateHashCode(Object value, SharedSessionContractImplementor session); /** * Make a loader capable of loading a single entity by natural-id diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/CompoundNaturalIdMapping.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/CompoundNaturalIdMapping.java index f88b391d33..98273fe951 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/CompoundNaturalIdMapping.java +++ b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/CompoundNaturalIdMapping.java @@ -172,8 +172,9 @@ public class CompoundNaturalIdMapping extends AbstractNaturalIdMapping implement } @Override - public int calculateHashCode(Object value) { - return 0; + public int calculateHashCode(Object value, SharedSessionContractImplementor session) { + Object o = disassemble( value, session ) ; + return Arrays.hashCode((Object[]) o); } @Override diff --git a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/SimpleNaturalIdMapping.java b/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/SimpleNaturalIdMapping.java index 49b97b8a0e..a3ee3c145a 100644 --- a/hibernate-core/src/main/java/org/hibernate/metamodel/mapping/internal/SimpleNaturalIdMapping.java +++ b/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) { + public int calculateHashCode(Object value, SharedSessionContractImplementor session) { //noinspection unchecked return value == null ? 0 : ( (JavaType) getJavaType() ).extractHashCode( value ); } diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/caching/mocked/NaturalIdCacheKeyTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/caching/mocked/NaturalIdCacheKeyTest.java index e44e45f749..03ea1eaf9c 100644 --- a/hibernate-core/src/test/java/org/hibernate/orm/test/caching/mocked/NaturalIdCacheKeyTest.java +++ b/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() ) ).thenAnswer( invocation -> invocation.getArguments()[0].hashCode() ); + when( naturalIdMapping.calculateHashCode( any(), eq( sessionImplementor ) ) ).thenAnswer( invocation -> invocation.getArguments()[0].hashCode() ); final NaturalIdCacheKey key = (NaturalIdCacheKey) DefaultCacheKeysFactory.staticCreateNaturalIdKey( new Object[] {"a", "b", "c"}, entityPersister, sessionImplementor ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/compound/CompoundNaturalIdCacheTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/compound/CompoundNaturalIdCacheTest.java new file mode 100644 index 0000000000..9be80e99da --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/mapping/naturalid/compound/CompoundNaturalIdCacheTest.java @@ -0,0 +1,186 @@ +/* + * 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 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.junit.jupiter.api.Assertions.assertTrue; + +/** + * @author Sylvain Dusart + */ +@TestForIssue(jiraKey = "HHH-16218") +public class CompoundNaturalIdCacheTest extends BaseCoreFunctionalTestCase { + + @Override + protected Class[] getAnnotatedClasses() { + return new Class[]{ + EntityWithSimpleNaturalId.class, + EntityWithCompoundNaturalId.class + }; + } + + @Override + protected void configure(Configuration configuration) { + super.configure(configuration); + + configuration.setProperty(AvailableSettings.STATEMENT_BATCH_SIZE, "5000"); + configuration.setProperty( AvailableSettings.SHOW_SQL, Boolean.FALSE.toString() ); + configuration.setProperty(AvailableSettings.GENERATE_STATISTICS, "false"); + } + + @Test + @Timeout(30) + public void createThenLoadTest() { + Session s = openSession(); + Transaction tx = s.beginTransaction(); + + int objectsNb = 20000; + + log.info("Starting creations"); + + var creationDurationForCompoundNaturalId = createEntities(i -> { + var entity = new EntityWithCompoundNaturalId(); + 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 creator, final int objectsNb) { + var start = System.currentTimeMillis(); + + for (int i = 0; i < objectsNb; i++) { + session.persist(creator.apply(i)); + } + + return System.currentTimeMillis() - start; + } + + 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 + @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 + 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; + } + } +}