Skip to content

Commit

Permalink
HHH-18976 Get rid of dynamic array instantiation in MultiEntityLoader…
Browse files Browse the repository at this point in the history
…Standard

MultiEntityLoaderStandard is used for arbitrary ID types, including IdClass,
making it very problematic to instantiate T[] where T is the ID type:
in native images, it requires registering T[] for reflection for every T
that can possibly be used as an ID type.

Fortunately, MultiEntityLoaderStandard does not, in fact, need
concrete-type arrays: Object[] works perfectly well with this
implementation, and only the other implementation,
MultiIdEntityLoaderArrayParam, actually needs concrete-type arrays.
We're truly in a lucky streak, because MultiIdEntityLoaderArrayParam is
only used for well-known, basic types such as Integer, which can easily
be registered for reflection in native images, and likely will be for
other reasons anyway.

Some of the dynamic instantiations were originally introduced to fix
the following issue:

* HHH-17201 -- tested in MultiIdEntityLoadTests

The corresponding tests still pass after removing these dynamic array
instantiations.
  • Loading branch information
yrodiere committed Jan 14, 2025
1 parent 62199d0 commit 5ffff1b
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
import org.hibernate.graph.GraphSemantic;
import org.hibernate.graph.RootGraph;
import org.hibernate.graph.spi.RootGraphImplementor;
import org.hibernate.loader.ast.internal.LoaderHelper;
import org.hibernate.persister.entity.EntityPersister;
import org.hibernate.loader.ast.spi.MultiIdLoadOptions;

Expand Down Expand Up @@ -191,7 +190,7 @@ public <K> List<T> multiLoad(List<K> ids) {
}
else {
return perform( () -> (List<T>) entityPersister.multiLoad(
ids.toArray( LoaderHelper.createTypedArray( ids.get( 0 ).getClass(), ids.size() ) ),
ids.toArray( new Object[0] ),
session,
this
) );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
import org.hibernate.engine.spi.EntityKey;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.event.spi.EventSource;
import org.hibernate.internal.build.AllowReflection;
import org.hibernate.loader.ast.spi.MultiIdEntityLoader;
import org.hibernate.loader.ast.spi.MultiIdLoadOptions;
import org.hibernate.loader.internal.CacheLoadHelper.PersistenceContextEntry;
Expand All @@ -22,7 +21,6 @@
import org.hibernate.sql.exec.spi.JdbcSelectExecutor;
import org.hibernate.type.descriptor.java.JavaType;

import java.lang.reflect.Array;
import java.util.ArrayList;
import java.util.List;

Expand All @@ -40,15 +38,12 @@
public abstract class AbstractMultiIdEntityLoader<T> implements MultiIdEntityLoader<T> {
private final EntityMappingType entityDescriptor;
private final SessionFactoryImplementor sessionFactory;
private final EntityIdentifierMapping identifierMapping;
protected final Object[] idArray;
protected final EntityIdentifierMapping identifierMapping;

@AllowReflection
public AbstractMultiIdEntityLoader(EntityMappingType entityDescriptor, SessionFactoryImplementor sessionFactory) {
this.entityDescriptor = entityDescriptor;
this.sessionFactory = sessionFactory;
identifierMapping = getLoadable().getIdentifierMapping();
idArray = (Object[]) Array.newInstance( identifierMapping.getJavaType().getJavaTypeClass(), 0 );
}

protected EntityMappingType getEntityDescriptor() {
Expand Down Expand Up @@ -303,10 +298,13 @@ else if ( unresolvedIds.size() == ids.length ) {
}
else {
// we need to load only some the ids
return unresolvedIds.toArray( idArray );
return toIdArray( unresolvedIds );
}
}

// Depending on the implementation, a specific subtype of Object[] (e.g. Integer[]) may be needed.
protected abstract Object[] toIdArray(List<Object> ids);

private boolean isIdCoercionEnabled() {
return !getSessionFactory().getJpaMetamodel().getJpaCompliance().isLoadByIdComplianceEnabled();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
*/
package org.hibernate.loader.ast.internal;

import java.lang.reflect.Array;
import java.util.Arrays;
import java.util.List;

import org.hibernate.LockOptions;
Expand All @@ -12,6 +14,7 @@
import org.hibernate.engine.spi.PersistenceContext;
import org.hibernate.engine.spi.SessionFactoryImplementor;
import org.hibernate.event.spi.EventSource;
import org.hibernate.internal.build.AllowReflection;
import org.hibernate.loader.ast.spi.MultiIdLoadOptions;
import org.hibernate.loader.ast.spi.SqlArrayMultiKeyLoader;
import org.hibernate.metamodel.mapping.BasicEntityIdentifierMapping;
Expand Down Expand Up @@ -44,12 +47,15 @@
public class MultiIdEntityLoaderArrayParam<E> extends AbstractMultiIdEntityLoader<E> implements SqlArrayMultiKeyLoader {
private final JdbcMapping arrayJdbcMapping;
private final JdbcParameter jdbcParameter;
protected final Object[] idArray;

@AllowReflection
public MultiIdEntityLoaderArrayParam(
EntityMappingType entityDescriptor,
SessionFactoryImplementor sessionFactory) {
super( entityDescriptor, sessionFactory );
final Class<?> idClass = idArray.getClass().getComponentType();
final Class<?> idClass = identifierMapping.getJavaType().getJavaTypeClass();
idArray = (Object[]) Array.newInstance( idClass, 0 );
arrayJdbcMapping = resolveArrayJdbcMapping(
getIdentifierMapping().getJdbcMapping(),
idClass,
Expand Down Expand Up @@ -117,7 +123,7 @@ protected void loadEntitiesById(

final JdbcParameterBindings jdbcParameterBindings = new JdbcParameterBindingsImpl(1);
jdbcParameterBindings.addBinding( jdbcParameter,
new JdbcParameterBindingImpl( arrayJdbcMapping, idsInBatch.toArray( idArray ) ) );
new JdbcParameterBindingImpl( arrayJdbcMapping, toIdArray( idsInBatch ) ) );

getJdbcSelectExecutor().executeQuery(
getSqlAstTranslatorFactory().buildSelectTranslator( getSessionFactory(), sqlAst )
Expand Down Expand Up @@ -165,7 +171,7 @@ protected void loadEntitiesWithUnresolvedIds(
.translate( NO_BINDINGS, QueryOptions.NONE );

final List<E> databaseResults = loadByArrayParameter(
unresolvableIds,
toIdArray( unresolvableIds ),
sqlAst,
jdbcSelectOperation,
jdbcParameter,
Expand All @@ -189,4 +195,19 @@ protected void loadEntitiesWithUnresolvedIds(
}
}

@Override
protected Object[] toIdArray(List<Object> ids) {
return ids.toArray( idArray );
}

protected Object[] toIdArray(Object[] ids) {
if ( ids.getClass().equals( idArray.getClass() ) ) {
return ids;
}
else {
Object[] typedIdArray = Arrays.copyOf( idArray, ids.length );
System.arraycopy( ids, 0, typedIdArray, 0, ids.length );
return typedIdArray;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -218,4 +218,10 @@ protected void loadEntitiesWithUnresolvedIds(
idPosition += batchSize;
}
}

@Override
protected Object[] toIdArray(List<Object> ids) {
// This loader implementation doesn't need arrays to have a specific type, Object[] will do.
return ids.toArray( new Object[0] );
}
}

0 comments on commit 5ffff1b

Please sign in to comment.