diff --git a/src/main/java/io/ebeaninternal/server/persist/BatchControl.java b/src/main/java/io/ebeaninternal/server/persist/BatchControl.java index 0b6fd3a66..0b3bd28b3 100644 --- a/src/main/java/io/ebeaninternal/server/persist/BatchControl.java +++ b/src/main/java/io/ebeaninternal/server/persist/BatchControl.java @@ -5,12 +5,12 @@ import io.ebeaninternal.server.core.PersistRequest; import io.ebeaninternal.server.core.PersistRequestBean; import io.ebeaninternal.server.core.PersistRequestUpdateSql; import io.ebeaninternal.server.deploy.BeanDescriptor; -import io.ebeaninternal.server.deploy.BeanPropertyAssocOne; import java.sql.SQLException; import java.util.ArrayList; import java.util.Arrays; import java.util.HashMap; +import java.util.IdentityHashMap; import java.util.List; /** @@ -29,6 +29,8 @@ import java.util.List; */ public final class BatchControl { + private static final Object DUMMY = new Object(); + /** * Used to sort queue entries by depth. */ @@ -46,6 +48,17 @@ public final class BatchControl { */ private final HashMap beanHoldMap = new HashMap<>(); + /** + * Set of beans in this batch. This is used to ensure that a single bean instance is not included + * in the batch twice (two separate insert requests etc). + */ + private final IdentityHashMap persistedBeans = new IdentityHashMap<>(); + + /** + * Helper to determine statement ordering based on depth (and type). + */ + private final BatchDepthOrder depthOrder = new BatchDepthOrder(); + private final SpiTransaction transaction; /** @@ -63,13 +76,10 @@ public final class BatchControl { private boolean batchFlushOnMixed = true; - private int maxDepth; - /** * Size of the largest buffer. */ private int bufferMax; - private int topCounter; private Queue earlyQueue; private Queue lateQueue; @@ -183,6 +193,14 @@ public final class BatchControl { */ private boolean addToBatch(PersistRequestBean request) throws BatchedSqlException { + Object alreadyInBatch = persistedBeans.put(request.getEntityBean(), DUMMY); + if (alreadyInBatch != null) { + // special case where the same bean instance has already been + // added to the batch (doesn't really occur with non-batching + // as the bean gets changed from dirty to loaded earlier) + return false; + } + BatchedBeanHolder beanHolder = getBeanHolder(request); int bufferSize = beanHolder.append(request); @@ -227,14 +245,14 @@ public final class BatchControl { } /** - * Flush without resetting the topOrder (maintains the depth info). + * Flush without resetting the depth info. */ public void flush() throws BatchedSqlException { flushBuffer(false); } /** - * Flush with a reset the topOrder (fully empty the batch). + * Flush with a reset of the depth info. */ public void flushReset() throws BatchedSqlException { flushBuffer(true); @@ -246,8 +264,8 @@ public final class BatchControl { public void clear() { pstmtHolder.clear(); beanHoldMap.clear(); - maxDepth = 0; - topCounter = 0; + depthOrder.clear(); + persistedBeans.clear(); } private void flushBuffer(boolean resetTop) throws BatchedSqlException { @@ -289,10 +307,10 @@ public final class BatchControl { for (BatchedBeanHolder beanHolder : bsArray) { beanHolder.executeNow(); } - + persistedBeans.clear(); if (resetTop) { beanHoldMap.clear(); - maxDepth = 0; + depthOrder.clear(); } } catch (BatchedSqlException e) { // clear the batch on error in case we want to @@ -306,69 +324,28 @@ public final class BatchControl { * Return an entry for the given type description. The type description is * typically the bean class name (or table name for MapBeans). */ - private BatchedBeanHolder getBeanHolder(PersistRequestBean request) throws BatchedSqlException { + private BatchedBeanHolder getBeanHolder(PersistRequestBean request) { - BeanDescriptor beanDescriptor = request.getBeanDescriptor(); - BatchedBeanHolder batchBeanHolder = beanHoldMap.get(beanDescriptor.rootName()); + int depth = transaction.depth(); + BeanDescriptor desc = request.getBeanDescriptor(); + + // batching by bean type AND depth + String key = desc.rootName() + ":" + depth; + + BatchedBeanHolder batchBeanHolder = beanHoldMap.get(key); if (batchBeanHolder == null) { - int relativeDepth = transaction.depth(); - - int beanDepth = 100 + relativeDepth; - if (relativeDepth == 0 && !beanHoldMap.isEmpty()) { - // could be non-cascading or uni-directional relationship - // so see look for a 'parent' in the beanHoldMap - int maybe = relativeToParentDepth(beanDescriptor); - if (maybe != -1) { - beanDepth = maybe; - } else { - // additional "top level" bean type ordered by save() order - beanDepth += ++topCounter; - } - } - - maxDepth = Math.max(maxDepth, beanDepth); - batchBeanHolder = new BatchedBeanHolder(this, beanDescriptor, beanDepth); - beanHoldMap.put(beanDescriptor.rootName(), batchBeanHolder); + int ordering = depthOrder.orderingFor(depth); + batchBeanHolder = new BatchedBeanHolder(this, desc, ordering); + beanHoldMap.put(key, batchBeanHolder); } return batchBeanHolder; } - /** - * Find a depth based on imported relationships (to a parent that is already in the buffer). - */ - private int relativeToParentDepth(BeanDescriptor beanDescriptor) { - - BeanPropertyAssocOne[] imported = beanDescriptor.propertiesOneImported(); - if (imported.length == 0) { - // a top level type so just maintain the order relative to the current depth - return maxDepth + 1; - } - - int parentMaxDepth = -1; - - for (BeanPropertyAssocOne parent : imported) { - BatchedBeanHolder parentBatch = beanHoldMap.get(parent.getTargetDescriptor().rootName()); - if (parentBatch != null) { - // deeper that the parent - parentMaxDepth = Math.max(parentMaxDepth, parentBatch.getOrder() + 1); - } - } - return parentMaxDepth; - } - /** * Return true if this holds no persist requests. */ private boolean isBeansEmpty() { - if (beanHoldMap.isEmpty()) { - return true; - } - for (BatchedBeanHolder beanHolder : beanHoldMap.values()) { - if (!beanHolder.isEmpty()) { - return false; - } - } - return true; + return persistedBeans.isEmpty(); } /** diff --git a/src/main/java/io/ebeaninternal/server/persist/BatchDepthOrder.java b/src/main/java/io/ebeaninternal/server/persist/BatchDepthOrder.java new file mode 100644 index 000000000..7fe9c10b5 --- /dev/null +++ b/src/main/java/io/ebeaninternal/server/persist/BatchDepthOrder.java @@ -0,0 +1,33 @@ +package io.ebeaninternal.server.persist; + +import java.util.HashMap; +import java.util.Map; + +/** + * Helper to determine batch execution order for BatchedBeanHolders. + */ +class BatchDepthOrder { + + private final Map map = new HashMap<>(); + + /** + * Return the batch order for the given depth. + */ + int orderingFor(int depth) { + final Counter slot = map.computeIfAbsent(depth, integer -> new Counter()); + return (depth * 100) + slot.increment(); + } + + void clear() { + map.clear(); + } + + private static class Counter { + + int count; + + public int increment() { + return count++; + } + } +} diff --git a/src/main/java/io/ebeaninternal/server/persist/BatchedBeanHolder.java b/src/main/java/io/ebeaninternal/server/persist/BatchedBeanHolder.java index 61c46ef0c..b32878494 100644 --- a/src/main/java/io/ebeaninternal/server/persist/BatchedBeanHolder.java +++ b/src/main/java/io/ebeaninternal/server/persist/BatchedBeanHolder.java @@ -5,7 +5,6 @@ import io.ebeaninternal.server.core.PersistRequestBean; import io.ebeaninternal.server.deploy.BeanDescriptor; import java.util.ArrayList; -import java.util.IdentityHashMap; /** * Holds lists of persist requests for beans of a given type. @@ -21,8 +20,6 @@ import java.util.IdentityHashMap; */ public class BatchedBeanHolder { - private static final Object DUMMY = new Object(); - /** * The owning queue. */ @@ -50,12 +47,6 @@ public class BatchedBeanHolder { */ private ArrayList deletes; - /** - * Set of beans in this batch. This is used to ensure that a single bean instance is not included - * in the batch twice (two separate insert requests etc). - */ - private final IdentityHashMap persistedBeans = new IdentityHashMap<>(); - /** * Create a new entry with a given type and depth. */ @@ -99,7 +90,6 @@ public class BatchedBeanHolder { updates = new ArrayList<>(); control.executeNow(bufferedUpdates); } - persistedBeans.clear(); } @Override @@ -123,14 +113,6 @@ public class BatchedBeanHolder { */ public int append(PersistRequestBean request) { - Object alreadyInBatch = persistedBeans.put(request.getEntityBean(), DUMMY); - if (alreadyInBatch != null) { - // special case where the same bean instance has already been - // added to the batch (doesn't really occur with non-batching - // as the bean gets changed from dirty to loaded earlier) - return 0; - } - request.setBatched(); switch (request.getType()) { @@ -161,10 +143,4 @@ public class BatchedBeanHolder { } } - /** - * Return true if this is empty containing no batched beans. - */ - public boolean isEmpty() { - return persistedBeans.isEmpty(); - } } diff --git a/src/test/java/io/ebeaninternal/server/persist/BatchDepthOrderTest.java b/src/test/java/io/ebeaninternal/server/persist/BatchDepthOrderTest.java new file mode 100644 index 000000000..d9640e7da --- /dev/null +++ b/src/test/java/io/ebeaninternal/server/persist/BatchDepthOrderTest.java @@ -0,0 +1,48 @@ +package io.ebeaninternal.server.persist; + +import org.junit.Test; + +import static org.junit.Assert.*; + +public class BatchDepthOrderTest { + + @Test + public void orderingFor() { + + BatchDepthOrder depthOrder = new BatchDepthOrder(); + + assertEquals(0, depthOrder.orderingFor(0)); + assertEquals(1, depthOrder.orderingFor(0)); + assertEquals(2, depthOrder.orderingFor(0)); + assertEquals(100, depthOrder.orderingFor(1)); + assertEquals(101, depthOrder.orderingFor(1)); + assertEquals(200, depthOrder.orderingFor(2)); + assertEquals(3, depthOrder.orderingFor(0)); + + assertEquals(-100, depthOrder.orderingFor(-1)); + assertEquals(-99, depthOrder.orderingFor(-1)); + assertEquals(-98, depthOrder.orderingFor(-1)); + + assertEquals(-200, depthOrder.orderingFor(-2)); + assertEquals(-199, depthOrder.orderingFor(-2)); + } + + + @Test + public void clear() { + + BatchDepthOrder depthOrder = new BatchDepthOrder(); + + assertEquals(0, depthOrder.orderingFor(0)); + assertEquals(1, depthOrder.orderingFor(0)); + assertEquals(100, depthOrder.orderingFor(1)); + assertEquals(101, depthOrder.orderingFor(1)); + + depthOrder.clear(); + + assertEquals(0, depthOrder.orderingFor(0)); + assertEquals(100, depthOrder.orderingFor(1)); + assertEquals(200, depthOrder.orderingFor(2)); + } + +} diff --git a/src/test/java/io/ebeaninternal/server/persist/BatchedBeanHolderTest.java b/src/test/java/io/ebeaninternal/server/persist/BatchedBeanHolderTest.java deleted file mode 100644 index 12a7acef0..000000000 --- a/src/test/java/io/ebeaninternal/server/persist/BatchedBeanHolderTest.java +++ /dev/null @@ -1,45 +0,0 @@ -package io.ebeaninternal.server.persist; - -import io.ebean.BaseTestCase; -import io.ebeaninternal.api.TDSpiEbeanServer; -import io.ebeaninternal.server.core.PersistRequest; -import io.ebeaninternal.server.core.PersistRequestBean; -import io.ebeaninternal.server.deploy.BeanDescriptor; -import io.ebeaninternal.server.deploy.BeanManager; -import org.junit.Test; -import org.tests.model.basic.Customer; - -import java.sql.Timestamp; - -import static org.junit.Assert.assertEquals; - -public class BatchedBeanHolderTest extends BaseTestCase { - - @SuppressWarnings({ "rawtypes", "unchecked" }) - @Test - public void testAppend() { - - TDSpiEbeanServer server = new TDSpiEbeanServer("foo"); - - BeanDescriptor beanDescriptor = getBeanDescriptor(Customer.class); - - //BeanDescriptor beanDescriptor = Mockito.mock(BeanDescriptor.class); - BeanManager beanManager = new BeanManager(beanDescriptor, null); - - - BatchedBeanHolder holder = new BatchedBeanHolder(null, beanDescriptor, 1); - - Customer customer = new Customer(); - customer.setUpdtime(new Timestamp(System.currentTimeMillis())); - PersistRequestBean req1 = new PersistRequestBean(server, customer, null, beanManager, null, null, PersistRequest.Type.INSERT, 0); - - - int size = holder.append(req1); - assertEquals(1, size); - - PersistRequestBean req2 = new PersistRequestBean(server, customer, null, beanManager, null, null, PersistRequest.Type.INSERT, 0); - size = holder.append(req2); - assertEquals(0, size); - - } -} diff --git a/src/test/java/org/tests/cascade/TestMultiCascadeBatch.java b/src/test/java/org/tests/cascade/TestMultiCascadeBatch.java index 1e23d0cb0..d2733ddc3 100644 --- a/src/test/java/org/tests/cascade/TestMultiCascadeBatch.java +++ b/src/test/java/org/tests/cascade/TestMultiCascadeBatch.java @@ -28,6 +28,7 @@ public class TestMultiCascadeBatch extends BaseTestCase { @Test public void testMultipleCascadeInsideTransaction() { + final Site mainSite = new Site(); mainSite.setName("mainSite"); Ebean.save(mainSite); diff --git a/src/test/java/org/tests/model/basic/xtra/TestInsertBatchThenFlushThenUpdate.java b/src/test/java/org/tests/model/basic/xtra/TestInsertBatchThenFlushThenUpdate.java index 294e8d898..6fe2e0d18 100644 --- a/src/test/java/org/tests/model/basic/xtra/TestInsertBatchThenFlushThenUpdate.java +++ b/src/test/java/org/tests/model/basic/xtra/TestInsertBatchThenFlushThenUpdate.java @@ -38,7 +38,7 @@ public class TestInsertBatchThenFlushThenUpdate extends BaseTestCase { Ebean.save(parent); // nothing flushed yet - assertEquals(0, LoggedSqlCollector.start().size()); + assertThat(LoggedSqlCollector.start()).isEmpty(); txn.flushBatch(); @@ -49,14 +49,14 @@ public class TestInsertBatchThenFlushThenUpdate extends BaseTestCase { Ebean.save(parent); // nothing flushed yet - assertEquals(0, LoggedSqlCollector.start().size()); + assertThat(LoggedSqlCollector.start()).isEmpty(); Ebean.commitTransaction(); // insert statements for EdExtendedParent List loggedSql2 = LoggedSqlCollector.start(); - assertEquals(2, loggedSql2.size()); - assertTrue(loggedSql2.get(0).contains(" update td_parent ")); + assertThat(loggedSql2).hasSize(2); + assertThat(loggedSql2.get(0)).contains(" update td_parent "); } }