From be895c722bab4d7755fe5d805277edca7964fb74 Mon Sep 17 00:00:00 2001 From: Gavin King Date: Fri, 25 Oct 2024 22:01:47 +0200 Subject: [PATCH] HHH-18767 make MultiIdEntityLoaderArrayParam respect explicit BatchSize keep ignoring the *implicit* upper limit from the Dialect refactor a very long method which was extremely hard to understand Signed-off-by: Gavin King --- .../MultiIdEntityLoaderArrayParam.java | 229 +++++++++++------- .../internal/MultiIdEntityLoaderStandard.java | 184 +++++++------- .../entity/AbstractEntityPersister.java | 11 +- 3 files changed, 239 insertions(+), 185 deletions(-) diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderArrayParam.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderArrayParam.java index 9372168d79..f5f20e5cbf 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderArrayParam.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderArrayParam.java @@ -40,9 +40,11 @@ import org.hibernate.sql.results.internal.RowTransformerStandardImpl; import org.hibernate.sql.results.spi.ManagedResultConsumer; import org.checkerframework.checker.nullness.qual.NonNull; +import org.hibernate.type.descriptor.java.JavaType; import static java.lang.Boolean.TRUE; import static org.hibernate.internal.util.collections.CollectionHelper.isEmpty; +import static org.hibernate.loader.ast.internal.CacheEntityLoaderHelper.loadFromSessionCacheStatic; /** * @author Steve Ebersole @@ -50,9 +52,11 @@ import static org.hibernate.internal.util.collections.CollectionHelper.isEmpty; public class MultiIdEntityLoaderArrayParam extends AbstractMultiIdEntityLoader implements SqlArrayMultiKeyLoader { private final JdbcMapping arrayJdbcMapping; private final JdbcParameter jdbcParameter; + private final int idJdbcTypeCount; - public MultiIdEntityLoaderArrayParam(EntityMappingType entityDescriptor, SessionFactoryImplementor sessionFactory) { + public MultiIdEntityLoaderArrayParam(EntityMappingType entityDescriptor, int identifierColumnSpan, SessionFactoryImplementor sessionFactory) { super( entityDescriptor, sessionFactory ); + this.idJdbcTypeCount = identifierColumnSpan; final Class arrayClass = createTypedArray( 0 ).getClass(); arrayJdbcMapping = MultiKeyLoadHelper.resolveArrayJdbcMapping( getSessionFactory().getTypeConfiguration().getBasicTypeRegistry().getRegisteredType( arrayClass ), @@ -77,88 +81,152 @@ public class MultiIdEntityLoaderArrayParam extends AbstractMultiIdEntityLoade ); } + assert loadOptions.isOrderReturnEnabled(); + final boolean coerce = !getSessionFactory().getJpaMetamodel().getJpaCompliance().isLoadByIdComplianceEnabled(); - final LockOptions lockOptions = (loadOptions.getLockOptions() == null) + final JavaType idType = getLoadable().getIdentifierMapping().getJavaType(); + + final LockOptions lockOptions = loadOptions.getLockOptions() == null ? new LockOptions( LockMode.NONE ) : loadOptions.getLockOptions(); + final int maxBatchSize = maxBatchSize( ids, loadOptions ); + final List result = CollectionHelper.arrayList( ids.length ); - List idsToLoadFromDatabase = null; - List idsToLoadFromDatabaseResultIndexes = null; + + final List idsInBatch = new ArrayList<>(); + final List elementPositionsLoadedByBatch = new ArrayList<>(); for ( int i = 0; i < ids.length; i++ ) { - final Object id; - if ( coerce ) { - id = getLoadable().getIdentifierMapping().getJavaType().coerce( ids[ i ], session ); - } - else { - id = ids[ i ]; - } - + final Object id = coerce ? idType.coerce( ids[i], session ) : ids[i]; final EntityKey entityKey = new EntityKey( id, getLoadable().getEntityPersister() ); - if ( loadOptions.isSessionCheckingEnabled() || loadOptions.isSecondLevelCacheCheckingEnabled() ) { - LoadEvent loadEvent = new LoadEvent( - id, - getLoadable().getJavaType().getJavaTypeClass().getName(), - lockOptions, - session, - LoaderHelper.getReadOnlyFromLoadQueryInfluencers(session) + if ( !loadFromCaches( loadOptions, session, id, lockOptions, entityKey, result, i ) ) { + // if we did not hit any of the continues above, + // then we need to batch load the entity state. + idsInBatch.add( id ); + + if ( idsInBatch.size() >= maxBatchSize ) { + // we've hit the allotted max-batch-size, perform an "intermediate load" + loadEntitiesById( loadOptions, session, lockOptions, idsInBatch ); + idsInBatch.clear(); + } + + // Save the EntityKey instance for use later + result.add( i, entityKey ); + elementPositionsLoadedByBatch.add( i ); + } + } + + if ( !idsInBatch.isEmpty() ) { + // we still have ids to load from the processing above since + // the last max-batch-size trigger, perform a load for them + loadEntitiesById( loadOptions, session, lockOptions, idsInBatch ); + } + + // for each result where we set the EntityKey earlier, replace them + handleResults( loadOptions, session, elementPositionsLoadedByBatch, result ); + + //noinspection unchecked + return (List) result; + } + + private boolean loadFromCaches( + MultiIdLoadOptions loadOptions, + EventSource session, + Object id, + LockOptions lockOptions, + EntityKey entityKey, + List result, + int i) { + if ( loadOptions.isSessionCheckingEnabled() || loadOptions.isSecondLevelCacheCheckingEnabled() ) { + final LoadEvent loadEvent = new LoadEvent( + id, + getLoadable().getJavaType().getJavaTypeClass().getName(), + lockOptions, + session, + LoaderHelper.getReadOnlyFromLoadQueryInfluencers( session ) + ); + + Object managedEntity = null; + + if ( loadOptions.isSessionCheckingEnabled() ) { + // look for it in the Session first + final PersistenceContextEntry persistenceContextEntry = + loadFromSessionCacheStatic( loadEvent, entityKey, LoadEventListener.GET ); + managedEntity = persistenceContextEntry.getEntity(); + + if ( managedEntity != null + && !loadOptions.isReturnOfDeletedEntitiesEnabled() + && !persistenceContextEntry.isManaged() ) { + // put a null in the result + result.add( i, null ); + return true; + } + } + + if ( managedEntity == null && loadOptions.isSecondLevelCacheCheckingEnabled() ) { + // look for it in the SessionFactory + managedEntity = CacheEntityLoaderHelper.INSTANCE.loadFromSecondLevelCache( + loadEvent, + getLoadable().getEntityPersister(), + entityKey ); - - Object managedEntity = null; - - if ( loadOptions.isSessionCheckingEnabled() ) { - // look for it in the Session first - final PersistenceContextEntry persistenceContextEntry = CacheEntityLoaderHelper.loadFromSessionCacheStatic( - loadEvent, - entityKey, - LoadEventListener.GET - ); - managedEntity = persistenceContextEntry.getEntity(); - - if ( managedEntity != null - && !loadOptions.isReturnOfDeletedEntitiesEnabled() - && !persistenceContextEntry.isManaged() ) { - // put a null in the result - result.add( i, null ); - continue; - } - } - - if ( managedEntity == null && loadOptions.isSecondLevelCacheCheckingEnabled() ) { - // look for it in the SessionFactory - managedEntity = CacheEntityLoaderHelper.INSTANCE.loadFromSecondLevelCache( - loadEvent, - getLoadable().getEntityPersister(), - entityKey - ); - } - - if ( managedEntity != null ) { - result.add( i, managedEntity ); - continue; - } } - // if we did not hit any of the continues above, then we need to batch - // load the entity state. - if ( idsToLoadFromDatabase == null ) { - idsToLoadFromDatabase = new ArrayList<>(); - idsToLoadFromDatabaseResultIndexes = new ArrayList<>(); + if ( managedEntity != null ) { + result.add( i, managedEntity ); + return true; } - // hold its place in the result with the EntityKey, we'll come back to it later - result.add( i, entityKey ); - idsToLoadFromDatabase.add( id ); - idsToLoadFromDatabaseResultIndexes.add( i ); } + return false; + } - if ( idsToLoadFromDatabase == null ) { - // all the given ids were already associated with the Session - //noinspection unchecked - return (List) result; + private static void handleResults( + MultiIdLoadOptions loadOptions, + EventSource session, + List elementPositionsLoadedByBatch, + List result) { + final PersistenceContext persistenceContext = session.getPersistenceContext(); + for ( Integer position : elementPositionsLoadedByBatch ) { + // the element value at this position in the result List should be + // the EntityKey for that entity - reuse it + final EntityKey entityKey = (EntityKey) result.get( position ); + BatchFetchQueueHelper.removeBatchLoadableEntityKey( entityKey, session ); + Object entity = persistenceContext.getEntity( entityKey ); + if ( entity != null && !loadOptions.isReturnOfDeletedEntitiesEnabled() ) { + // make sure it is not DELETED + final EntityEntry entry = persistenceContext.getEntry( entity ); + if ( entry.getStatus().isDeletedOrGone() ) { + // the entity is locally deleted, and the options ask that we not return such entities... + entity = null; + } + else { + entity = persistenceContext.proxyFor( entity ); + } + } + result.set( position, entity ); } + } + private int maxBatchSize(K[] ids, MultiIdLoadOptions loadOptions) { + if ( loadOptions.getBatchSize() != null && loadOptions.getBatchSize() > 0 ) { + return loadOptions.getBatchSize(); + } + else { + // disable batching by default + return ids.length; +// return getSessionFactory().getJdbcServices().getJdbcEnvironment().getDialect() +// .getBatchLoadSizingStrategy().determineOptimalBatchLoadSize( +// idJdbcTypeCount, +// ids.length, +// getSessionFactory().getSessionFactoryOptions().inClauseParameterPaddingEnabled() +// ); + } + } + + private void loadEntitiesById( + MultiIdLoadOptions loadOptions, EventSource session, LockOptions lockOptions, List idsToLoadFromDatabase) { final SelectStatement sqlAst = LoaderSelectBuilder.createSelectBySingleArrayParameter( getLoadable(), getIdentifierMapping(), @@ -194,37 +262,12 @@ public class MultiIdEntityLoaderArrayParam extends AbstractMultiIdEntityLoade jdbcParameterBindings, new ExecutionContextWithSubselectFetchHandler( session, subSelectFetchableKeysHandler, - TRUE.equals( loadOptions.getReadOnly(session) ) ), + TRUE.equals( loadOptions.getReadOnly( session ) ) ), RowTransformerStandardImpl.instance(), null, idsToLoadFromDatabase.size(), ManagedResultConsumer.INSTANCE ); - - for ( int i = 0; i < idsToLoadFromDatabaseResultIndexes.size(); i++ ) { - final Integer resultIndex = idsToLoadFromDatabaseResultIndexes.get(i); - - // the element value at this position in the result List should be - // the EntityKey for that entity - reuse it - final EntityKey entityKey = (EntityKey) result.get( resultIndex ); - BatchFetchQueueHelper.removeBatchLoadableEntityKey( entityKey, session ); - Object entity = persistenceContext.getEntity( entityKey ); - if ( entity != null && !loadOptions.isReturnOfDeletedEntitiesEnabled() ) { - // make sure it is not DELETED - final EntityEntry entry = persistenceContext.getEntry( entity ); - if ( entry.getStatus().isDeletedOrGone() ) { - // the entity is locally deleted, and the options ask that we not return such entities... - entity = null; - } - else { - entity = persistenceContext.proxyFor( entity ); - } - } - result.set( resultIndex, entity ); - } - - //noinspection unchecked - return (List) result; } @@ -345,7 +388,7 @@ public class MultiIdEntityLoaderArrayParam extends AbstractMultiIdEntityLoade Object resolvedEntity = null; // look for it in the Session first - final PersistenceContextEntry persistenceContextEntry = CacheEntityLoaderHelper.loadFromSessionCacheStatic( + final PersistenceContextEntry persistenceContextEntry = loadFromSessionCacheStatic( loadEvent, entityKey, LoadEventListener.GET diff --git a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderStandard.java b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderStandard.java index 391ef3ce2f..e817bb6a1f 100644 --- a/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderStandard.java +++ b/hibernate-core/src/main/java/org/hibernate/loader/ast/internal/MultiIdEntityLoaderStandard.java @@ -12,8 +12,6 @@ import java.util.List; import org.hibernate.LockMode; import org.hibernate.LockOptions; -import org.hibernate.dialect.Dialect; -import org.hibernate.engine.jdbc.env.spi.JdbcEnvironment; import org.hibernate.engine.spi.EntityEntry; import org.hibernate.engine.spi.EntityKey; import org.hibernate.engine.spi.PersistenceContext; @@ -24,6 +22,7 @@ import org.hibernate.event.spi.EventSource; import org.hibernate.event.spi.LoadEvent; import org.hibernate.event.spi.LoadEventListener; import org.hibernate.internal.util.collections.CollectionHelper; +import org.hibernate.loader.ast.internal.CacheEntityLoaderHelper.PersistenceContextEntry; import org.hibernate.loader.ast.spi.MultiIdLoadOptions; import org.hibernate.persister.entity.EntityPersister; import org.hibernate.query.spi.QueryOptions; @@ -36,9 +35,11 @@ import org.hibernate.sql.exec.spi.JdbcParametersList; import org.hibernate.sql.results.internal.RowTransformerStandardImpl; import org.hibernate.sql.results.spi.ListResultsConsumer; +import org.hibernate.type.descriptor.java.JavaType; import org.jboss.logging.Logger; import static java.lang.Boolean.TRUE; +import static org.hibernate.loader.ast.internal.CacheEntityLoaderHelper.loadFromSessionCacheStatic; /** * Standard MultiIdEntityLoader @@ -71,108 +72,110 @@ public class MultiIdEntityLoaderStandard extends AbstractMultiIdEntityLoader< assert loadOptions.isOrderReturnEnabled(); - final JdbcEnvironment jdbcEnvironment = getSessionFactory().getJdbcServices().getJdbcEnvironment(); - final Dialect dialect = jdbcEnvironment.getDialect(); + final boolean coerce = !getSessionFactory().getJpaMetamodel().getJpaCompliance().isLoadByIdComplianceEnabled(); + final JavaType idType = getLoadable().getIdentifierMapping().getJavaType(); - final List result = CollectionHelper.arrayList( ids.length ); - - final LockOptions lockOptions = (loadOptions.getLockOptions() == null) + final LockOptions lockOptions = loadOptions.getLockOptions() == null ? new LockOptions( LockMode.NONE ) : loadOptions.getLockOptions(); - final int maxBatchSize; - if ( loadOptions.getBatchSize() != null && loadOptions.getBatchSize() > 0 ) { - maxBatchSize = loadOptions.getBatchSize(); - } - else { - maxBatchSize = dialect.getBatchLoadSizingStrategy().determineOptimalBatchLoadSize( - idJdbcTypeCount, - ids.length, - getSessionFactory().getSessionFactoryOptions().inClauseParameterPaddingEnabled() - ); - } + final int maxBatchSize = maxBatchSize( ids, loadOptions ); + + final List result = CollectionHelper.arrayList( ids.length ); final List idsInBatch = new ArrayList<>(); final List elementPositionsLoadedByBatch = new ArrayList<>(); - final boolean coerce = !getSessionFactory().getJpaMetamodel().getJpaCompliance().isLoadByIdComplianceEnabled(); for ( int i = 0; i < ids.length; i++ ) { - final Object id; - if ( coerce ) { - id = getLoadable().getIdentifierMapping().getJavaType().coerce( ids[i], session ); - } - else { - id = ids[i]; - } + final Object id = coerce ? idType.coerce( ids[i], session ) : ids[i]; final EntityKey entityKey = new EntityKey( id, getLoadable().getEntityPersister() ); - if ( loadOptions.isSessionCheckingEnabled() || loadOptions.isSecondLevelCacheCheckingEnabled() ) { - LoadEvent loadEvent = new LoadEvent( - id, - getLoadable().getJavaType().getJavaTypeClass().getName(), - lockOptions, - session, - LoaderHelper.getReadOnlyFromLoadQueryInfluencers(session) - ); + if ( !loadFromCaches( loadOptions, session, id, lockOptions, entityKey, result, i ) ) { + // if we did not hit any of the continues above, + // then we need to batch load the entity state. + idsInBatch.add( id ); - Object managedEntity = null; - - if ( loadOptions.isSessionCheckingEnabled() ) { - // look for it in the Session first - CacheEntityLoaderHelper.PersistenceContextEntry persistenceContextEntry = CacheEntityLoaderHelper.INSTANCE - .loadFromSessionCache( - loadEvent, - entityKey, - LoadEventListener.GET - ); - managedEntity = persistenceContextEntry.getEntity(); - - if ( managedEntity != null - && !loadOptions.isReturnOfDeletedEntitiesEnabled() - && !persistenceContextEntry.isManaged() ) { - // put a null in the result - result.add( i, null ); - continue; - } + if ( idsInBatch.size() >= maxBatchSize ) { + // we've hit the allotted max-batch-size, perform an "intermediate load" + loadEntitiesById( idsInBatch, lockOptions, loadOptions, session ); + idsInBatch.clear(); } - if ( managedEntity == null && loadOptions.isSecondLevelCacheCheckingEnabled() ) { - // look for it in the SessionFactory - managedEntity = CacheEntityLoaderHelper.INSTANCE.loadFromSecondLevelCache( - loadEvent, - getLoadable().getEntityPersister(), - entityKey - ); - } - - if ( managedEntity != null ) { - result.add( i, managedEntity ); - continue; - } + // Save the EntityKey instance for use later + result.add( i, entityKey ); + elementPositionsLoadedByBatch.add( i ); } - - // if we did not hit any of the continues above, then we need to batch - // load the entity state. - idsInBatch.add( id ); - - if ( idsInBatch.size() >= maxBatchSize ) { - // we've hit the allotted max-batch-size, perform an "intermediate load" - loadEntitiesById( idsInBatch, lockOptions, loadOptions, session ); - idsInBatch.clear(); - } - - // Save the EntityKey instance for use later - result.add( i, entityKey ); - elementPositionsLoadedByBatch.add( i ); } if ( !idsInBatch.isEmpty() ) { - // we still have ids to load from the processing above since the last max-batch-size trigger, - // perform a load for them + // we still have ids to load from the processing above since + // the last max-batch-size trigger, perform a load for them loadEntitiesById( idsInBatch, lockOptions, loadOptions, session ); } // for each result where we set the EntityKey earlier, replace them + handleResults( loadOptions, session, elementPositionsLoadedByBatch, result ); + + //noinspection unchecked + return (List) result; + } + + private boolean loadFromCaches( + MultiIdLoadOptions loadOptions, + EventSource session, + Object id, + LockOptions lockOptions, + EntityKey entityKey, + List result, + int i) { + if ( loadOptions.isSessionCheckingEnabled() || loadOptions.isSecondLevelCacheCheckingEnabled() ) { + final LoadEvent loadEvent = new LoadEvent( + id, + getLoadable().getJavaType().getJavaTypeClass().getName(), + lockOptions, + session, + LoaderHelper.getReadOnlyFromLoadQueryInfluencers( session ) + ); + + Object managedEntity = null; + + if ( loadOptions.isSessionCheckingEnabled() ) { + // look for it in the Session first + final PersistenceContextEntry persistenceContextEntry = + loadFromSessionCacheStatic( loadEvent, entityKey, LoadEventListener.GET ); + managedEntity = persistenceContextEntry.getEntity(); + + if ( managedEntity != null + && !loadOptions.isReturnOfDeletedEntitiesEnabled() + && !persistenceContextEntry.isManaged() ) { + // put a null in the result + result.add( i, null ); + return true; + } + } + + if ( managedEntity == null && loadOptions.isSecondLevelCacheCheckingEnabled() ) { + // look for it in the SessionFactory + managedEntity = CacheEntityLoaderHelper.INSTANCE.loadFromSecondLevelCache( + loadEvent, + getLoadable().getEntityPersister(), + entityKey + ); + } + + if ( managedEntity != null ) { + result.add( i, managedEntity ); + return true; + } + } + return false; + } + + private static void handleResults( + MultiIdLoadOptions loadOptions, + EventSource session, + List elementPositionsLoadedByBatch, + List result) { final PersistenceContext persistenceContext = session.getPersistenceContextInternal(); for ( Integer position : elementPositionsLoadedByBatch ) { // the element value at this position in the result List should be @@ -192,9 +195,20 @@ public class MultiIdEntityLoaderStandard extends AbstractMultiIdEntityLoader< } result.set( position, entity ); } + } - //noinspection unchecked - return (List) result; + private int maxBatchSize(Object[] ids, MultiIdLoadOptions loadOptions) { + if ( loadOptions.getBatchSize() != null && loadOptions.getBatchSize() > 0 ) { + return loadOptions.getBatchSize(); + } + else { + return getSessionFactory().getJdbcServices().getJdbcEnvironment().getDialect() + .getBatchLoadSizingStrategy().determineOptimalBatchLoadSize( + idJdbcTypeCount, + ids.length, + getSessionFactory().getSessionFactoryOptions().inClauseParameterPaddingEnabled() + ); + } } private List loadEntitiesById( @@ -334,7 +348,7 @@ public class MultiIdEntityLoaderStandard extends AbstractMultiIdEntityLoader< Object managedEntity = null; // look for it in the Session first - CacheEntityLoaderHelper.PersistenceContextEntry persistenceContextEntry = CacheEntityLoaderHelper.INSTANCE + PersistenceContextEntry persistenceContextEntry = CacheEntityLoaderHelper.INSTANCE .loadFromSessionCache( loadEvent, entityKey, diff --git a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java index 67070b1325..79b6318715 100644 --- a/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java +++ b/hibernate-core/src/main/java/org/hibernate/persister/entity/AbstractEntityPersister.java @@ -855,13 +855,10 @@ public abstract class AbstractEntityPersister } protected MultiIdEntityLoader buildMultiIdLoader() { - if ( getIdentifierType() instanceof BasicType - && supportsSqlArrayType( factory.getJdbcServices().getDialect() ) ) { - return new MultiIdEntityLoaderArrayParam<>( this, factory ); - } - else { - return new MultiIdEntityLoaderStandard<>( this, identifierColumnSpan, factory ); - } + final Dialect dialect = factory.getJdbcServices().getDialect(); + return getIdentifierType() instanceof BasicType && supportsSqlArrayType( dialect ) + ? new MultiIdEntityLoaderArrayParam<>( this, identifierColumnSpan, factory ) + : new MultiIdEntityLoaderStandard<>( this, identifierColumnSpan, factory ); } private String getIdentitySelectString(Dialect dialect) {