HHH-6848 : Performance Optimization of in memory merge algorithm (Wim…

… Ockerman)

hibernate-core/src/main/java/org/hibernate/event/internal/EventCache.java

@@ -38,6 +38,14 @@
  * to the EventCache before the operation has cascaded to that
  * entity.
  * <p/>
+ * There are some restriction;
+ * <ul>
+ *     <li>the same value cannot be associated with more than one key</li>
+ *     <li>Methods that return collections (e.g., {@link #keySet()},
+ *          {@link #values()}, {@link #entrySet()}) return an
+ *          unnmodifiable view of the collection.</li>
+ * </ul>
+ * <p/>
  * The following methods can be used by event listeners (and other
  * classes) in the same package to add entities to an EventCache
  * and indicate if the operation is being performed on the entity:<p/>
@@ -85,9 +93,9 @@ public boolean containsKey(Object entity) {
 	}
 
 	/**
-	 * Returns true if this EventCache maps one or more entities to the specified copy.
+	 * Returns true if this EventCache maps an entity to the specified copy.
 	 * @param copy must be non-null
-	 * @return true if this EventCache maps one or more entities to the specified copy
+	 * @return true if this EventCache maps an entity to the specified copy
 	 * @throws NullPointerException if copy is null
 	 */
 	public boolean containsValue(Object copy) {
@@ -141,10 +149,11 @@ public Set keySet() {
 	/**
 	 * Associates the specified entity with the specified copy in this EventCache;
 	 * @param entity must be non-null
-	 * @param copy must be non- null
+	 * @param copy must be non- null and must not be associated with any other entity in this EntityCache.
 	 * @return previous copy associated with specified entity, or null if
 	 * there was no mapping for entity.
 	 * @throws NullPointerException if entity or copy is null
+	 * @throws IllegalStateException if the specified copy is already associated with a different entity.
 	 */
 	public Object put(Object entity, Object copy) {
 		return put( entity, copy, Boolean.FALSE );
@@ -153,12 +162,13 @@ public Object put(Object entity, Object copy) {
 	/**
 	 * Associates the specified entity with the specified copy in this EventCache;
 	 * @param entity must be non-null
-	 * @param copy must be non- null
-	 * @param isOperatedOn indicates if the operation is performed on the entity
+	 * @param copy must be non- null and must not be associated with any other entity in this EntityCache.
+	 * @param isOperatedOn indicates if the operation is performed on the entity.
 	 *
 	 * @return previous copy associated with specified entity, or null if
 	 * there was no mapping for entity.
 	 * @throws NullPointerException if entity or copy is null
+	 * @throws IllegalStateException if the specified copy is already associated with a different entity.
 	 */
 	/* package-private */ Object put(Object entity, Object copy, boolean isOperatedOn) {
 		if ( entity == null || copy == null ) {
@@ -171,25 +181,34 @@ public Object put(Object entity, Object copy) {
 
 		if ( oldCopy == null ) {
 			if  ( oldEntity != null ) {
-				throw new IllegalStateException( "An entity copy is already assigned to a different entity." );
+				throw new IllegalStateException( "An entity copy was already assigned to a different entity." );
 			}
 			if ( oldOperatedOn != null ) {
 				throw new IllegalStateException( "entityToOperatedOnFlagMap contains an entity, but entityToCopyMap does not." );
 			}
 		}
 		else {
-			if ( oldEntity == null ) {
-				throw new IllegalStateException( "An entity already had a copy in entityToCopyMap, but the specified copy was not in copyToEntityMap." );
+			if ( oldCopy != copy ) {
+				// Replaced an entity copy with a new copy; need to remove the oldCopy from copyToEntityMap
+				// to synch things up.
+				Object removedEntity = copyToEntityMap.remove( oldCopy );
+				if ( removedEntity != entity ) {
+					throw new IllegalStateException( "An unexpected entity was associated with the old entity copy." );
+				}
+				if ( oldEntity != null ) {
+					throw new IllegalStateException( "A new entity copy is already associated with a different entity." );
+				}
+			}
+			else {
+				// Replaced an entity copy with the same copy in entityToCopyMap.
+				// Make sure that copy is associated with the same entity in copyToEntityMap.
+				if ( oldEntity != entity ) {
+					throw new IllegalStateException( "An entity copy was associated with a different entity than provided." );
+				}
 			}
 			if ( oldOperatedOn == null ) {
 				throw new IllegalStateException( "entityToCopyMap contained an entity, but entityToOperatedOnFlagMap did not." );
 			}
-			if ( oldEntity != entity ) {
-				throw new IllegalStateException( "An entity copy was associated with a different entity than provided." );
-			}
-			if ( oldCopy != copy ) {
-				throw new IllegalStateException( "An entity was already associated with a copy that is different from the copy provided." );
-			}
 		}
 
 		return oldCopy;

hibernate-core/src/test/java/org/hibernate/event/internal/EventCacheTest.java

@@ -319,24 +319,28 @@ public void testEntityToCopyFillFollowedByModifyValueOfEntrySetElement() {
 		assertSame( copy, entry.getValue() );
 	}
 
-	public void testReplaceCopyForEntity() {
+	public void testReplaceEntityCopy() {
 
 		EventCache cache = new EventCache();
 		Simple entity = new Simple( 1 );
 		Simple copy = new Simple( 0 );
 		cache.put(entity, copy);
 
-		try {
-			cache.put( entity, new Simple( 0 ) );
-			fail( "should have thrown IllegalStateException");
-		}
-		catch( IllegalStateException ex ) {
-			// expected
-		}
+		Simple copyNew = new Simple( 0 );
+		assertSame( copy, cache.put( entity, copyNew ) );
+		assertSame( copyNew, cache.get( entity ) );
+
+		checkCacheConsistency( cache, 1 );
+
+		copy = copyNew;
+		copyNew = new Simple( 1 );
+		assertSame( copy, cache.put( entity, copyNew ) );
+		assertSame( copyNew, cache.get( entity ) );
 
+		checkCacheConsistency( cache, 1 );
 	}
 
-	public void testReplaceEntityForCopy() {
+	public void testCopyAssociatedWithNewAndExistingEntity() {
 
 		EventCache cache = new EventCache();
 		Simple entity = new Simple( 1 );
@@ -353,26 +357,7 @@ public void testReplaceEntityForCopy() {
 
 	}
 
-	public void testReplaceEntityForExistingCopy() {
-
-		EventCache cache = new EventCache();
-		Simple entity1 = new Simple( 1 );
-		Simple copy1 = new Simple( 0 );
-		cache.put(entity1, copy1);
-		Simple entity2 = new Simple( 2 );
-		Simple copy2 = new Simple( 0 );
-		cache.put( entity2, copy2 );
-
-		try {
-			cache.put( entity1, copy2 );
-			fail( "should have thrown IllegalStateException");
-		}
-		catch( IllegalStateException ex ) {
-			// expected
-		}
-	}
-
-	public void testReplaceCopyForExistingEntity() {
+	public void testCopyAssociatedWith2ExistingEntities() {
 
 		EventCache cache = new EventCache();
 		Simple entity1 = new Simple( 1 );