diff --git a/hibernate-core/src/main/java/org/hibernate/collection/spi/AbstractPersistentCollection.java b/hibernate-core/src/main/java/org/hibernate/collection/spi/AbstractPersistentCollection.java index bfe295e77d6a..136c07d8b94c 100644 --- a/hibernate-core/src/main/java/org/hibernate/collection/spi/AbstractPersistentCollection.java +++ b/hibernate-core/src/main/java/org/hibernate/collection/spi/AbstractPersistentCollection.java @@ -79,6 +79,8 @@ public abstract class AbstractPersistentCollection implements Serializable, P private String sessionFactoryUuid; private boolean allowLoadOutsideTransaction; + private int instanceId; + /** * Not called by Hibernate, but used by non-JDK serialization, * eg. SOAP libraries. @@ -1362,4 +1364,13 @@ public void setOwner(Object owner) { this.owner = owner; } + @Override + public int $$_hibernate_getInstanceId() { + return instanceId; + } + + @Override + public void $$_hibernate_setInstanceId(int instanceId) { + this.instanceId = instanceId; + } } diff --git a/hibernate-core/src/main/java/org/hibernate/collection/spi/PersistentCollection.java b/hibernate-core/src/main/java/org/hibernate/collection/spi/PersistentCollection.java index 68df77b5aab1..89379aa9e93d 100644 --- a/hibernate-core/src/main/java/org/hibernate/collection/spi/PersistentCollection.java +++ b/hibernate-core/src/main/java/org/hibernate/collection/spi/PersistentCollection.java @@ -11,6 +11,7 @@ import org.hibernate.HibernateException; import org.hibernate.Incubating; +import org.hibernate.engine.spi.InstanceIdentity; import org.hibernate.engine.spi.SharedSessionContractImplementor; import org.hibernate.metamodel.mapping.PluralAttributeMapping; import org.hibernate.persister.collection.CollectionPersister; @@ -52,7 +53,7 @@ * @author Gavin King */ @Incubating -public interface PersistentCollection extends LazyInitializable { +public interface PersistentCollection extends LazyInitializable, InstanceIdentity { /** * Get the owning entity. Note that the owner is only * set during the flush cycle, and when a new collection 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 bcde88fe6b5f..107201f9fa2e 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 @@ -55,7 +55,8 @@ import org.hibernate.event.spi.PostLoadEventListener; import org.hibernate.internal.CoreMessageLogger; import org.hibernate.internal.util.collections.CollectionHelper; -import org.hibernate.internal.util.collections.IdentityMap; +import org.hibernate.internal.util.collections.InstanceIdentityMap; +import org.hibernate.internal.util.collections.StandardStack; import org.hibernate.metamodel.spi.MappingMetamodelImplementor; import org.hibernate.persister.collection.CollectionPersister; import org.hibernate.persister.entity.EntityPersister; @@ -131,7 +132,11 @@ the following fields are used in all circumstances, and are not worth (or not su private IdentityHashMap> arrayHolders; // Identity map of CollectionEntry instances, by the collection wrapper - private IdentityMap, CollectionEntry> collectionEntries; + private InstanceIdentityMap, CollectionEntry> collectionEntries; + + // current collection instance id and stack of reusable ones + private StandardStack reusableCollectionInstanceIds = null; + private int currentCollectionInstanceId; // Collection wrappers, by the CollectionKey private HashMap> collectionsByKey; @@ -255,7 +260,7 @@ public void clear() { final SharedSessionContractImplementor session = getSession(); if ( collectionEntries != null ) { - IdentityMap.onEachKey( collectionEntries, k -> k.unsetSession( session ) ); + collectionEntries.forEach( (k, v) -> k.unsetSession( session ) ); } arrayHolders = null; @@ -267,6 +272,8 @@ public void clear() { collectionsByKey = null; nonlazyCollections = null; collectionEntries = null; + currentCollectionInstanceId = 0; + reusableCollectionInstanceIds = null; unownedCollections = null; nullifiableEntityKeys = null; deletedUnloadedEntityKeys = null; @@ -1082,7 +1089,6 @@ public void replaceCollection(CollectionPersister persister, PersistentCollectio "Replacement of not directly accessible collection found: " + oldCollection.getRole() ); } assert !collection.isDirectlyAccessible(); - final IdentityMap, CollectionEntry> collectionEntries = getOrInitializeCollectionEntries(); final CollectionEntry oldEntry = collectionEntries.remove( oldCollection ); final CollectionEntry entry; if ( oldEntry.getLoadedPersister() != null ) { @@ -1093,7 +1099,7 @@ public void replaceCollection(CollectionPersister persister, PersistentCollectio // A newly wrapped collection entry = new CollectionEntry( persister, collection ); } - collectionEntries.put( collection, entry ); + putCollectionEntry( collection, entry ); final Object key = collection.getKey(); if ( key != null ) { final CollectionKey collectionKey = new CollectionKey( entry.getLoadedPersister(), key ); @@ -1112,7 +1118,7 @@ public void replaceCollection(CollectionPersister persister, PersistentCollectio * @param key The key of the collection's entry. */ private void addCollection(PersistentCollection coll, CollectionEntry entry, Object key) { - getOrInitializeCollectionEntries().put( coll, entry ); + putCollectionEntry( coll, entry ); final CollectionKey collectionKey = new CollectionKey( entry.getLoadedPersister(), key ); final PersistentCollection old = addCollectionByKey( collectionKey, coll ); if ( old != null ) { @@ -1125,11 +1131,12 @@ private void addCollection(PersistentCollection coll, CollectionEntry entry, } } - private IdentityMap, CollectionEntry> getOrInitializeCollectionEntries() { + private void putCollectionEntry(PersistentCollection collection, CollectionEntry entry) { if ( this.collectionEntries == null ) { - this.collectionEntries = IdentityMap.instantiateSequenced( INIT_COLL_SIZE ); + this.collectionEntries = new InstanceIdentityMap<>(); } - return this.collectionEntries; + collection.$$_hibernate_setInstanceId( nextCollectionInstanceId() ); + this.collectionEntries.put( collection, entry ); } /** @@ -1140,7 +1147,7 @@ private IdentityMap, CollectionEntry> getOrInitializeCol */ private void addCollection(PersistentCollection collection, CollectionPersister persister) { final CollectionEntry ce = new CollectionEntry( persister, collection ); - getOrInitializeCollectionEntries().put( collection, ce ); + putCollectionEntry( collection, ce ); } @Override @@ -1375,11 +1382,17 @@ public int getNumberOfManagedEntities() { return collectionEntries; } + private int nextCollectionInstanceId() { + return reusableCollectionInstanceIds != null && !reusableCollectionInstanceIds.isEmpty() ? + reusableCollectionInstanceIds.pop() : + currentCollectionInstanceId++; + } + @Override public void forEachCollectionEntry(BiConsumer, CollectionEntry> action, boolean concurrent) { if ( collectionEntries != null ) { if ( concurrent ) { - for ( Entry,CollectionEntry> entry : IdentityMap.concurrentEntries( collectionEntries ) ) { + for ( Entry,CollectionEntry> entry : collectionEntries.toArray() ) { action.accept( entry.getKey(), entry.getValue() ); } } @@ -2032,7 +2045,7 @@ public static StatefulPersistenceContext deserialize( final PersistentCollection pc = (PersistentCollection) ois.readObject(); final CollectionEntry ce = CollectionEntry.deserialize( ois, session ); pc.setCurrentSession( session ); - rtn.getOrInitializeCollectionEntries().put( pc, ce ); + rtn.putCollectionEntry( pc, ce ); } count = ois.readInt(); @@ -2174,7 +2187,17 @@ public int getCollectionEntriesSize() { @Override public CollectionEntry removeCollectionEntry(PersistentCollection collection) { - return collectionEntries == null ? null : collectionEntries.remove(collection); + if ( collectionEntries != null ) { + final int instanceId = collection.$$_hibernate_getInstanceId(); + if ( reusableCollectionInstanceIds == null ) { + reusableCollectionInstanceIds = new StandardStack<>(); + } + reusableCollectionInstanceIds.push( instanceId ); + return collectionEntries.remove( collection ); + } + else { + return null; + } } @Override 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 3763044a0649..f4bd9266391f 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 @@ -34,7 +34,7 @@ import org.hibernate.event.spi.PersistContext; import org.hibernate.internal.CoreMessageLogger; import org.hibernate.internal.util.EntityPrinter; -import org.hibernate.internal.util.collections.IdentityMap; +import org.hibernate.internal.util.collections.InstanceIdentityMap; import org.hibernate.persister.entity.EntityPersister; import org.jboss.logging.Logger; @@ -190,7 +190,7 @@ private void prepareCollectionFlushes(PersistenceContext persistenceContext) thr persistenceContext.getCollectionEntries(); if ( collectionEntries != null ) { for ( Map.Entry, CollectionEntry> entry : - ( (IdentityMap, CollectionEntry>) collectionEntries ).entryArray() ) { + ( (InstanceIdentityMap, CollectionEntry>) collectionEntries ).toArray() ) { entry.getValue().preFlush( entry.getKey() ); } } @@ -271,7 +271,7 @@ private int flushCollections(final EventSource session, final PersistenceContext } else { count = collectionEntries.size(); - for ( Map.Entry, CollectionEntry> me : ( (IdentityMap, CollectionEntry>) collectionEntries ).entryArray() ) { + for ( Map.Entry, CollectionEntry> me : ( (InstanceIdentityMap, CollectionEntry>) collectionEntries ).toArray() ) { final CollectionEntry ce = me.getValue(); if ( !ce.isReached() && !ce.isIgnore() ) { Collections.processUnreachableCollection( me.getKey(), session ); diff --git a/hibernate-core/src/test/java/org/hibernate/orm/test/pc/InstanceIdentityTest.java b/hibernate-core/src/test/java/org/hibernate/orm/test/pc/InstanceIdentityTest.java new file mode 100644 index 000000000000..fe7f5c15aa26 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/orm/test/pc/InstanceIdentityTest.java @@ -0,0 +1,187 @@ +/* + * SPDX-License-Identifier: LGPL-2.1-or-later + * Copyright Red Hat Inc. and Hibernate Authors + */ +package org.hibernate.orm.test.pc; + +import jakarta.persistence.ElementCollection; +import jakarta.persistence.Entity; +import jakarta.persistence.Id; +import jakarta.persistence.OneToMany; +import org.hibernate.annotations.Immutable; +import org.hibernate.collection.spi.PersistentCollection; +import org.hibernate.engine.spi.ManagedEntity; +import org.hibernate.testing.bytecode.enhancement.extension.BytecodeEnhanced; +import org.hibernate.testing.orm.junit.DomainModel; +import org.hibernate.testing.orm.junit.SessionFactory; +import org.hibernate.testing.orm.junit.SessionFactoryScope; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import static org.assertj.core.api.Assertions.assertThat; + +@DomainModel(annotatedClasses = { + InstanceIdentityTest.ImmutableEntity.class, + InstanceIdentityTest.EntityWithCollections.class, +}) +@SessionFactory +@BytecodeEnhanced +public class InstanceIdentityTest { + @Test + public void testEnhancedImmutableEntity(SessionFactoryScope scope) { + scope.inTransaction( session -> { + final ImmutableEntity entity1 = new ImmutableEntity( 1, "entity_1" ); + session.persist( entity1 ); + + // false warning, bytecode enhancement of the test class will make the cast work + assertThat( ((ManagedEntity) entity1).$$_hibernate_getInstanceId() ).isGreaterThanOrEqualTo( 0 ); + assertThat( session.contains( entity1 ) ).isTrue(); + + final ImmutableEntity entity2 = new ImmutableEntity( 2, "entity_2" ); + session.persist( entity2 ); + final ImmutableEntity entity3 = new ImmutableEntity( 3, "entity_3" ); + session.persist( entity3 ); + } ); + + scope.inSession( session -> { + assertThat( session.find( ImmutableEntity.class, 1 ) ).isNotNull().extracting( ImmutableEntity::getName ) + .isEqualTo( "entity_1" ); + + final List immutableEntities = session.createQuery( + "from ImmutableEntity", + ImmutableEntity.class + ).getResultList(); + + assertThat( immutableEntities ).hasSize( 3 ); + + // test find again, this time from 1st level cache + final ImmutableEntity entity2 = session.find( ImmutableEntity.class, 2 ); + assertThat( entity2 ).isNotNull().extracting( ImmutableEntity::getName ) + .isEqualTo( "entity_2" ); + + session.detach( entity2 ); + assertThat( session.contains( entity2 ) ).isFalse(); + } ); + } + + @Test + public void testPersistentCollections(SessionFactoryScope scope) { + scope.inTransaction( session -> { + final ImmutableEntity immutableEntity = new ImmutableEntity( 4, "entity_4" ); + session.persist( immutableEntity ); + final EntityWithCollections entity = new EntityWithCollections( 1 ); + entity.getStringList().addAll( List.of( "one", "two" ) ); + entity.getEntityMap().put( "4", immutableEntity ); + session.persist( entity ); + + assertThat( entity.getStringList() ).isInstanceOf( PersistentCollection.class ); + final PersistentCollection persistentList = (PersistentCollection) entity.getStringList(); + assertThat( persistentList.$$_hibernate_getInstanceId() ).isGreaterThanOrEqualTo( 0 ); + + assertThat( entity.getEntityMap() ).isInstanceOf( PersistentCollection.class ); + final PersistentCollection persistentMap = (PersistentCollection) entity.getEntityMap(); + assertThat( persistentMap.$$_hibernate_getInstanceId() ).isGreaterThanOrEqualTo( 0 ); + + assertThat( session.getPersistenceContextInternal().getCollectionEntries() ).isNotNull() + .containsKeys( persistentList, persistentMap ); + } ); + + scope.inTransaction( session -> { + final EntityWithCollections entity = session.find( EntityWithCollections.class, 1 ); + entity.getStringList().add( "three" ); + entity.getEntityMap().clear(); + } ); + + scope.inSession( session -> { + final EntityWithCollections entity = session.find( EntityWithCollections.class, 1 ); + assertThat( entity.getStringList() ).hasSize( 3 ).containsExactly( "one", "two", "three" ); + assertThat( entity.getEntityMap() ).isEmpty(); + } ); + } + + @AfterEach + public void tearDown(SessionFactoryScope scope) { + scope.getSessionFactory().getSchemaManager().truncateMappedObjects(); + } + + @Immutable + @Entity(name = "ImmutableEntity") + static class ImmutableEntity { + @Id + private Integer id; + + private String name; + + public ImmutableEntity() { + } + + public ImmutableEntity(Integer id, String name) { + this.id = id; + this.name = name; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + } + + @Entity(name = "EntityWithCollections") + static class EntityWithCollections { + @Id + private Integer id; + + @ElementCollection + private List stringList = new ArrayList<>(); + + @OneToMany + private Map entityMap = new HashMap<>(); + + public EntityWithCollections() { + } + + public EntityWithCollections(Integer id) { + this.id = id; + } + + public Integer getId() { + return id; + } + + public void setId(Integer id) { + this.id = id; + } + + public List getStringList() { + return stringList; + } + + public void setStringList(List stringList) { + this.stringList = stringList; + } + + public Map getEntityMap() { + return entityMap; + } + + public void setEntityMap(Map entityMap) { + this.entityMap = entityMap; + } + } +}