[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz edited comment on HHH-7206 at 3/29/12 4:13 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. 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. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz edited comment on HHH-7206 at 3/29/12 4:12 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. 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 ** a) leads to all entities and collection beeing detected for changes (unless the persistent context is completely empty), changes are enqueued to executions (flushEverythingToExecutions) ** b) 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. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz edited comment on HHH-7206 at 3/29/12 5:03 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. 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: 1. Anticipate/move entityStateInsertedNotification/entityStateDeletedNotification calls from flush directly to persist/delete calls: {code:title=AbstractEntityInsertAction.java|borderStyle=solid} protected AbstractEntityInsertAction( ... // 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( ... // 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) // 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) {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

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz edited comment on HHH-7206 at 3/29/12 7:10 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 ); if (!entry.requiresDirtyCheck(entity)) { continue; } final Object[] values = evl.getValues( entity, entry, true, 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...NotificationForNaturalId 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. 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* -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz commented on HHH-7206: -------------------------------------- Here following a concrete example in which these enhancement show the impact on performance. Assume we have a entity class representing the yearly final stocks of a magazine. {code} @Entity class FinalStock { @Id long id; @NaturalId(mutable=true) String articleNumber; @NaturalId(mutable=true) int year; @OneToMany(cascade=All) Set<Inventory> inventories; } {code} Immagine that we want compare all final stocks between the years 2011 and 2010, in order to avoid multiple database hits, we fetch all interested entities into persistent context with a single query, that's reasonable. {code} //session.setFlushMode(Auto) leaving it to the default flush mode List<FinalStock> list = session.createQuery("from FinalStock where year in (2011,2010)").list(); // loading 1.000 FinalStock objs for (FinalStock stock : list) { String correspondging_article_other_year = calculate(stock); //algorythm which cannot replaced by a sofisticated outer join query FinalStock stockOtherYear = session.byNaturalId(FinalStock.class).using("articleNumber",correspondging_article_other_year). using("year",getOtherYear(stock.year)).load(); if (stockOtherYear != null) { // true for 90% or cases //do some work here causing dirty FinalStock objects and dirty collections Inventory inv = new Inventory(....); stock.add(inv); } } {code} As we have all concerning entities already loaded in context, and as there's an excpectance around 90% to retrieve the corresponding stock, we should expect the NaturalId lookups performing very fast. In truth without the first enhancement above, hibernate would spend most of it's time in 1000 total unnecessary and expensive dirty check/session flushes. With the first enhancement implemented, hibernate still would spend substantial time in synchronizing 1000 times (for each naturalIdLoadAccess) the 1000 FinalStocks which are in context. With the second enhancement implemented too(using setAutoSynchronization set to false), the whole thing begins to work in reasonable way. The last wormwood to eliminate are the queries (database-hits) which are performed in those 10% of the cases, where the corresponding NaturalId get not resolved from cache. Since in our concrete case we have already loaded all interested objects in context, those queris are pure overhead. So finally it would be fine to have a further method on NaturalIdLoadAccess allowing do prevent loads from datasource. This 3 enhancements put together in my opinion would open new horizons in hibernate, allowing to do fast in-memory operations which previously were only possible with huge user-code. Please give us your feedback ... -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz edited comment on HHH-7206 at 3/29/12 9:02 AM: --------------------------------------------------------------- Here following a concrete example in which these enhancement show the impact on performance. Assume we have a entity class representing the yearly final stocks of a magazine. {code} @Entity class FinalStock { @Id long id; @Version int version; @NaturalId(mutable=true) String articleNumber; @NaturalId(mutable=true) int year; @OneToMany(cascade=All) @org.hibernate.annotations.OptimisticLock(excluded=false) Set<Inventory> inventories; } {code} Immagine that we want compare all final stocks between the years 2011 and 2010, in order to avoid multiple database hits, we fetch all interested entities into persistent context with a single query, that's reasonable. {code} //session.setFlushMode(Auto) leaving it to the default flush mode List<FinalStock> list = session.createQuery("from FinalStock where year in (2011,2010)").list(); // loading 1.000 FinalStocks for (FinalStock stock : list) { String correspondging_article_other_year = calculate(stock); //algorythm which cannot replaced by a sofisticated outer join query FinalStock stockOtherYear = session.byNaturalId(FinalStock.class).using("articleNumber",correspondging_article_other_year). using("year",getOtherYear(stock.year)).load(); if (stockOtherYear != null) { // true for 90% of cases //do some work here causing some dirty FinalStock objects and dirty collections if (condition) { Inventory inv = new Inventory(....); stock.add(inv); } } } {code} As we have all concerning entities already loaded in context, and as there's an excpectance around 90% to retrieve the corresponding stock, we should expect the NaturalId lookups performing very fast. In truth without the first enhancement above, hibernate would spend most of it's time in 1000 unnecessary and expensive dirty check/session flushes. With the first enhancement implemented, hibernate still would spend substantial time in synchronizing 1000 times (for each naturalIdLoadAccess) the 1000 FinalStocks which are in context. With the second enhancement implemented too(using setAutoSynchronization set to false), the whole thing begins to work in reasonable way. The last wormwood to eliminate are the queries (database-hits) which are performed in those 10% of the cases, where the corresponding NaturalId get not resolved from cache. Since in our concrete case we have already loaded all interested objects in context, those queries are overhead. So finally it would be fine to have a further method on NaturalIdLoadAccess allowing do prevent loads from datasource. This 3 enhancements put together in my opinion would open new horizons in hibernate, allowing to do fast in-memory operations which previously were only possible with huge user-code. Please give us your feedback ... was (Author: pb00067): Here following a concrete example in which these enhancement show the impact on performance. Assume we have a entity class representing the yearly final stocks of a magazine. {code} @Entity class FinalStock { @Id long id; @Version int version; @NaturalId(mutable=true) String articleNumber; @NaturalId(mutable=true) int year; @OneToMany(cascade=All) @org.hibernate.annotations.OptimisticLock(excluded=false) Set<Inventory> inventories; } {code} Immagine that we want compare all final stocks between the years 2011 and 2010, in order to avoid multiple database hits, we fetch all interested entities into persistent context with a single query, that's reasonable. {code} //session.setFlushMode(Auto) leaving it to the default flush mode List<FinalStock> list = session.createQuery("from FinalStock where year in (2011,2010)").list(); // loading 1.000 FinalStocks for (FinalStock stock : list) { String correspondging_article_other_year = calculate(stock); //algorythm which cannot replaced by a sofisticated outer join query FinalStock stockOtherYear = session.byNaturalId(FinalStock.class).using("articleNumber",correspondging_article_other_year). using("year",getOtherYear(stock.year)).load(); if (stockOtherYear != null) { // true for 90% of cases //do some work here causing dirty FinalStock objects and dirty collections Inventory inv = new Inventory(....); stock.add(inv); } } {code} As we have all concerning entities already loaded in context, and as there's an excpectance around 90% to retrieve the corresponding stock, we should expect the NaturalId lookups performing very fast. In truth without the first enhancement above, hibernate would spend most of it's time in 1000 total unnecessary and expensive dirty check/session flushes. With the first enhancement implemented, hibernate still would spend substantial time in synchronizing 1000 times (for each naturalIdLoadAccess) the 1000 FinalStocks which are in context. With the second enhancement implemented too(using setAutoSynchronization set to false), the whole thing begins to work in reasonable way. The last wormwood to eliminate are the queries (database-hits) which are performed in those 10% of the cases, where the corresponding NaturalId get not resolved from cache. Since in our concrete case we have already loaded all interested objects in context, those queris are pure overhead. So finally it would be fine to have a further method on NaturalIdLoadAccess allowing do prevent loads from datasource. This 3 enhancements put together in my opinion would open new horizons in hibernate, allowing to do fast in-memory operations which previously were only possible with huge user-code. Please give us your feedback ... -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz edited comment on HHH-7206 at 3/29/12 9:18 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 synchronization: {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 ); if (!entry.requiresDirtyCheck(entity)) { continue; } final Object[] values = evl.getValues( entity, entry, true, 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...NotificationForNaturalId 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 ); if (!entry.requiresDirtyCheck(entity)) { continue; } final Object[] values = evl.getValues( entity, entry, true, 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...NotificationForNaturalId -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz commented on HHH-7206: -------------------------------------- ??I still like the idea of "synchronization" there being the process of writing those discovered changes to the database?? @Steve: I think writing discovered changes to the database is not practicable with partial synchronizing (= synchronizing just the entity objects of a singular persister): immagine having entity class A with a ToOne relation towards class B. If you associate a new persisted B instance to an A instance and then just flush the changes on persister A, this will lead to a foreign key contraint violation exception on most databases. Furthermore I like to pinpoint once again the serious implications due implicite/unnecessary flushes when working with Long Conversations: both official solicited patterns "Detached Objects (Session per Request with DetachedObjects)" as well as "Extended Session (Session per Conversation)" do rely on the priciple having session FlushMode set to NEVER or COMMIT, and thus having no implicite flush until the very last transaction. Apropos of nothing: currently with session FlushMode set to NEVER or COMMIT the NaturalIdLoadAccess don't recognizes transactional changes at all, so within Long Conversations the new feature is pretty useless: #NaturalIdLoadAccess does not consider updates: this affects the functionality of mutable NaturalIds #NaturalIdLoadAccess does consider neither inserts nor deletions: this even affects the functionality of *immutable* NaturalIds The main aspect of this improvement should be to make NaturalIdLoadAccess feasible also with implementing long conversations. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz edited comment on HHH-7206 at 4/2/12 4:49 AM: -------------------------------------------------------------- ??I still like the idea of "synchronization" there being the process of writing those discovered changes to the database?? @Steve: I think writing discovered changes to the database is not practicable with partial synchronizings (= synchronizing just the entity objects of the persister concerning the currently executed NaturalId lookup). Immagine having entity class A with a ToOne relation towards class B. If you associate a new persisted B instance to an A instance and then just flush the changes on persister A, this will lead to a foreign key contraint violation exception on most databases. Furthermore I like to pinpoint once again the serious implications due implicite/unnecessary flushes when implementing Long Conversations: both official solicited patterns "Detached Objects (Session per Request with DetachedObjects)" as well as "Extended Session (Session per Conversation)" do rely on the priciple having session FlushMode set to NEVER or COMMIT and thus having no implicite flush until the very last transaction. Apropos of nothing: currently with session FlushMode set to NEVER or COMMIT the NaturalIdLoadAccess don't recognizes transactional changes at all, so within Long Conversations the new feature is pretty useless: - NaturalIdLoadAccess does not consider updates: this affects the functionality of mutable NaturalIds - NaturalIdLoadAccess does consider neither inserts nor deletions: this even affects the functionality of *immutable* NaturalIds The main aspect of this improvement should be to make NaturalIdLoadAccess feasible also with implementing long conversations. was (Author: pb00067): ??I still like the idea of "synchronization" there being the process of writing those discovered changes to the database?? @Steve: I think writing discovered changes to the database is not practicable with partial synchronizing (= synchronizing just the entity objects of a singular persister): immagine having entity class A with a ToOne relation towards class B. If you associate a new persisted B instance to an A instance and then just flush the changes on persister A, this will lead to a foreign key contraint violation exception on most databases. Furthermore I like to pinpoint once again the serious implications due implicite/unnecessary flushes when working with Long Conversations: both official solicited patterns "Detached Objects (Session per Request with DetachedObjects)" as well as "Extended Session (Session per Conversation)" do rely on the priciple having session FlushMode set to NEVER or COMMIT, and thus having no implicite flush until the very last transaction. Apropos of nothing: currently with session FlushMode set to NEVER or COMMIT the NaturalIdLoadAccess don't recognizes transactional changes at all, so within Long Conversations the new feature is pretty useless: -NaturalIdLoadAccess does not consider updates: this affects the functionality of mutable NaturalIds -NaturalIdLoadAccess does consider neither inserts nor deletions: this even affects the functionality of *immutable* NaturalIds The main aspect of this improvement should be to make NaturalIdLoadAccess feasible also with implementing long conversations. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz edited comment on HHH-7206 at 4/2/12 5:14 AM: -------------------------------------------------------------- ??I still like the idea of "synchronization" there being the process of writing those discovered changes to the database?? @Steve: I think writing discovered changes to the database is not practicable with partial synchronizings (= synchronizing just the entity objects of the persister concerning the currently executed NaturalId lookup). Immagine having entity class A with a ToOne relation towards class B. If you associate a new persisted B instance to an A instance and then just flush the changes on persister A, this will lead to a foreign key contraint violation exception on most databases. Furthermore of the main aspects of this improvement should be making NaturalIdLoadAccess feasible also with implementing *long* *conversations*. As you surely know, both official solicited patterns "Detached Objects (Session per Request with DetachedObjects)" as well as "Extended Session (Session per Conversation)" do rely on the priciple having session FlushMode set to NEVER or COMMIT and thus having no implicite flush until the very last transaction. Thus we cannot execute anything to database during NaturalIdLoadAccess when session FlushMode is set to < AUTO Currently with session FlushMode set to NEVER or COMMIT the NaturalIdLoadAccess don't recognizes transactional changes at all, so within Long Conversations the NaturalIdLoadAccess is pretty useless: - NaturalIdLoadAccess does not consider updates: this affects the functionality of mutable NaturalIds - NaturalIdLoadAccess does consider neither inserts nor deletions: this even affects the functionality of +immutable+ NaturalIds was (Author: pb00067): ??I still like the idea of "synchronization" there being the process of writing those discovered changes to the database?? @Steve: I think writing discovered changes to the database is not practicable with partial synchronizings (= synchronizing just the entity objects of the persister concerning the currently executed NaturalId lookup). Immagine having entity class A with a ToOne relation towards class B. If you associate a new persisted B instance to an A instance and then just flush the changes on persister A, this will lead to a foreign key contraint violation exception on most databases. Furthermore of the main aspects of this improvement should be making NaturalIdLoadAccess feasible also with implementing *long* *conversations*. As you surely know, both official solicited patterns "Detached Objects (Session per Request with DetachedObjects)" as well as "Extended Session (Session per Conversation)" do rely on the priciple having session FlushMode set to NEVER or COMMIT and thus having no implicite flush until the very last transaction. Thus we cannot execute anything to database during NaturalIdLoadAccess if using FlushMode < Auto Currently with session FlushMode set to NEVER or COMMIT the NaturalIdLoadAccess don't recognizes transactional changes at all, so within Long Conversations the NaturalIdLoadAccess is pretty useless: - NaturalIdLoadAccess does not consider updates: this affects the functionality of mutable NaturalIds - NaturalIdLoadAccess does consider neither inserts nor deletions: this even affects the functionality of *immutable* NaturalIds -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Steve Ebersole commented on HHH-7206: ------------------------------------- Changed issue summary. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Steve Ebersole commented on HHH-7206: ------------------------------------- Another issue here is that the propsed change is locking the second level cache very prematurely. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Steve Ebersole updated HHH-7206: -------------------------------- Assignee: Steve Ebersole -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz commented on HHH-7206: -------------------------------------- Hi Steve, I'm already aware of this, just not noticed it yet here, because I tried to be as sintetic as possible and because also hibernate4.1.1 does call autoFlushIfRequired() in resolveNaturalId even if natural id is immutable, so that's a particularity which has to do be fixed anyway. Other sessions do never pick up uncommitted changes from the second level cache (that would be a big issue indeed)! -in the case you use 2L-Cache with transactional CacheConcurrencyStrategy for example Infinispan the other session always see last commited cache entry (InfiniSpan is a multi-version-capable cache for this purpose) -in the case you use 2L-Cache with read-write CacheConcurrencyStrategy the cached entry gets locked until the end of the transaction so that other sessions don't pick up an uncommited value. It is indeed true, that the proposed change is updating the second level cache rather early, but as the user is more likely to commit rather than rollback his transaction finally, this should not be a problem. Last but not least also a regular flush could occur already in the first phases of a transaction, with the identical effects, true or not? Sorry, but I cannot follow your argumentation: even if we would write just the natural-id changes to the database, on every next flush that entity gets again checked for dirtiness anyway. Writing just the detected natural-id changes immediately to the database, don't saves hiberante from absolutely nothing in terms of total work amount IMO, in contrary: you would split up the changes in more parts needing in total up to 2 update-executions instead to a single one. If you insist, we can also write just the detected natural-id changes immediately, but surely not if session flushMode is set to flushmode < AUTO, because otherwise, as already mentioned above, we would break the long conversation patterns which trust in no implicit flushes occuring. As the NaturalIdLoad feature is brand new, now we are still in time to define how it should work. Which kind of troubles do you see concretely with a natural-id mutation in-line within other use-cases? In-line natural-id mutation handling is of basic importance for all users applying the long conversation patterns. The long conversation pattern should really not be a foreign use-case to you. Also the concept of pre-fetching all interested entities into context with less as possible database-hits, is suggested practise in hibernate documentation, so the use-case I have shown should not be that particular foreign. Basically that what I want ask you and Eric is: How should hibernate users applying the long conversation patterns (session.flushMode set to < AUTO) benefit from the new NaturalId feature? Currently the new apis are pretty useless in this context because transactional changes are not considered. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz edited comment on HHH-7206 at 4/3/12 7:30 AM: -------------------------------------------------------------- Hi Steve, I'm already aware of this, just not noticed it yet here, because I tried to be as sintetic as possible and because also hibernate4.1.1 does call autoFlushIfRequired() in resolveNaturalId even if natural id is immutable, so that's a todo which has to do be fixed anyway. Other sessions do never pick up uncommitted changes from the second level cache (that would be a big issue indeed)! -in the case you use 2L-Cache with transactional CacheConcurrencyStrategy for example Infinispan the other session always see last commited cache entry (InfiniSpan is a multi-version-capable cache for this purpose) -in the case you use 2L-Cache with read-write CacheConcurrencyStrategy the cached entry gets locked until the end of the transaction so that other sessions don't pick up an uncommited value. It is true, that the proposed change is updating the second level cache rather early, but as the user is more likely to commit rather than rollback his transaction finally, this should not be a problem. Last but not least also a regular flush could occur already in the first phases of a transaction, with the identical effects, true or not? Sorry, but I cannot follow your argumentation: even if we would write just the natural-id changes to the database, on every next flush that entity gets again checked for dirtiness anyway. Writing just the detected natural-id changes immediately to the database, don't saves hiberante from absolutely nothing in terms of total work amount IMO, in contrary: you would split up the changes in more parts needing in total up to 2 update-executions instead to a single one. If you insist, we can also write just the detected natural-id changes immediately, but surely not if session flushMode is set to flushmode < AUTO, because otherwise, as already mentioned above, we would break the long conversation patterns which base in no implicit flushes occuring until the last transaction. In-line natural-id mutation handling is of basic importance for all users applying the long conversation patterns. The long conversation pattern should really not be a foreign use-case to you. Also the concept of pre-fetching all interested entities into context with less as possible database-hits, is suggested practice in hibernate documentation, so the use-case I have shown should not be that particular foreign. Basically that what I want ask you and Eric is: How should hibernate users applying the long conversation patterns (session.flushMode set to < AUTO) benefit from the new NaturalId feature? Currently the new apis are pretty useless in this context because transactional changes are not considered. As the NaturalIdLoad feature is brand new, now we are still in time to define how it should work. What are your concrete fears, in implementing a in-line natural-id mutation handling for all use-cases? Finally all proposed enhancements are strictly concerning the NaturalId handling, nothing else, so all other stuff works as before: I let run the matrix-tests today with the proposed changes and, except the tests regarding NaturalId which obviously have to be adapted, the rest ran witoud failures: {code} ... :hibernate-core:processMatrixResources UP-TO-DATE :hibernate-core:matrixClasses :hibernate-core:matrixUnitTest Test org.hibernate.test.naturalid.immutableentity.ImmutableEntityNaturalIdTest FAILED Test org.hibernate.test.naturalid.inheritance.InheritedNaturalIdTest FAILED Test org.hibernate.test.naturalid.mutable.MutableNaturalIdTest FAILED Test org.hibernate.test.naturalid.mutable.cached.CachedMutableNaturalIdTest FAILED 3037 tests completed, 5 failures {code} I understand that along classic flushing, in-line natural-id mutation handling would represent a complete new approach of synchronization process, but I think it is worth to be implemented and maintained. Last but not least also users using flushmode AUTO would benefit from this new architecture in terms of performance, as hibernate gets saved from having to scan the entire persistence context for dirty-changes on each NaturalId load access. I understand that you as project maintainer, are mainly concerned about hibernate being working correctly, anyway I hope there are other hibernate-developers attributing some importance to performance aspects too. was (Author: pb00067): Hi Steve, I'm already aware of this, just not noticed it yet here, because I tried to be as sintetic as possible and because also hibernate4.1.1 does call autoFlushIfRequired() in resolveNaturalId even if natural id is immutable, so that's a particularity which has to do be fixed anyway. Other sessions do never pick up uncommitted changes from the second level cache (that would be a big issue indeed)! -in the case you use 2L-Cache with transactional CacheConcurrencyStrategy for example Infinispan the other session always see last commited cache entry (InfiniSpan is a multi-version-capable cache for this purpose) -in the case you use 2L-Cache with read-write CacheConcurrencyStrategy the cached entry gets locked until the end of the transaction so that other sessions don't pick up an uncommited value. It is indeed true, that the proposed change is updating the second level cache rather early, but as the user is more likely to commit rather than rollback his transaction finally, this should not be a problem. Last but not least also a regular flush could occur already in the first phases of a transaction, with the identical effects, true or not? Sorry, but I cannot follow your argumentation: even if we would write just the natural-id changes to the database, on every next flush that entity gets again checked for dirtiness anyway. Writing just the detected natural-id changes immediately to the database, don't saves hiberante from absolutely nothing in terms of total work amount IMO, in contrary: you would split up the changes in more parts needing in total up to 2 update-executions instead to a single one. If you insist, we can also write just the detected natural-id changes immediately, but surely not if session flushMode is set to flushmode < AUTO, because otherwise, as already mentioned above, we would break the long conversation patterns which trust in no implicit flushes occuring. As the NaturalIdLoad feature is brand new, now we are still in time to define how it should work. Which kind of troubles do you see concretely with a natural-id mutation in-line within other use-cases? In-line natural-id mutation handling is of basic importance for all users applying the long conversation patterns. The long conversation pattern should really not be a foreign use-case to you. Also the concept of pre-fetching all interested entities into context with less as possible database-hits, is suggested practice in hibernate documentation, so the use-case I have shown should not be that particular foreign. Basically that what I want ask you and Eric is: How should hibernate users applying the long conversation patterns (session.flushMode set to < AUTO) benefit from the new NaturalId feature? Currently the new apis are pretty useless in this context because transactional changes are not considered. -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Guenther Demetz edited comment on HHH-7206 at 4/3/12 12:33 PM: --------------------------------------------------------------- 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 call from flush directly to persist call: {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=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 } {code} Second step is to deactivate autoflush in resolveNaturalId replacing it with a in-memory synchronization: {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 ); if (!entry.requiresDirtyCheck(entity)) { continue; } final Object[] values = evl.getValues( entity, entry, true, 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. 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 synchronization: {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 ); if (!entry.requiresDirtyCheck(entity)) { continue; } final Object[] values = evl.getValues( entity, entry, true, 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...NotificationForNaturalId -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira

[ https://hibernate.onjira.com/browse/HHH-7206?page=com.atlassian.jira.plug... ] Steve Ebersole resolved HHH-7206. --------------------------------- Resolution: Fixed Fix Version/s: 4.1.2 -- This message is automatically generated by JIRA. For more information on JIRA, see: http://www.atlassian.com/software/jira