Skip to content

Commit

Permalink
HHH-18326 Use instance identity to track persistent collections
Browse files Browse the repository at this point in the history
  • Loading branch information
mbladel committed Jan 21, 2025
1 parent 62141c8 commit 010c6dc
Show file tree
Hide file tree
Showing 5 changed files with 239 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ public abstract class AbstractPersistentCollection<E> 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.
Expand Down Expand Up @@ -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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -52,7 +53,7 @@
* @author Gavin King
*/
@Incubating
public interface PersistentCollection<E> extends LazyInitializable {
public interface PersistentCollection<E> extends LazyInitializable, InstanceIdentity {
/**
* Get the owning entity. Note that the owner is only
* set during the flush cycle, and when a new collection
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -131,7 +132,11 @@ the following fields are used in all circumstances, and are not worth (or not su
private IdentityHashMap<Object, PersistentCollection<?>> arrayHolders;

// Identity map of CollectionEntry instances, by the collection wrapper
private IdentityMap<PersistentCollection<?>, CollectionEntry> collectionEntries;
private InstanceIdentityMap<PersistentCollection<?>, CollectionEntry> collectionEntries;

// current collection instance id and stack of reusable ones
private StandardStack<Integer> reusableCollectionInstanceIds = null;
private int currentCollectionInstanceId;

// Collection wrappers, by the CollectionKey
private HashMap<CollectionKey, PersistentCollection<?>> collectionsByKey;
Expand Down Expand Up @@ -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;
Expand All @@ -267,6 +272,8 @@ public void clear() {
collectionsByKey = null;
nonlazyCollections = null;
collectionEntries = null;
currentCollectionInstanceId = 0;
reusableCollectionInstanceIds = null;
unownedCollections = null;
nullifiableEntityKeys = null;
deletedUnloadedEntityKeys = null;
Expand Down Expand Up @@ -1082,7 +1089,6 @@ public void replaceCollection(CollectionPersister persister, PersistentCollectio
"Replacement of not directly accessible collection found: " + oldCollection.getRole() );
}
assert !collection.isDirectlyAccessible();
final IdentityMap<PersistentCollection<?>, CollectionEntry> collectionEntries = getOrInitializeCollectionEntries();
final CollectionEntry oldEntry = collectionEntries.remove( oldCollection );
final CollectionEntry entry;
if ( oldEntry.getLoadedPersister() != null ) {
Expand All @@ -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 );
Expand All @@ -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 ) {
Expand All @@ -1125,11 +1131,12 @@ private void addCollection(PersistentCollection<?> coll, CollectionEntry entry,
}
}

private IdentityMap<PersistentCollection<?>, 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 );
}

/**
Expand All @@ -1140,7 +1147,7 @@ private IdentityMap<PersistentCollection<?>, CollectionEntry> getOrInitializeCol
*/
private void addCollection(PersistentCollection<?> collection, CollectionPersister persister) {
final CollectionEntry ce = new CollectionEntry( persister, collection );
getOrInitializeCollectionEntries().put( collection, ce );
putCollectionEntry( collection, ce );
}

@Override
Expand Down Expand Up @@ -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<PersistentCollection<?>, CollectionEntry> action, boolean concurrent) {
if ( collectionEntries != null ) {
if ( concurrent ) {
for ( Entry<PersistentCollection<?>,CollectionEntry> entry : IdentityMap.concurrentEntries( collectionEntries ) ) {
for ( Entry<PersistentCollection<?>,CollectionEntry> entry : collectionEntries.toArray() ) {
action.accept( entry.getKey(), entry.getValue() );
}
}
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -190,7 +190,7 @@ private void prepareCollectionFlushes(PersistenceContext persistenceContext) thr
persistenceContext.getCollectionEntries();
if ( collectionEntries != null ) {
for ( Map.Entry<PersistentCollection<?>, CollectionEntry> entry :
( (IdentityMap<PersistentCollection<?>, CollectionEntry>) collectionEntries ).entryArray() ) {
( (InstanceIdentityMap<PersistentCollection<?>, CollectionEntry>) collectionEntries ).toArray() ) {
entry.getValue().preFlush( entry.getKey() );
}
}
Expand Down Expand Up @@ -271,7 +271,7 @@ private int flushCollections(final EventSource session, final PersistenceContext
}
else {
count = collectionEntries.size();
for ( Map.Entry<PersistentCollection<?>, CollectionEntry> me : ( (IdentityMap<PersistentCollection<?>, CollectionEntry>) collectionEntries ).entryArray() ) {
for ( Map.Entry<PersistentCollection<?>, CollectionEntry> me : ( (InstanceIdentityMap<PersistentCollection<?>, CollectionEntry>) collectionEntries ).toArray() ) {
final CollectionEntry ce = me.getValue();
if ( !ce.isReached() && !ce.isIgnore() ) {
Collections.processUnreachableCollection( me.getKey(), session );
Expand Down
Original file line number Diff line number Diff line change
@@ -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<ImmutableEntity> 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<String> stringList = new ArrayList<>();

@OneToMany
private Map<String, ImmutableEntity> 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<String> getStringList() {
return stringList;
}

public void setStringList(List<String> stringList) {
this.stringList = stringList;
}

public Map<String, ImmutableEntity> getEntityMap() {
return entityMap;
}

public void setEntityMap(Map<String, ImmutableEntity> entityMap) {
this.entityMap = entityMap;
}
}
}

0 comments on commit 010c6dc

Please sign in to comment.