From f89bf35106d834415432252be120146abf52ffa1 Mon Sep 17 00:00:00 2001 From: Sanne Grinovero Date: Wed, 30 Oct 2019 10:27:12 +0000 Subject: [PATCH] HHH-13654 Make AbstractFlushingEventListener#entitiesByKey also lazily initialized --- .../internal/StatefulPersistenceContext.java | 83 ++++++++++++------- .../engine/spi/PersistenceContext.java | 10 +++ .../AbstractFlushingEventListener.java | 6 +- .../util/collections/LazyIterator.java | 40 --------- ...StatelessSessionPersistentContextTest.java | 9 ++ 5 files changed, 77 insertions(+), 71 deletions(-) delete mode 100755 hibernate-core/src/main/java/org/hibernate/internal/util/collections/LazyIterator.java diff --git a/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java b/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java index 8b0e547ff6..1364becc38 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/internal/StatefulPersistenceContext.java @@ -95,7 +95,7 @@ public class StatefulPersistenceContext implements PersistenceContext { private SharedSessionContractImplementor session; // Loaded entity instances, by EntityKey - private Map entitiesByKey; + private HashMap entitiesByKey; // Loaded entity instances, by EntityUniqueKey private Map entitiesByUniqueKey; @@ -155,8 +155,6 @@ public class StatefulPersistenceContext implements PersistenceContext { */ public StatefulPersistenceContext(SharedSessionContractImplementor session) { this.session = session; - - entitiesByKey = new HashMap<>( INIT_COLL_SIZE ); entityEntryContext = new EntityEntryContext( this ); } @@ -239,7 +237,7 @@ public class StatefulPersistenceContext implements PersistenceContext { } arrayHolders = null; - entitiesByKey.clear(); + entitiesByKey = null; entitiesByUniqueKey = null; entityEntryContext.clear(); parentsByChild = null; @@ -383,34 +381,43 @@ public class StatefulPersistenceContext implements PersistenceContext { @Override public void addEntity(EntityKey key, Object entity) { + if ( entitiesByKey == null ) { + entitiesByKey = new HashMap<>( INIT_COLL_SIZE ); + } entitiesByKey.put( key, entity ); final BatchFetchQueue fetchQueue = this.batchFetchQueue; if ( fetchQueue != null ) { - fetchQueue.removeBatchLoadableEntityKey(key); + fetchQueue.removeBatchLoadableEntityKey( key ); } } @Override public Object getEntity(EntityKey key) { - return entitiesByKey.get( key ); + return entitiesByKey == null ? null : entitiesByKey.get( key ); } @Override public boolean containsEntity(EntityKey key) { - return entitiesByKey.containsKey( key ); + return entitiesByKey == null ? false : entitiesByKey.containsKey( key ); } @Override public Object removeEntity(EntityKey key) { - final Object entity = entitiesByKey.remove( key ); - if ( entitiesByUniqueKey != null ) { - final Iterator itr = entitiesByUniqueKey.values().iterator(); - while ( itr.hasNext() ) { - if ( itr.next() == entity ) { - itr.remove(); + final Object entity; + if ( entitiesByKey != null ) { + entity = entitiesByKey.remove( key ); + if ( entitiesByUniqueKey != null ) { + final Iterator itr = entitiesByUniqueKey.values().iterator(); + while ( itr.hasNext() ) { + if ( itr.next() == entity ) { + itr.remove(); + } } } } + else { + entity = null; + } // Clear all parent cache parentsByChild = null; @@ -757,6 +764,9 @@ public class StatefulPersistenceContext implements PersistenceContext { @Override public void addEnhancedProxy(EntityKey key, PersistentAttributeInterceptable entity) { + if ( entitiesByKey == null ) { + entitiesByKey = new HashMap<>( INIT_COLL_SIZE ); + } entitiesByKey.put( key, entity ); } @@ -1062,9 +1072,25 @@ public class StatefulPersistenceContext implements PersistenceContext { return nullifiableEntityKeys; } + /** + * @deprecated this will be removed: it provides too wide access, making it hard to optimise the internals + * for specific access needs. Consider using #iterateEntities instead. + * @return + */ + @Deprecated @Override public Map getEntitiesByKey() { - return entitiesByKey; + return entitiesByKey == null ? Collections.emptyMap() : entitiesByKey; + } + + @Override + public Iterator managedEntitiesIterator() { + if ( entitiesByKey == null ) { + return Collections.emptyIterator(); + } + else { + return entitiesByKey.values().iterator(); + } } @Override @@ -1200,13 +1226,9 @@ public class StatefulPersistenceContext implements PersistenceContext { @Override public String toString() { - if ( collectionsByKey == null ) { - return "PersistenceContext[entityKeys=" + entitiesByKey.keySet() + ",collectionKeys=[]]"; - } - else { - return "PersistenceContext[entityKeys=" + entitiesByKey.keySet() - + ",collectionKeys=" + collectionsByKey.keySet() + "]"; - } + final String entityKeySet = entitiesByKey == null ? "[]" : entitiesByKey.keySet().toString(); + final String collectionsKeySet = collectionsByKey == null ? "[]" : collectionsByKey.keySet().toString(); + return "PersistenceContext[entityKeys=" + entityKeySet + ", collectionKeys=" + collectionsKeySet + "]"; } @Override @@ -1503,7 +1525,7 @@ public class StatefulPersistenceContext implements PersistenceContext { @Override public void replaceDelayedEntityIdentityInsertKeys(EntityKey oldKey, Serializable generatedId) { - final Object entity = entitiesByKey.remove( oldKey ); + final Object entity = entitiesByKey == null ? null : entitiesByKey.remove( oldKey ); final EntityEntry oldEntry = entityEntryContext.removeEntityEntry( entity ); this.parentsByChild = null; @@ -1536,13 +1558,18 @@ public class StatefulPersistenceContext implements PersistenceContext { oos.writeBoolean( defaultReadOnly ); oos.writeBoolean( hasNonReadOnlyEntities ); - oos.writeInt( entitiesByKey.size() ); - if ( LOG.isTraceEnabled() ) { - LOG.trace( "Starting serialization of [" + entitiesByKey.size() + "] entitiesByKey entries" ); + if ( entitiesByKey == null ) { + oos.writeInt( 0 ); } - for ( Map.Entry entry : entitiesByKey.entrySet() ) { - entry.getKey().serialize( oos ); - oos.writeObject( entry.getValue() ); + else { + oos.writeInt( entitiesByKey.size() ); + if ( LOG.isTraceEnabled() ) { + LOG.trace( "Starting serialization of [" + entitiesByKey.size() + "] entitiesByKey entries" ); + } + for ( Map.Entry entry : entitiesByKey.entrySet() ) { + entry.getKey().serialize( oos ); + oos.writeObject( entry.getValue() ); + } } if ( entitiesByUniqueKey == null ) { diff --git a/hibernate-core/src/main/java/org/hibernate/engine/spi/PersistenceContext.java b/hibernate-core/src/main/java/org/hibernate/engine/spi/PersistenceContext.java index d32d492664..b90ff597b3 100644 --- a/hibernate-core/src/main/java/org/hibernate/engine/spi/PersistenceContext.java +++ b/hibernate-core/src/main/java/org/hibernate/engine/spi/PersistenceContext.java @@ -9,6 +9,7 @@ package org.hibernate.engine.spi; import java.io.Serializable; import java.util.Collection; import java.util.HashSet; +import java.util.Iterator; import java.util.Map; import java.util.function.BiConsumer; import java.util.function.Supplier; @@ -486,7 +487,10 @@ public interface PersistenceContext { /** * Get the mapping from key value to entity instance + * @deprecated this will be removed: it provides too wide access, making it hard to optimise the internals + * for specific access needs. Consider using #iterateEntities instead. */ + @Deprecated Map getEntitiesByKey(); /** @@ -793,6 +797,12 @@ public interface PersistenceContext { */ void removeCollectionByKey(CollectionKey collectionKey); + /** + * A read-only iterator on all entities managed by this persistence context + * @return + */ + Iterator managedEntitiesIterator(); + /** * Provides centralized access to natural-id-related functionality. */ diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractFlushingEventListener.java b/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractFlushingEventListener.java index 632ed591e8..162af5e1a3 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractFlushingEventListener.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/AbstractFlushingEventListener.java @@ -37,7 +37,6 @@ import org.hibernate.event.spi.FlushEntityEventListener; import org.hibernate.event.spi.FlushEvent; import org.hibernate.internal.CoreMessageLogger; import org.hibernate.internal.util.EntityPrinter; -import org.hibernate.internal.util.collections.LazyIterator; import org.hibernate.persister.entity.EntityPersister; import org.jboss.logging.Logger; @@ -77,7 +76,7 @@ public abstract class AbstractFlushingEventListener implements JpaBootstrapSensi EventSource session = event.getSession(); final PersistenceContext persistenceContext = session.getPersistenceContextInternal(); - session.getInterceptor().preFlush( new LazyIterator( persistenceContext.getEntitiesByKey() ) ); + session.getInterceptor().preFlush( persistenceContext.managedEntitiesIterator() ); prepareEntityFlushes( session, persistenceContext ); // we could move this inside if we wanted to @@ -397,6 +396,7 @@ public abstract class AbstractFlushingEventListener implements JpaBootstrapSensi } protected void postPostFlush(SessionImplementor session) { - session.getInterceptor().postFlush( new LazyIterator( session.getPersistenceContextInternal().getEntitiesByKey() ) ); + session.getInterceptor().postFlush( session.getPersistenceContextInternal().managedEntitiesIterator() ); } + } diff --git a/hibernate-core/src/main/java/org/hibernate/internal/util/collections/LazyIterator.java b/hibernate-core/src/main/java/org/hibernate/internal/util/collections/LazyIterator.java deleted file mode 100755 index 5d6cde5ca1..0000000000 --- a/hibernate-core/src/main/java/org/hibernate/internal/util/collections/LazyIterator.java +++ /dev/null @@ -1,40 +0,0 @@ -/* - * 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 . - */ -package org.hibernate.internal.util.collections; - -import java.util.Iterator; -import java.util.Map; - -public final class LazyIterator implements Iterator { - - private final Map map; - private Iterator iterator; - - private Iterator getIterator() { - if (iterator==null) { - iterator = map.values().iterator(); - } - return iterator; - } - - public LazyIterator(Map map) { - this.map = map; - } - - public boolean hasNext() { - return getIterator().hasNext(); - } - - public Object next() { - return getIterator().next(); - } - - public void remove() { - throw new UnsupportedOperationException(); - } - -} diff --git a/hibernate-core/src/test/java/org/hibernate/test/stateless/StatelessSessionPersistentContextTest.java b/hibernate-core/src/test/java/org/hibernate/test/stateless/StatelessSessionPersistentContextTest.java index ed007f2ad8..8ec58b38fc 100644 --- a/hibernate-core/src/test/java/org/hibernate/test/stateless/StatelessSessionPersistentContextTest.java +++ b/hibernate-core/src/test/java/org/hibernate/test/stateless/StatelessSessionPersistentContextTest.java @@ -6,6 +6,7 @@ */ package org.hibernate.test.stateless; +import java.util.Collections; import java.util.function.Consumer; import javax.persistence.Entity; import javax.persistence.GeneratedValue; @@ -115,10 +116,18 @@ public class StatelessSessionPersistentContextTest extends BaseCoreFunctionalTes "StatelessSession: PersistenceContext has not been cleared", persistenceContextInternal.getEntitiesByKey().isEmpty() ); + assertTrue( + "StatelessSession: PersistenceContext has not been cleared", + persistenceContextInternal.managedEntitiesIterator() == Collections.emptyIterator() + ); assertTrue( "StatelessSession: PersistenceContext has not been cleared", persistenceContextInternal.getCollectionsByKey().isEmpty() ); + assertTrue( + "StatelessSession: PersistenceContext has not been cleared", + persistenceContextInternal.getCollectionsByKey() == Collections.emptyMap() + ); } @Entity(name = "TestEntity")