HHH-14620 Do not initialize collections just to take a snapshot of their size

As far as I can tell this is safe enough for dirty tracking:

* The collection tracker will return a size of -1 for that collection
* Which is exactly the behavior we currently get after
  $$_hibernane_clearDirtyCollectionNames is called if a collection has
  been "retrieved" (getter called) but was not initialized.
* This will mainly prevent some optimizations because we will no longer
  be able to tell whether a collection is "dirty" or not.

I think we should be able to restore those optimizations: for
PersistentCollection instances, we would store the "initial" size
inside the collection itself upon initialization,
and we would compare THAT size to the current size in implementations
of $$_hibernate_areCollectionFieldsDirty (see
org.hibernate.bytecode.enhance.internal.bytebuddy.CodeTemplates).

Alternatively we could store the CollectionTracker inside the
PersistentCollection so that the collection can update the tracker
upon initialization.

However, that's outside the scope of this bug, that would require
significant testing, and that may cause conflicts with ORM 6, so I won't
do it here.
This commit is contained in:
Yoann Rodière 2021-05-19 16:25:35 +02:00
parent eb6c68cdc6
commit b9270e44b1
2 changed files with 22 additions and 12 deletions

View File

@ -16,6 +16,7 @@ import java.util.Set;
import org.hibernate.LockMode;
import org.hibernate.bytecode.enhance.spi.CollectionTracker;
import org.hibernate.bytecode.enhance.spi.LazyPropertyInitializer;
import org.hibernate.collection.spi.PersistentCollection;
import org.hibernate.engine.spi.SelfDirtinessTracker;
import org.hibernate.engine.spi.SharedSessionContractImplementor;
import org.hibernate.engine.spi.Status;
@ -152,11 +153,18 @@ public class LazyAttributeLoadingInterceptor extends AbstractLazyLoadInterceptor
private void takeCollectionSizeSnapshot(Object target, String fieldName, Object value) {
if ( value instanceof Collection && target instanceof SelfDirtinessTracker ) {
// This must be called first, so that we remember that there is a collection out there,
// even if we don't know its size (see below).
CollectionTracker tracker = ( (SelfDirtinessTracker) target ).$$_hibernate_getCollectionTracker();
if ( tracker == null ) {
( (SelfDirtinessTracker) target ).$$_hibernate_clearDirtyAttributes();
tracker = ( (SelfDirtinessTracker) target ).$$_hibernate_getCollectionTracker();
}
if ( value instanceof PersistentCollection && !( (PersistentCollection) value ).wasInitialized() ) {
// Cannot take a snapshot of an un-initialized collection.
return;
}
tracker.add( fieldName, ( (Collection) value ).size() );
}
}

View File

@ -119,16 +119,17 @@ public class OnDemandLoadTest extends BaseCoreFunctionalTestCase {
store.getInventories().size();
assertTrue( isPropertyInitialized( store, "inventories" ) );
// the extra Session is the temp Session needed to perform the init
assertEquals( 2, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 1, sessionFactory().getStatistics().getSessionCloseCount() );
// the extra Sessions are the temp Sessions needed to perform the init:
// first the entity, then the collection (since it's lazy)
assertEquals( 3, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 2, sessionFactory().getStatistics().getSessionCloseCount() );
// clear Session again. The collection should still be recognized as initialized from above
s.clear();
assertNotNull( store );
assertTrue( isPropertyInitialized( store, "inventories" ) );
assertEquals( 2, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 1, sessionFactory().getStatistics().getSessionCloseCount() );
assertEquals( 3, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 2, sessionFactory().getStatistics().getSessionCloseCount() );
// lets clear the Session again and this time reload the Store
s.clear();
@ -138,21 +139,22 @@ public class OnDemandLoadTest extends BaseCoreFunctionalTestCase {
// collection should be back to uninitialized since we have a new entity instance
assertFalse( isPropertyInitialized( store, "inventories" ) );
assertEquals( 2, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 1, sessionFactory().getStatistics().getSessionCloseCount() );
assertEquals( 3, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 2, sessionFactory().getStatistics().getSessionCloseCount() );
store.getInventories().size();
assertTrue( isPropertyInitialized( store, "inventories" ) );
// the extra Session is the temp Session needed to perform the init
assertEquals( 3, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 2, sessionFactory().getStatistics().getSessionCloseCount() );
// the extra Sessions are the temp Sessions needed to perform the init:
// first the entity, then the collection (since it's lazy)
assertEquals( 5, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 4, sessionFactory().getStatistics().getSessionCloseCount() );
// clear Session again. The collection should still be recognized as initialized from above
s.clear();
assertNotNull( store );
assertTrue( isPropertyInitialized( store, "inventories" ) );
assertEquals( 3, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 2, sessionFactory().getStatistics().getSessionCloseCount() );
assertEquals( 5, sessionFactory().getStatistics().getSessionOpenCount() );
assertEquals( 4, sessionFactory().getStatistics().getSessionCloseCount() );
} );
}