From 71a0698226861dff8e38fba6bd0b5f1fdf32ca5e Mon Sep 17 00:00:00 2001 From: Gail Badner Date: Mon, 9 Apr 2012 14:54:50 -0700 Subject: [PATCH] HHH-6848 : Performance Optimization of in memory merge algorithm (Wim Ockerman) --- .../hibernate/event/internal/EventCache.java | 67 +++++++++- .../event/internal/EventCacheTest.java | 117 ++++++++++++++++++ 2 files changed, 178 insertions(+), 6 deletions(-) create mode 100644 hibernate-core/src/test/java/org/hibernate/event/internal/EventCacheTest.java diff --git a/hibernate-core/src/main/java/org/hibernate/event/internal/EventCache.java b/hibernate-core/src/main/java/org/hibernate/event/internal/EventCache.java index f44fa989c8..8205cc3eaa 100644 --- a/hibernate-core/src/main/java/org/hibernate/event/internal/EventCache.java +++ b/hibernate-core/src/main/java/org/hibernate/event/internal/EventCache.java @@ -20,6 +20,41 @@ * Free Software Foundation, Inc. * 51 Franklin Street, Fifth Floor * Boston, MA 02110-1301 USA + * + NOTICE: File changed by Wim Ockerman @ CISCO on 2011/10/20 + * Reasons of change: + * 1. The EventCache is used during the merging of object graphs + * to persisted objects. During this process the EventCache + * builds up a map of entities (persistable objects) to + * copies (original detached objects or loaded entities). + * At certain places in the merge algorithm the inverse relation + * is needed, see + * ...def.AbstractSaveEventListener.performSaveOrReplicate + * -> calling persister.getPropertyValuesToInsert(.., getMergeMap(),..) + * The getMergeMap() call calls through this this class' invertMap method. + * + * In the original implementation of invertMap() a map was created on the spot + * and all the elements in the entityToCopyMap where put in, now as copy + * to entity pair. + * Because this action can happen a lot in a big detached graph wile merging, the size + * of the entityToCopyMap can become substantial and thus also the creation of the + * inverted map. + * Tests with large graphs thus showed quadratic loss of performance. From a certain + * graph size this became substantial. + * + * + * As solution an inverseEntitiyToCopyMap (so a copyToEntityMap) is maintained now + * along with the changes to the entityToCopyMap. + * + * This made the merge action more linear in terms of performance. + * + * The changes are covered by a new UnitTest. + * + * + * Changes in the code have preceding comments of format + * CHANGE Wim Ockerman [:] + * And end with: + * END OF CHANGE */ package org.hibernate.event.internal; @@ -55,6 +90,10 @@ class EventCache implements Map { // key is an entity involved with the operation performed by the listener; // value can be either a copy of the entity or the entity itself + // CHANGE Wim Ockerman 2011/10/20: maintains the inverse of the entityToCopyMap for performance reasons. + private Map copyToEntityMap = new IdentityHashMap( 10 ); + // END OF CHANGE + private Map entityToOperatedOnFlagMap = new IdentityHashMap( 10 ); // key is an entity involved with the operation performed by the listener; // value is a flag indicating if the listener explicitly operates on the entity @@ -64,6 +103,11 @@ class EventCache implements Map { */ public void clear() { entityToCopyMap.clear(); + + // CHANGE Wim Ockerman 2011/10/20 + copyToEntityMap.clear(); + // END OF CHANGE + entityToOperatedOnFlagMap.clear(); } @@ -143,6 +187,9 @@ class EventCache implements Map { throw new NullPointerException( "null entities and copies are not supported by " + getClass().getName() ); } entityToOperatedOnFlagMap.put( entity, Boolean.FALSE ); + // CHANGE Wim Ockerman 2011/10/20 + copyToEntityMap.put(copy, entity); + // END OF CHANGE return entityToCopyMap.put( entity, copy ); } @@ -161,6 +208,9 @@ class EventCache implements Map { throw new NullPointerException( "null entities and copies are not supported by " + getClass().getName() ); } entityToOperatedOnFlagMap.put( entity, Boolean.valueOf( isOperatedOn ) ); + // CHANGE Wim Ockerman 2011/10/20 + copyToEntityMap.put(copy, entity); + // END OF CHANGE return entityToCopyMap.put( entity, copy ); } @@ -176,6 +226,9 @@ class EventCache implements Map { throw new NullPointerException( "null entities and copies are not supported by " + getClass().getName() ); } entityToCopyMap.put( entry.getKey(), entry.getValue() ); + // CHANGE Wim Ockerman 2011/10/20 + copyToEntityMap.put(entry.getValue(), entry.getKey()); + // END OF CHANGE entityToOperatedOnFlagMap.put( entry.getKey(), Boolean.FALSE ); } } @@ -191,7 +244,11 @@ class EventCache implements Map { throw new NullPointerException( "null entities are not supported by " + getClass().getName() ); } entityToOperatedOnFlagMap.remove( entity ); - return entityToCopyMap.remove( entity ); + // CHANGE Wim Ockerman 2011/10/20 + Object result = entityToCopyMap.remove( entity ); + copyToEntityMap.remove(result); + // END OF CHANGE + return result; } /** @@ -246,10 +303,8 @@ class EventCache implements Map { * @return the copy-entity mappings */ public Map invertMap() { - Map result = new IdentityHashMap( entityToCopyMap.size() ); - for ( Entry entry : (Set)entityToCopyMap.entrySet() ) { - result.put( entry.getValue(), entry.getKey() ); - } - return result; + // CHANGE Wim Ockerman 2011/10/20 + return copyToEntityMap; + // END OF CHANGE } } \ No newline at end of file diff --git a/hibernate-core/src/test/java/org/hibernate/event/internal/EventCacheTest.java b/hibernate-core/src/test/java/org/hibernate/event/internal/EventCacheTest.java new file mode 100644 index 0000000000..32ab342745 --- /dev/null +++ b/hibernate-core/src/test/java/org/hibernate/event/internal/EventCacheTest.java @@ -0,0 +1,117 @@ +/* + * Hibernate, Relational Persistence for Idiomatic Java + * + * Copyright (c) 2008, Red Hat Middleware LLC or third-party contributors as + * indicated by the @author tags or express copyright attribution + * statements applied by the authors. All third-party contributions are + * distributed under license by Red Hat Middleware LLC. + * + * This copyrighted material is made available to anyone wishing to use, modify, + * copy, or redistribute it subject to the terms and conditions of the GNU + * Lesser General Public License, as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY + * or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this distribution; if not, write to: + * Free Software Foundation, Inc. + * 51 Franklin Street, Fifth Floor + * Boston, MA 02110-1301 USA + * + */ +package org.hibernate.event.internal; + +import java.util.HashMap; +import java.util.Map; + +import junit.framework.TestCase; + +/** + * 2011/10/20 Unit test for code added in EventCache for performance improvement. + * @author Wim Ockerman @ CISCO + * + * + * + */ +public class EventCacheTest extends TestCase { + + public void testEntityToCopyFillFollowedByCopyToEntityMapping() { + EventCache cache = new EventCache(); + Object entity = new Integer(1); + Object copy = new Integer(2); + + cache.put(entity, copy); + assertTrue(cache.containsKey(entity)); + assertFalse(cache.containsKey(copy)); + + assertTrue(cache.invertMap().containsKey(copy)); + assertFalse(cache.invertMap().containsKey(entity)); + + cache.clear(); + assertFalse(cache.containsKey(entity)); + assertFalse(cache.invertMap().containsKey(copy)); + } + + public void testEntityToCopyFillFollowedByCopyToEntityMappingOnRemove() { + EventCache cache = new EventCache(); + Object entity = new Integer(1); + Object copy = new Integer(2); + + cache.put(entity, copy); + assertTrue(cache.containsKey(entity)); + assertFalse(cache.containsKey(copy)); + + assertTrue(cache.invertMap().containsKey(copy)); + assertFalse(cache.invertMap().containsKey(entity)); + + cache.remove(entity); + assertFalse(cache.containsKey(entity)); + assertFalse(cache.invertMap().containsKey(copy)); + } + + public void testEntityToCopyFillFollowedByCopyToEntityUsingPutAll() { + EventCache cache = new EventCache(); + Map input = new HashMap(); + Object entity1 = new Integer(1); + Object copy1 = new Integer(2); + input.put(entity1, copy1); + Object entity2 = new Integer(3); + Object copy2 = new Integer(2); + input.put(entity2, copy2); + cache.putAll(input); + + assertTrue(cache.containsKey(entity1)); + assertFalse(cache.containsKey(copy1)); + assertTrue(cache.containsKey(entity2)); + assertFalse(cache.containsKey(copy2)); + + assertTrue(cache.invertMap().containsKey(copy1)); + assertFalse(cache.invertMap().containsKey(entity1)); + + assertTrue(cache.invertMap().containsKey(copy2)); + assertFalse(cache.invertMap().containsKey(entity2)); + } + + public void testEntityToCopyFillFollowedByCopyToEntityMappingUsingPutWithSetOperatedOnArg() { + EventCache cache = new EventCache(); + Object entity = new Integer(1); + Object copy = new Integer(2); + + cache.put(entity, copy, true); + assertTrue(cache.containsKey(entity)); + assertFalse(cache.containsKey(copy)); + + assertTrue(cache.invertMap().containsKey(copy)); + assertFalse(cache.invertMap().containsKey(entity)); + + cache.clear(); + cache.put(entity, copy, false); + assertTrue(cache.containsKey(entity)); + assertFalse(cache.containsKey(copy)); + } + + +}