[hibernate-issues] [Hibernate-JIRA] Issue Comment Edited: (HHH-7206) NaturalIdLoadAccess: NaturalId synchronization with no need to flush at all

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz edited comment on HHH-7206 at 3/29/12 5:14 AM: --------------------------------------------------------------- Motivation: Although documented nowhere, currently NaturalIdLoadAccess is working like query-apis, relying on flushes in order to get the context synchronized. This has some serious impacts on performance and functionality: 1. with session flushMode set to >= AUTO, *each* single naturalId-load-action - leads to all entities and collection beeing detected for changes (unless the persistent context is completely empty), changes are enqueued to executions (flushEverythingToExecutions) - leads to executions being propagated to the database (performExecutions) if there is at least one action enqueued for concerning querySpace or if session flushMode is set to ALWAYS Thus performing many NaturalIdLoadAccesses within a medium-large sized context, has serious performance impacts! Furthermore on long-timed transactions flushes are problematic. The time-window between flush and final commit easily become big enough to cause lock-waits or/and lock-timeouts on concurrent transactions. 2. by setting session flushMode to <= COMMIT, NaturalIdLoadAccess suddenly does not consider unflushed changes at all. Since not documented, this represents some lack in functionality. The improvement idea is to *do all natural-id synchronizations in memory*, without ever touching the context's actionqueue. Here the steps to do it: Anticipate/move entityStateInsertedNotification/entityStateDeletedNotification calls from flush directly to persist/delete calls: {code:title=AbstractEntityInsertAction.java|borderStyle=solid} protected AbstractEntityInsertAction( ... (unchanged, as it is) // add following lines in the constructor PersistenceContext persistenceContext = session.getPersistenceContext(); EntityEntry entityEntry = persistenceContext.getEntry(instance); persistenceContext.entityStateInsertedNotification(entityEntry, state); } {code} {code:title=EntityDeleteAction.java|borderStyle=solid} protected EntityDeleteAction( ... (unchanged, as it is) // add following lines in the constructor PersistenceContext persistenceContext = session.getPersistenceContext(); EntityEntry entityEntry = persistenceContext.getEntry(instance); persistenceContext.entityStateDeletedNotification(entityEntry, state); } {code} {code:title=EntityEntry.java|borderStyle=solid} private void notifyLoadedStateInserted(Object[] state) { if ( persistenceContext == null ) { throw new HibernateException( "PersistenceContext was null on attempt to insert loaded state" ); } // persistenceContext.entityStateInsertedNotification( this, state ); call moved to AbstractEntityInsertAction } private void notifyLoadedStateDeleted(Object[] deletedState) { if ( persistenceContext == null ) { throw new HibernateException( "PersistenceContext was null on attempt to delete loaded state" ); } // persistenceContext.entityStateDeletedNotification( this, deletedState ); call moved to EntityDeleteAction } public void postUpdate(Object entity, Object[] updatedState, Object nextVersion) { ... (unchanged, as it is) // notifyLoadedStateUpdated( updatedState ); call deferred to resolveNaturalId } {code} Second step is to deactivate autoflush in resolveNaturalId replacing it with a in-memory synchonization: {code:title=SessionImpl$BaseNaturalIdLoadAccessImpl.java|borderStyle=solid} protected final Serializable resolveNaturalId(Map<String, Object> naturalIdParameters) { // remove autoFlush // final Set<Serializable> querySpaces = new LinkedHashSet<Serializable>(); // for ( final Serializable querySpace : entityPersister.getQuerySpaces() ) { // querySpaces.add( querySpace ); // } // // autoFlushIfRequired( querySpaces ); if (isTransactionInProgress() && getPersistenceContext().getEntityEntries().size() > 0) { StatefulPersistenceContext pc = (StatefulPersistenceContext) getPersistenceContext(); final IdentifierLoadAccess identifierLoadAccess = getIdentifierLoadAccess(); final DefaultFlushEntityEventListener evl = new DefaultFlushEntityEventListener(); for (Serializable pk : pc.getCachedNaturalPks(entityPersister)) { final Object entity = identifierLoadAccess.getReference(pk); final EntityEntry entry = pc.getEntry(entity); final FlushEntityEvent entityEvent = new FlushEntityEvent( SessionImpl.this, entity, entry ); final Object[] values = evl.getValues( entity, entry, entry.requiresDirtyCheck(entity), SessionImpl.this ); entityEvent.setPropertyValues(values); evl.dirtyCheck(entityEvent); int[] dirtyProperties = entityEvent.getDirtyProperties(); if (dirtyProperties != null) { pc.entityStateUpdatedNotification( entry, values ); // synchronizes natural id cache } } } ... (unchanged, as it is) {code} The rest are merely new getter methods: {code:title=NaturalIdXrefDelegate.java|borderStyle=solid} public Collection<Serializable> getCachedNaturalPks(EntityPersister persister) { NaturalIdResolutionCache entityNaturalIdResolutionCache = naturalIdResolutionCacheMap.get( persister ); if (entityNaturalIdResolutionCache != null) { return entityNaturalIdResolutionCache.getCachedPks(); } return new ArrayList<Serializable>(); } {code} {code:title=NaturalIdXrefDelegate$NaturalIdResolutionCache.java|borderStyle=solid} public Collection<Serializable> getCachedPks() { return naturalIdToPkMap.values(); } {code} {code:title=StatefulPersistenceContext.java|borderStyle=solid} public Collection<Serializable> getCachedNaturalPks(EntityPersister persister) { return naturalIdXrefDelegate.getCachedNaturalPks(persister); } {code} That's all in essence. N.B.: As now the 3 entityState-notification-methods would be definitely for NaturalId purpose only, in this context it may be better, to rename them in something like entityState...Notification*ForNaturalId* was (Author: pb00067): Motivation: Although documented nowhere, currently NaturalIdLoadAccess is working like query-apis, relying on flushes in order to get the context synchronized. This has some serious impacts on performance and functionality: 1. with session flushMode set to >= AUTO, *each* single naturalId-load-action - leads to all entities and collection beeing detected for changes (unless the persistent context is completely empty), changes are enqueued to executions (flushEverythingToExecutions) - leads to executions being propagated to the database (performExecutions) if there is at least one action enqueued for concerning querySpace or if session flushMode is set to ALWAYS Thus performing many NaturalIdLoadAccesses within a medium-large sized context, has serious performance impacts! Furthermore on long-timed transactions flushes are problematic. The time-window between flush and final commit easily become big enough to cause lock-waits or/and lock-timeouts on concurrent transactions. 2. by setting session flushMode to <= COMMIT, NaturalIdLoadAccess suddenly does not consider unflushed changes at all. Since not documented, this represents some lack in functionality. The improvement idea is to *do all natural-id synchronizations in memory*, without ever touching the context's actionqueue. Here the steps to do it: Anticipate/move entityStateInsertedNotification/entityStateDeletedNotification calls from flush directly to persist/delete calls: {code:title=AbstractEntityInsertAction.java|borderStyle=solid} protected AbstractEntityInsertAction( ... (unchanged, as it is) // add following lines in the constructor PersistenceContext persistenceContext = session.getPersistenceContext(); EntityEntry entityEntry = persistenceContext.getEntry(instance); persistenceContext.entityStateInsertedNotification(entityEntry, state); } {code} {code:title=EntityDeleteAction.java|borderStyle=solid} protected EntityDeleteAction( ... (unchanged, as it is) // add following lines in the constructor PersistenceContext persistenceContext = session.getPersistenceContext(); EntityEntry entityEntry = persistenceContext.getEntry(instance); persistenceContext.entityStateDeletedNotification(entityEntry, state); } {code} {code:title=EntityEntry.java|borderStyle=solid} private void notifyLoadedStateInserted(Object[] state) { if ( persistenceContext == null ) { throw new HibernateException( "PersistenceContext was null on attempt to insert loaded state" ); } // persistenceContext.entityStateInsertedNotification( this, state ); call moved to AbstractEntityInsertAction } private void notifyLoadedStateDeleted(Object[] deletedState) { if ( persistenceContext == null ) { throw new HibernateException( "PersistenceContext was null on attempt to delete loaded state" ); } // persistenceContext.entityStateDeletedNotification( this, deletedState ); call moved to EntityDeleteAction } public void postUpdate(Object entity, Object[] updatedState, Object nextVersion) { ... (unchanged, as it is) // notifyLoadedStateUpdated( updatedState ); call deferred to resolveNaturalId } {code} Second step is to deactivate autoflush in resolveNaturalId replacing it with a in-memory synchonization: {code:title=SessionImpl$BaseNaturalIdLoadAccessImpl.java|borderStyle=solid} protected final Serializable resolveNaturalId(Map<String, Object> naturalIdParameters) { // remove autoFlush // final Set<Serializable> querySpaces = new LinkedHashSet<Serializable>(); // for ( final Serializable querySpace : entityPersister.getQuerySpaces() ) { // querySpaces.add( querySpace ); // } // // autoFlushIfRequired( querySpaces ); if (isTransactionInProgress() && getPersistenceContext().getEntityEntries().size() > 0) { StatefulPersistenceContext pc = (StatefulPersistenceContext) getPersistenceContext(); final IdentifierLoadAccess identifierLoadAccess = getIdentifierLoadAccess(); final DefaultFlushEntityEventListener evl = new DefaultFlushEntityEventListener(); for (Serializable pk : pc.getCachedNaturalPks(entityPersister)) { final Object entity = identifierLoadAccess.getReference(pk); final EntityEntry entry = pc.getEntry(entity); final FlushEntityEvent entityEvent = new FlushEntityEvent( SessionImpl.this, entity, entry ); final Object[] values = evl.getValues( entity, entry, entry.requiresDirtyCheck(entity), SessionImpl.this ); entityEvent.setPropertyValues(values); evl.dirtyCheck(entityEvent); int[] dirtyProperties = entityEvent.getDirtyProperties(); if (dirtyProperties != null) { pc.entityStateUpdatedNotification( entry, values ); // synchronizes natural id cache } } } ... (unchanged, as it is) {code} The rest are merely new getter methods: {code:title=NaturalIdXrefDelegate.java|borderStyle=solid} public Collection<Serializable> getCachedNaturalPks(EntityPersister persister) { NaturalIdResolutionCache entityNaturalIdResolutionCache = naturalIdResolutionCacheMap.get( persister ); if (entityNaturalIdResolutionCache != null) { return entityNaturalIdResolutionCache.getCachedPks(); } return new ArrayList<Serializable>(); } {code} {code:title=NaturalIdXrefDelegate$NaturalIdResolutionCache.java|borderStyle=solid} public Collection<Serializable> getCachedPks() { return naturalIdToPkMap.values(); } {code} {code:title=StatefulPersistenceContext.java|borderStyle=solid} public Collection<Serializable> getCachedNaturalPks(EntityPersister persister) { return naturalIdXrefDelegate.getCachedNaturalPks(persister); } {code} That's all in essence. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira