Skip to content

Commit

Permalink
HSEARCH-2197 Loading of search results produces useless garbage.
Browse files Browse the repository at this point in the history
  • Loading branch information
golovnin committed Mar 30, 2016
1 parent 8a34744 commit 68cfda9
Show file tree
Hide file tree
Showing 13 changed files with 41 additions and 40 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public Object loadWithoutTiming(EntityInfo entityInfo) {
public abstract Object executeLoad(EntityInfo entityInfo);

@Override
public List load(EntityInfo... entityInfos) {
public List load(List<EntityInfo> entityInfos) {
long startTime = 0;
if ( takeTimings ) {
startTime = System.nanoTime();
Expand All @@ -63,7 +63,7 @@ public List load(EntityInfo... entityInfos) {
return loadedObjects;
}

public abstract List executeLoad(EntityInfo... entityInfo);
public abstract List executeLoad(List<EntityInfo> entityInfo);
}


Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,11 @@ private CriteriaObjectInitializer() {
}

@Override
public void initializeObjects(EntityInfo[] entityInfos,
public void initializeObjects(List<EntityInfo> entityInfos,
LinkedHashMap<EntityInfoLoadKey, Object> idToObjectMap,
ObjectInitializationContext objectInitializationContext) {
// Do not call isTimeOut here as the caller might be the last biggie on the list.
final int maxResults = entityInfos.length;
final int maxResults = entityInfos.size();
if ( log.isTraceEnabled() ) {
log.tracef( "Load %d objects using criteria queries", maxResults );
}
Expand Down Expand Up @@ -116,7 +116,7 @@ private void setCriteriaTimeout(Criteria criteria, TimeoutManager timeoutManager
* will contain one criteria object for each id space used by the given infos. A single criteria will be returned in
* case all the entity infos originate from the same id space.
*/
private List<Criteria> buildUpCriteria(EntityInfo[] entityInfos, ObjectInitializationContext objectInitializationContext) {
private List<Criteria> buildUpCriteria(List<EntityInfo> entityInfos, ObjectInitializationContext objectInitializationContext) {
Map<Class<?>, List<EntityInfo>> infosByIdSpace = groupInfosByIdSpace( entityInfos, objectInitializationContext );

// all entities from same id space -> single criteria
Expand Down Expand Up @@ -188,7 +188,7 @@ private Criterion getIdListCriterion(List<EntityInfo> entityInfos, ObjectInitial
*
* @return The given entity infos, keyed by the root entity type of id spaces
*/
private Map<Class<?>, List<EntityInfo>> groupInfosByIdSpace(EntityInfo[] entityInfos, ObjectInitializationContext objectInitializationContext) {
private Map<Class<?>, List<EntityInfo>> groupInfosByIdSpace(List<EntityInfo> entityInfos, ObjectInitializationContext objectInitializationContext) {
ServiceManager serviceManager = objectInitializationContext.getExtendedSearchIntegrator().getServiceManager();
IdUniquenessResolver resolver = serviceManager.requestService( IdUniquenessResolver.class );
SessionFactoryImplementor sessionFactory = (SessionFactoryImplementor) objectInitializationContext.getSession().getSessionFactory();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ public List list() {
hSearchQuery.getTimeoutManager().start();
final List<EntityInfo> entityInfos = hSearchQuery.queryEntityInfos();
Loader loader = getLoader();
List list = loader.load( entityInfos.toArray( new EntityInfo[entityInfos.size()] ) );
List list = loader.load( entityInfos );
//no need to timeoutManager.isTimedOut from this point, we don't do anything intensive
if ( resultTransformer == null || loader instanceof ProjectionLoader ) {
//stay consistent with transformTuple which can only be executed during a projection
Expand Down Expand Up @@ -383,7 +383,7 @@ public Object loadWithoutTiming(EntityInfo entityInfo) {
}

@Override
public List load(EntityInfo... entityInfos) {
public List load(List<EntityInfo> entityInfos) {
throw new UnsupportedOperationException( "noLoader should not be used" );
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void init(

Object loadWithoutTiming(EntityInfo entityInfo);

List load(EntityInfo... entityInfos);
List load(List<EntityInfo> entityInfos);

boolean isSizeSafe();
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package org.hibernate.search.query.hibernate.impl;

import java.util.LinkedHashMap;
import java.util.List;

import org.hibernate.search.query.engine.spi.EntityInfo;
import org.hibernate.search.util.logging.impl.Log;
Expand All @@ -33,11 +34,11 @@ private LookupObjectInitializer() {
}

@Override
public void initializeObjects(EntityInfo[] entityInfos, LinkedHashMap<EntityInfoLoadKey, Object> idToObjectMap, ObjectInitializationContext objectInitializationContext) {
public void initializeObjects(List<EntityInfo> entityInfos, LinkedHashMap<EntityInfoLoadKey, Object> idToObjectMap, ObjectInitializationContext objectInitializationContext) {
boolean traceEnabled = log.isTraceEnabled();

// Do not call isTimeOut here as the caller might be the last biggie on the list.
final int maxResults = entityInfos.length;
final int maxResults = entityInfos.size();
if ( maxResults == 0 ) {
if ( traceEnabled ) {
log.tracef( "No object to initialize", maxResults );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,13 @@ public Object executeLoad(EntityInfo entityInfo) {
}

@Override
public List executeLoad(EntityInfo... entityInfos) {
if ( entityInfos.length == 0 ) {
public List executeLoad(List<EntityInfo> entityInfos) {
if ( entityInfos.isEmpty() ) {
return Collections.EMPTY_LIST;
}

if ( entityInfos.length == 1 ) {
final Object entity = load( entityInfos[0] );
if ( entityInfos.size() == 1 ) {
final Object entity = load( entityInfos.get( 0 ) );
if ( entity == null ) {
return Collections.EMPTY_LIST;
}
Expand All @@ -96,7 +96,7 @@ public List executeLoad(EntityInfo... entityInfos) {
}
}

LinkedHashMap<EntityInfoLoadKey, Object> idToObjectMap = new LinkedHashMap<>( (int) ( entityInfos.length / 0.75 ) + 1 );
LinkedHashMap<EntityInfoLoadKey, Object> idToObjectMap = new LinkedHashMap<>( (int) ( entityInfos.size() / 0.75 ) + 1 );

// split EntityInfo per root entity
Map<RootEntityMetadata, List<EntityInfo>> entityInfoBuckets = new HashMap<>( entityMetadata.size() );
Expand Down Expand Up @@ -128,10 +128,9 @@ public List executeLoad(EntityInfo... entityInfos) {
for ( Map.Entry<RootEntityMetadata, List<EntityInfo>> entry : entityInfoBuckets.entrySet() ) {
final RootEntityMetadata key = entry.getKey();
final List<EntityInfo> value = entry.getValue();
final EntityInfo[] bucketEntityInfos = value.toArray( new EntityInfo[value.size()] );

objectInitializer.initializeObjects(
bucketEntityInfos, idToObjectMap, new ObjectInitializationContext(
value, idToObjectMap, new ObjectInitializationContext(
key.criteria,
key.rootEntity,
extendedIntegrator,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package org.hibernate.search.query.hibernate.impl;

import java.util.LinkedHashMap;
import java.util.List;

import org.hibernate.search.query.engine.spi.EntityInfo;

Expand All @@ -27,7 +28,7 @@ public interface ObjectInitializer {
* @param idToObjectMap map keeping to store the loaded entities in
* @param objectInitializationContext gives access to the resources needed in the context of entity initialization
*/
void initializeObjects(EntityInfo[] entityInfos,
void initializeObjects(List<EntityInfo> entityInfos,
LinkedHashMap<EntityInfoLoadKey, Object> idToObjectMap,
ObjectInitializationContext objectInitializationContext);
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ public PersistenceContextObjectInitializer(ObjectInitializer delegate) {
}

@Override
public void initializeObjects(EntityInfo[] entityInfos,
public void initializeObjects(List<EntityInfo> entityInfos,
LinkedHashMap<EntityInfoLoadKey, Object> idToObjectMap,
ObjectInitializationContext objectInitializationContext) {
// Do not call isTimeOut here as the caller might be the last biggie on the list.
final int numberOfObjectsToInitialize = entityInfos.length;
final int numberOfObjectsToInitialize = entityInfos.size();

if ( numberOfObjectsToInitialize == 0 ) {
if ( log.isTraceEnabled() ) {
Expand Down Expand Up @@ -80,7 +80,7 @@ public void initializeObjects(EntityInfo[] entityInfos,

if ( remainingSize > 0 ) {
delegate.initializeObjects(
remainingEntityInfos.toArray( new EntityInfo[remainingSize] ),
remainingEntityInfos,
idToObjectMap,
objectInitializationContext
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,14 @@ private boolean projectionEnabledOnThis(final EntityInfo entityInfo) {
}

@Override
public List load(EntityInfo... entityInfos) {
public List load(List<EntityInfo> entityInfos) {
//no need to timeouManage here, the underlying loader is the real time consumer
List results = new ArrayList( entityInfos.length );
if ( entityInfos.length == 0 ) {
List results = new ArrayList( entityInfos.size() );
if ( entityInfos.isEmpty() ) {
return results;
}

if ( projectionEnabledOnThis( entityInfos[0] ) ) {
if ( projectionEnabledOnThis( entityInfos.get( 0 ) ) ) {
Loader objectLoader = getObjectLoader();
objectLoader.load( entityInfos ); // load by batch
for ( EntityInfo entityInfo : entityInfos ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,24 +60,24 @@ public void setEntityType(Class entityType) {
public final Object executeLoad(EntityInfo entityInfo) {
//if explicit criteria, make sure to use it to load the objects
if ( isExplicitCriteria ) {
load( new EntityInfo[] { entityInfo } );
load( Collections.singletonList( entityInfo ) );
}
final Object result = ObjectLoaderHelper.load( entityInfo, session );
timeoutManager.isTimedOut();
return result;
}

@Override
public final List executeLoad(EntityInfo... entityInfos) {
public final List executeLoad(List<EntityInfo> entityInfos) {
if ( entityType == null ) {
throw new AssertionFailure( "EntityType not defined" );
}

if ( entityInfos.length == 0 ) {
if ( entityInfos.isEmpty() ) {
return Collections.EMPTY_LIST;
}

LinkedHashMap<EntityInfoLoadKey, Object> idToObjectMap = new LinkedHashMap<>( (int) ( entityInfos.length / 0.75 ) + 1 );
LinkedHashMap<EntityInfoLoadKey, Object> idToObjectMap = new LinkedHashMap<>( (int) ( entityInfos.size() / 0.75 ) + 1 );
for ( EntityInfo entityInfo : entityInfos ) {
idToObjectMap.put(
new EntityInfoLoadKey( entityInfo.getClazz(), entityInfo.getId() ),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ private LoadedObject ensureCurrentLoaded() {
}
//preload efficiently by batches:
if ( sizeToLoad > 1 ) {
loader.load( entityInfosToLoad.toArray( new EntityInfo[sizeToLoad] ) );
loader.load( entityInfosToLoad );
//(no references stored at this point: they still need to be loaded one by one to inject null results)
}
return resultsContext[ current - first ];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,11 +31,11 @@ public SecondLevelCacheObjectInitializer(ObjectInitializer delegate) {
}

@Override
public void initializeObjects(EntityInfo[] entityInfos, LinkedHashMap<EntityInfoLoadKey, Object> idToObjectMap, ObjectInitializationContext objectInitializationContext) {
public void initializeObjects(List<EntityInfo> entityInfos, LinkedHashMap<EntityInfoLoadKey, Object> idToObjectMap, ObjectInitializationContext objectInitializationContext) {
boolean traceEnabled = log.isTraceEnabled();

// Do not call isTimeOut here as the caller might be the last biggie on the list.
final int maxResults = entityInfos.length;
final int maxResults = entityInfos.size();
if ( maxResults == 0 ) {
if ( traceEnabled ) {
log.tracef( "No object to initialize", maxResults );
Expand All @@ -44,7 +44,7 @@ public void initializeObjects(EntityInfo[] entityInfos, LinkedHashMap<EntityInfo
}

// check the second-level cache
List<EntityInfo> remainingEntityInfos = new ArrayList<>( entityInfos.length );
List<EntityInfo> remainingEntityInfos = new ArrayList<>( entityInfos.size() );
for ( EntityInfo entityInfo : entityInfos ) {
if ( ObjectLoaderHelper.areDocIdAndEntityIdIdentical( entityInfo, objectInitializationContext.getSession() ) ) {
final boolean isIn2LCache = objectInitializationContext.getSession()
Expand Down Expand Up @@ -82,7 +82,7 @@ public void initializeObjects(EntityInfo[] entityInfos, LinkedHashMap<EntityInfo
}
if ( remainingSize > 0 ) {
delegate.initializeObjects(
remainingEntityInfos.toArray( new EntityInfo[remainingSize] ),
remainingEntityInfos,
idToObjectMap,
objectInitializationContext
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public class ObjectLookupAndDatabaseRetrievalConfigurationTest {

@Test
@BMRule(targetClass = "org.hibernate.search.query.hibernate.impl.CriteriaObjectInitializer",
targetMethod = "initializeObjects(org.hibernate.search.query.engine.spi.EntityInfo[], java.util.LinkedHashMap, org.hibernate.search.query.hibernate.impl.ObjectInitializationContext)",
targetMethod = "initializeObjects(java.util.List, java.util.LinkedHashMap, org.hibernate.search.query.hibernate.impl.ObjectInitializationContext)",
helper = "org.hibernate.search.testsupport.BytemanHelper",
action = "countInvocation()",
name = "testSetLookupMethodPersistenceContext")
Expand All @@ -65,7 +65,7 @@ public void testDefaultLookupMethod() throws Exception {

@Test
@BMRule(targetClass = "org.hibernate.search.query.hibernate.impl.PersistenceContextObjectInitializer",
targetMethod = "initializeObjects(org.hibernate.search.query.engine.spi.EntityInfo[], java.util.LinkedHashMap, org.hibernate.search.query.hibernate.impl.ObjectInitializationContext)",
targetMethod = "initializeObjects(java.util.List, java.util.LinkedHashMap, org.hibernate.search.query.hibernate.impl.ObjectInitializationContext)",
helper = "org.hibernate.search.testsupport.BytemanHelper",
action = "countInvocation()",
name = "testSetLookupMethodPersistenceContext")
Expand All @@ -89,7 +89,7 @@ public void testSetLookupMethodPersistenceContextUpperCase() throws Exception {

@Test
@BMRule(targetClass = "org.hibernate.search.query.hibernate.impl.PersistenceContextObjectInitializer",
targetMethod = "initializeObjects(org.hibernate.search.query.engine.spi.EntityInfo[], java.util.LinkedHashMap, org.hibernate.search.query.hibernate.impl.ObjectInitializationContext)",
targetMethod = "initializeObjects(java.util.List, java.util.LinkedHashMap, org.hibernate.search.query.hibernate.impl.ObjectInitializationContext)",
helper = "org.hibernate.search.testsupport.BytemanHelper",
action = "countInvocation()",
name = "testSetLookupMethodPersistenceContext")
Expand All @@ -113,7 +113,7 @@ public void testSetLookupMethodPersistenceContextLowerCase() throws Exception {

@Test
@BMRule(targetClass = "org.hibernate.search.query.hibernate.impl.LookupObjectInitializer",
targetMethod = "initializeObjects(org.hibernate.search.query.engine.spi.EntityInfo[], java.util.LinkedHashMap, org.hibernate.search.query.hibernate.impl.ObjectInitializationContext)",
targetMethod = "initializeObjects(java.util.List, java.util.LinkedHashMap, org.hibernate.search.query.hibernate.impl.ObjectInitializationContext)",
helper = "org.hibernate.search.testsupport.BytemanHelper",
action = "countInvocation()",
name = "testSetLookupMethodPersistenceContext")
Expand All @@ -137,7 +137,7 @@ public void testSetDatabaseRetrievalMethodUpperCase() throws Exception {

@Test
@BMRule(targetClass = "org.hibernate.search.query.hibernate.impl.LookupObjectInitializer",
targetMethod = "initializeObjects(org.hibernate.search.query.engine.spi.EntityInfo[], java.util.LinkedHashMap, org.hibernate.search.query.hibernate.impl.ObjectInitializationContext)",
targetMethod = "initializeObjects(java.util.List, java.util.LinkedHashMap, org.hibernate.search.query.hibernate.impl.ObjectInitializationContext)",
helper = "org.hibernate.search.testsupport.BytemanHelper",
action = "countInvocation()",
name = "testSetLookupMethodPersistenceContext")
Expand Down

0 comments on commit 68cfda9

Please sign in to comment.