Skip to content

Commit

Permalink
HHH-15616 Mitigate performance impact of entity enhancement on Klass'…
Browse files Browse the repository at this point in the history 
…s _secondary_super_cache
  • Loading branch information
@Sanne @beikov
Sanne authored and beikov committed Oct 28, 2022
1 parent bc5b725 commit 5ea8768
Show file tree
Hide file tree
Showing 15 changed files with 375 additions and 72 deletions.
7 changes: 5 additions & 2 deletions hibernate-core/src/main/java/org/hibernate/Hibernate.java
View file
Expand Up @@ -12,6 +12,7 @@
import org.hibernate.bytecode.enhance.spi.interceptor.EnhancementAsProxyLazinessInterceptor;
import org.hibernate.collection.spi.PersistentCollection;
import org.hibernate.engine.HibernateIterator;
import org.hibernate.engine.internal.ManagedTypeHelper;
import org.hibernate.engine.jdbc.LobCreator;
import org.hibernate.engine.jdbc.spi.JdbcServices;
import org.hibernate.engine.spi.PersistentAttributeInterceptable;
Expand Down Expand Up @@ -85,8 +86,10 @@ public static boolean isInitialized(Object proxy) {
if ( proxy instanceof HibernateProxy ) {
return !( (HibernateProxy) proxy ).getHibernateLazyInitializer().isUninitialized();
}
else if ( proxy instanceof PersistentAttributeInterceptable ) {
final PersistentAttributeInterceptor interceptor = ( (PersistentAttributeInterceptable) proxy ).$$_hibernate_getInterceptor();
else if ( ManagedTypeHelper.isPersistentAttributeInterceptable( proxy ) ) {
final PersistentAttributeInterceptable asPersistentAttributeInterceptable = ManagedTypeHelper.asPersistentAttributeInterceptable(
proxy );
final PersistentAttributeInterceptor interceptor = asPersistentAttributeInterceptable.$$_hibernate_getInterceptor();
if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor ) {
return false;
}
Expand Down
69 changes: 62 additions & 7 deletions ...te-core/src/main/java/org/hibernate/bytecode/enhance/internal/bytebuddy/EnhancerImpl.java
View file
Expand Up @@ -34,6 +34,7 @@
import org.hibernate.bytecode.internal.bytebuddy.ByteBuddyState;
import org.hibernate.engine.spi.CompositeOwner;
import org.hibernate.engine.spi.CompositeTracker;
import org.hibernate.engine.spi.EnhancedEntity;
import org.hibernate.engine.spi.EntityEntry;
import org.hibernate.engine.spi.ExtendedSelfDirtinessTracker;
import org.hibernate.engine.spi.Managed;
Expand Down Expand Up @@ -154,12 +155,14 @@ private DynamicType.Builder<?> doEnhance(DynamicType.Builder<?> builder, TypeDes
log.debugf( "Skipping enhancement of [%s]: already enhanced", managedCtClass.getName() );
return null;
}
final EnhancementStatus es = new EnhancementStatus( managedCtClass.getName() );

if ( enhancementContext.isEntityClass( managedCtClass ) ) {
log.debugf( "Enhancing [%s] as Entity", managedCtClass.getName() );
builder = builder.implement( ManagedEntity.class )
.defineMethod( EnhancerConstants.ENTITY_INSTANCE_GETTER_NAME, Object.class, Visibility.PUBLIC )
.intercept( FixedValue.self() );
es.enabledInterfaceManagedEntity();

builder = addFieldWithGetterAndSetter(
builder,
Expand All @@ -183,7 +186,7 @@ private DynamicType.Builder<?> doEnhance(DynamicType.Builder<?> builder, TypeDes
EnhancerConstants.NEXT_SETTER_NAME
);

builder = addInterceptorHandling( builder, managedCtClass );
builder = addInterceptorHandling( builder, managedCtClass, es );

if ( enhancementContext.doDirtyCheckingInline( managedCtClass ) ) {
List<AnnotatedFieldDescription> collectionFields = collectCollectionFields( managedCtClass );
Expand All @@ -206,8 +209,10 @@ private DynamicType.Builder<?> doEnhance(DynamicType.Builder<?> builder, TypeDes
.intercept( implementationSuspendDirtyTracking )
.defineMethod( EnhancerConstants.TRACKER_COLLECTION_GET_NAME, CollectionTracker.class, Visibility.PUBLIC )
.intercept( implementationGetCollectionTrackerWithoutCollections );
es.enabledInterfaceSelfDirtinessTracker();
}
else {
//TODO es.enableInterfaceExtendedSelfDirtinessTracker ? Careful with consequences..
builder = builder.implement( ExtendedSelfDirtinessTracker.class )
.defineField( EnhancerConstants.TRACKER_FIELD_NAME, DirtyTracker.class, FieldPersistence.TRANSIENT, Visibility.PRIVATE )
.annotateField( AnnotationDescription.Builder.ofType( Transient.class ).build() )
Expand Down Expand Up @@ -302,13 +307,13 @@ private DynamicType.Builder<?> doEnhance(DynamicType.Builder<?> builder, TypeDes
}
}

return createTransformer( managedCtClass ).applyTo( builder );
return createTransformer( managedCtClass ).applyTo( builder, es );
}
else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
log.debugf( "Enhancing [%s] as Composite", managedCtClass.getName() );

builder = builder.implement( ManagedComposite.class );
builder = addInterceptorHandling( builder, managedCtClass );
builder = addInterceptorHandling( builder, managedCtClass, es );

if ( enhancementContext.doDirtyCheckingInline( managedCtClass ) ) {
builder = builder.implement( CompositeTracker.class )
Expand All @@ -335,17 +340,17 @@ else if ( enhancementContext.isCompositeClass( managedCtClass ) ) {
.intercept( implementationClearOwner );
}

return createTransformer( managedCtClass ).applyTo( builder );
return createTransformer( managedCtClass ).applyTo( builder, es );
}
else if ( enhancementContext.isMappedSuperclassClass( managedCtClass ) ) {
log.debugf( "Enhancing [%s] as MappedSuperclass", managedCtClass.getName() );

builder = builder.implement( ManagedMappedSuperclass.class );
return createTransformer( managedCtClass ).applyTo( builder );
return createTransformer( managedCtClass ).applyTo( builder, es );
}
else if ( enhancementContext.doExtendedEnhancement( managedCtClass ) ) {
log.debugf( "Extended enhancement of [%s]", managedCtClass.getName() );
return createTransformer( managedCtClass ).applyExtended( builder );
return createTransformer( managedCtClass ).applyExtended( builder, es );
}
else {
log.debugf( "Skipping enhancement of [%s]: not entity or composite", managedCtClass.getName() );
Expand All @@ -367,12 +372,13 @@ private boolean alreadyEnhanced(TypeDescription managedCtClass) {
return false;
}

private DynamicType.Builder<?> addInterceptorHandling(DynamicType.Builder<?> builder, TypeDescription managedCtClass) {
private DynamicType.Builder<?> addInterceptorHandling(DynamicType.Builder<?> builder, TypeDescription managedCtClass, EnhancementStatus es) {
// interceptor handling is only needed if class has lazy-loadable attributes
if ( enhancementContext.hasLazyLoadableAttributes( managedCtClass ) ) {
log.debugf( "Weaving in PersistentAttributeInterceptable implementation on [%s]", managedCtClass.getName() );

builder = builder.implement( PersistentAttributeInterceptable.class );
es.enabledInterfacePersistentAttributeInterceptable();

builder = addFieldWithGetterAndSetter(
builder,
Expand Down Expand Up @@ -592,4 +598,53 @@ void setClassNameAndBytes(String className, byte[] bytes) {
this.resolution = new Resolution.Explicit( bytes);
}
}

/**
* Attempt to keep track of which interfaces are being applied,
* so to attempt dodging the performance implications of for https://bugs.openjdk.org/browse/JDK-8180450
* We're optimising for the case in which entities are fully enhanced.
*/
final static class EnhancementStatus {

private final String typeName;
private boolean managedEntity = false;
private boolean selfDirtynessTracker = false;
private boolean persistentAttributeInterceptable = false;
private boolean applied = false;

public EnhancementStatus(String typeName) {
this.typeName = typeName;
}

public void enabledInterfaceManagedEntity() {
this.managedEntity = true;
}

public void enabledInterfaceSelfDirtinessTracker() {
this.selfDirtynessTracker = true;
}

public void enabledInterfacePersistentAttributeInterceptable() {
this.persistentAttributeInterceptable = true;
}

public DynamicType.Builder<?> applySuperInterfaceOptimisations(DynamicType.Builder<?> builder) {
if ( applied ) {
throw new IllegalStateException("Should not apply super-interface optimisations twice");
}
else {
applied = true;
if ( managedEntity && persistentAttributeInterceptable && selfDirtynessTracker ) {
log.debugf( "Applying Enhancer optimisations for type [%s]; adding EnhancedEntity as additional marker.", typeName );
return builder.implement( EnhancedEntity.class );
}
else {
log.debugf( "Applying Enhancer optimisations for type [%s]; NOT enabling EnhancedEntity as additional marker.", typeName );
}
//TODO consider applying a marker for other combinations of interfaces as well?
}
return builder;
}
}

}
7 changes: 4 additions & 3 deletions ...ava/org/hibernate/bytecode/enhance/internal/bytebuddy/PersistentAttributeTransformer.java
View file
Expand Up @@ -201,7 +201,8 @@ private AnnotatedFieldDescription getEnhancedField(String owner, String name, St
return null;
}

DynamicType.Builder<?> applyTo(DynamicType.Builder<?> builder) {
DynamicType.Builder<?> applyTo(DynamicType.Builder<?> builder, EnhancerImpl.EnhancementStatus es) {
builder = es.applySuperInterfaceOptimisations(builder);
boolean compositeOwner = false;

builder = builder.visit( new AsmVisitorWrapper.ForDeclaredMethods().invokable( NOT_HIBERNATE_GENERATED, this ) );
Expand Down Expand Up @@ -256,7 +257,7 @@ DynamicType.Builder<?> applyTo(DynamicType.Builder<?> builder) {
}

if ( enhancementContext.doExtendedEnhancement( managedCtClass ) ) {
builder = applyExtended( builder );
builder = applyExtended( builder, es );
}

return builder;
Expand Down Expand Up @@ -305,7 +306,7 @@ private Implementation fieldWriterImplementation(AnnotatedFieldDescription enhan
}
}

DynamicType.Builder<?> applyExtended(DynamicType.Builder<?> builder) {
DynamicType.Builder<?> applyExtended(DynamicType.Builder<?> builder, EnhancerImpl.EnhancementStatus es) {
AsmVisitorWrapper.ForDeclaredMethods.MethodVisitorWrapper enhancer = new FieldAccessEnhancer( managedCtClass, enhancementContext, classPool );
return builder.visit( new AsmVisitorWrapper.ForDeclaredMethods().invokable( NOT_HIBERNATE_GENERATED, enhancer ) );
}
Expand Down
21 changes: 12 additions & 9 deletions hibernate-core/src/main/java/org/hibernate/engine/internal/AbstractEntityEntry.java
View file
Expand Up @@ -22,6 +22,7 @@
import org.hibernate.engine.spi.EntityEntry;
import org.hibernate.engine.spi.EntityEntryExtraState;
import org.hibernate.engine.spi.EntityKey;
import org.hibernate.engine.spi.Managed;
import org.hibernate.engine.spi.PersistenceContext;
import org.hibernate.engine.spi.PersistentAttributeInterceptable;
import org.hibernate.engine.spi.PersistentAttributeInterceptor;
Expand Down Expand Up @@ -279,16 +280,18 @@ public void postUpdate(Object entity, Object[] updatedState, Object nextVersion)
getPersister().setPropertyValue( entity, getPersister().getVersionProperty(), nextVersion );
}

if( entity instanceof SelfDirtinessTracker ) {
( (SelfDirtinessTracker) entity ).$$_hibernate_clearDirtyAttributes();
}
ManagedTypeHelper.processIfSelfDirtinessTracker( entity, AbstractEntityEntry::clearDirtyAttributes );

getPersistenceContext().getSession()
.getFactory()
.getCustomEntityDirtinessStrategy()
.resetDirty( entity, getPersister(), (Session) getPersistenceContext().getSession() );
}

private static void clearDirtyAttributes(final SelfDirtinessTracker entity) {
entity.$$_hibernate_clearDirtyAttributes();
}

@Override
public void postDelete() {
setCompressedValue( EnumState.PREVIOUS_STATUS, getStatus() );
Expand Down Expand Up @@ -345,10 +348,10 @@ public boolean requiresDirtyCheck(Object entity) {

@SuppressWarnings( {"SimplifiableIfStatement"})
private boolean isUnequivocallyNonDirty(Object entity) {
if ( entity instanceof SelfDirtinessTracker ) {
if ( ManagedTypeHelper.isSelfDirtinessTracker( entity ) ) {
boolean uninitializedProxy = false;
if ( entity instanceof PersistentAttributeInterceptable ) {
final PersistentAttributeInterceptable interceptable = (PersistentAttributeInterceptable) entity;
if ( ManagedTypeHelper.isPersistentAttributeInterceptable( entity ) ) {
final PersistentAttributeInterceptable interceptable = ManagedTypeHelper.asPersistentAttributeInterceptable( entity );
final PersistentAttributeInterceptor interceptor = interceptable.$$_hibernate_getInterceptor();
if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor ) {
EnhancementAsProxyLazinessInterceptor enhancementAsProxyLazinessInterceptor = (EnhancementAsProxyLazinessInterceptor) interceptor;
Expand All @@ -365,11 +368,11 @@ else if ( entity instanceof HibernateProxy ) {
// we never have to check an uninitialized proxy
return uninitializedProxy || !persister.hasCollections()
&& !persister.hasMutableProperties()
&& !( (SelfDirtinessTracker) entity ).$$_hibernate_hasDirtyAttributes();
&& !ManagedTypeHelper.asSelfDirtinessTracker( entity ).$$_hibernate_hasDirtyAttributes();
}

if ( entity instanceof PersistentAttributeInterceptable ) {
final PersistentAttributeInterceptable interceptable = (PersistentAttributeInterceptable) entity;
if ( ManagedTypeHelper.isPersistentAttributeInterceptable( entity ) ) {
final PersistentAttributeInterceptable interceptable = ManagedTypeHelper.asPersistentAttributeInterceptable( entity );
final PersistentAttributeInterceptor interceptor = interceptable.$$_hibernate_getInterceptor();
if ( interceptor instanceof EnhancementAsProxyLazinessInterceptor ) {
// we never have to check an uninitialized proxy
Expand Down
14 changes: 8 additions & 6 deletions hibernate-core/src/main/java/org/hibernate/engine/internal/EntityEntryContext.java
View file
Expand Up @@ -9,6 +9,7 @@
import org.hibernate.HibernateException;
import org.hibernate.LockMode;
import org.hibernate.boot.registry.classloading.spi.ClassLoaderService;
import org.hibernate.engine.internal.ManagedTypeHelper;
import org.hibernate.engine.spi.EntityEntry;
import org.hibernate.engine.spi.ManagedEntity;
import org.hibernate.engine.spi.PersistenceContext;
Expand Down Expand Up @@ -91,21 +92,22 @@ public void addEntityEntry(Object entity, EntityEntry entityEntry) {
ManagedEntity managedEntity = getAssociatedManagedEntity( entity );
final boolean alreadyAssociated = managedEntity != null;
if ( !alreadyAssociated ) {
if ( ManagedEntity.class.isInstance( entity ) ) {
if ( ManagedTypeHelper.isManaged( entity ) ) {
final ManagedEntity managed = ManagedTypeHelper.asManagedEntity( entity );
if ( entityEntry.getPersister().isMutable() ) {
managedEntity = (ManagedEntity) entity;
managedEntity = managed;
// We know that managedEntity is not associated with the same PersistenceContext.
// Check if managedEntity is associated with a different PersistenceContext.
checkNotAssociatedWithOtherPersistenceContextIfMutable( managedEntity );
}
else {
// Create a holder for PersistenceContext-related data.
managedEntity = new ImmutableManagedEntityHolder( (ManagedEntity) entity );
managedEntity = new ImmutableManagedEntityHolder( managed );
if ( immutableManagedEntityXref == null ) {
immutableManagedEntityXref = new IdentityHashMap<ManagedEntity, ImmutableManagedEntityHolder>();
}
immutableManagedEntityXref.put(
(ManagedEntity) entity,
managed,
(ImmutableManagedEntityHolder) managedEntity
);
}
Expand Down Expand Up @@ -150,8 +152,8 @@ public void addEntityEntry(Object entity, EntityEntry entityEntry) {
}

private ManagedEntity getAssociatedManagedEntity(Object entity) {
if ( ManagedEntity.class.isInstance( entity ) ) {
final ManagedEntity managedEntity = (ManagedEntity) entity;
if ( ManagedTypeHelper.isManaged( entity ) ) {
final ManagedEntity managedEntity = ManagedTypeHelper.asManagedEntity( entity );
if ( managedEntity.$$_hibernate_getEntityEntry() == null ) {
// it is not associated
return null;
Expand Down

0 comments on commit 5ea8768

Please sign in to comment.