diff --git a/src/main/java/io/ebean/event/BeanQueryRequest.java b/src/main/java/io/ebean/event/BeanQueryRequest.java index ce327e67c..4c13aee54 100644 --- a/src/main/java/io/ebean/event/BeanQueryRequest.java +++ b/src/main/java/io/ebean/event/BeanQueryRequest.java @@ -24,6 +24,11 @@ public interface BeanQueryRequest { */ Query getQuery(); + /** + * Return true if an Id IN expression should have the bind parameters padded. + */ + boolean isPadInExpression(); + /** * Return true if multi-value binding using Array or Table Values is supported. */ diff --git a/src/main/java/io/ebeaninternal/api/LoadBeanRequest.java b/src/main/java/io/ebeaninternal/api/LoadBeanRequest.java index 3a5d9b79e..5f40bc89b 100644 --- a/src/main/java/io/ebeaninternal/api/LoadBeanRequest.java +++ b/src/main/java/io/ebeaninternal/api/LoadBeanRequest.java @@ -4,8 +4,6 @@ import io.ebean.bean.EntityBean; import io.ebean.bean.EntityBeanIntercept; import io.ebeaninternal.server.core.OrmQueryRequest; import io.ebeaninternal.server.deploy.BeanDescriptor; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.HashSet; @@ -17,8 +15,6 @@ import java.util.Set; */ public class LoadBeanRequest extends LoadRequest { - private static final Logger logger = LoggerFactory.getLogger(LoadBeanRequest.class); - private final List batch; private final LoadBeanBuffer loadBuffer; @@ -56,7 +52,7 @@ public class LoadBeanRequest extends LoadRequest { return loadBuffer.getBeanDescriptor().getBeanType(); } - public boolean isLoadCache() { + private boolean isLoadCache() { return loadCache; } @@ -75,17 +71,10 @@ public class LoadBeanRequest extends LoadRequest { /** * Return the load context. */ - public LoadBeanBuffer getLoadContext() { + private LoadBeanBuffer getLoadContext() { return loadBuffer; } - /** - * Return the property that invoked the lazy loading. - */ - public String getLazyLoadProperty() { - return lazyLoadProperty; - } - public int getBatchSize() { return getLoadContext().getBatchSize(); } @@ -93,30 +82,14 @@ public class LoadBeanRequest extends LoadRequest { /** * Return the list of Id values for the beans in the lazy load buffer. */ - public List getIdList(int batchSize) { + public List getIdList() { - List idList = new ArrayList<>(batchSize); + List idList = new ArrayList<>(); BeanDescriptor desc = loadBuffer.getBeanDescriptor(); for (EntityBeanIntercept ebi : batch) { - EntityBean bean = ebi.getOwner(); - idList.add(desc.getId(bean)); + idList.add(desc.getId(ebi.getOwner())); } - - - if (!desc.isMultiValueIdSupported() && !idList.isEmpty()) { - int extraIds = batchSize - batch.size(); - if (extraIds > 0) { - // for performance make up the Id's to the batch size - // so we get the same query (for Ebean and the db) - Object firstId = idList.get(0); - for (int i = 0; i < extraIds; i++) { - // just add the first Id again - idList.add(firstId); - } - } - } - return idList; } diff --git a/src/main/java/io/ebeaninternal/api/LoadManyRequest.java b/src/main/java/io/ebeaninternal/api/LoadManyRequest.java index 1b28ed2c3..0e234536f 100644 --- a/src/main/java/io/ebeaninternal/api/LoadManyRequest.java +++ b/src/main/java/io/ebeaninternal/api/LoadManyRequest.java @@ -3,6 +3,7 @@ package io.ebeaninternal.api; import io.ebean.bean.BeanCollection; import io.ebean.bean.EntityBean; import io.ebean.util.StringHelper; +import io.ebeaninternal.server.core.BindPadding; import io.ebeaninternal.server.core.OrmQueryRequest; import io.ebeaninternal.server.deploy.BeanDescriptor; import io.ebeaninternal.server.deploy.BeanPropertyAssocMany; @@ -65,13 +66,6 @@ public class LoadManyRequest extends LoadRequest { return batch; } - /** - * Return the load context. - */ - public LoadManyBuffer getLoadContext() { - return loadContext; - } - /** * Return true if lazy loading should only load the id values. *

@@ -80,14 +74,14 @@ public class LoadManyRequest extends LoadRequest { * used. *

*/ - public boolean isOnlyIds() { + private boolean isOnlyIds() { return onlyIds; } /** * Return true if we should load the Collection ids into the cache. */ - public boolean isLoadCache() { + private boolean isLoadCache() { return loadCache; } @@ -98,22 +92,16 @@ public class LoadManyRequest extends LoadRequest { return loadContext.getBatchSize(); } - private List getParentIdList(int batchSize) { + private List getParentIdList() { - ArrayList idList = new ArrayList<>(batchSize); + List idList = new ArrayList<>(); BeanPropertyAssocMany many = getMany(); for (BeanCollection bc : batch) { idList.add(many.getParentId(bc.getOwnerBean())); } - if (!many.getTargetDescriptor().isMultiValueIdSupported()) { - int extraIds = batchSize - batch.size(); - if (extraIds > 0) { - Object firstId = idList.get(0); - for (int i = 0; i < extraIds; i++) { - idList.add(firstId); - } - } + if (many.getTargetDescriptor().isPadInExpression()) { + BindPadding.padIds(idList); } return idList; @@ -123,7 +111,7 @@ public class LoadManyRequest extends LoadRequest { return loadContext.getBeanProperty(); } - public SpiQuery createQuery(SpiEbeanServer server, int batchSize) { + public SpiQuery createQuery(SpiEbeanServer server) { BeanPropertyAssocMany many = getMany(); @@ -142,10 +130,7 @@ public class LoadManyRequest extends LoadRequest { } query.setLazyLoadForParents(many); - - List idList = getParentIdList(batchSize); - many.addWhereParentIdIn(query, idList, loadContext.isUseDocStore()); - + many.addWhereParentIdIn(query, getParentIdList(), loadContext.isUseDocStore()); query.setPersistenceContext(loadContext.getPersistenceContext()); String mode = isLazy() ? "+lazy" : "+query"; diff --git a/src/main/java/io/ebeaninternal/server/core/BindPadding.java b/src/main/java/io/ebeaninternal/server/core/BindPadding.java new file mode 100644 index 000000000..75963723e --- /dev/null +++ b/src/main/java/io/ebeaninternal/server/core/BindPadding.java @@ -0,0 +1,57 @@ +package io.ebeaninternal.server.core; + +import java.util.List; + +/** + * Supports padding bind parameters for IN expressions. + *

+ * We do this in order to get better hit ratio on DB query plans. + *

+ */ +public final class BindPadding { + + /** + * Pad out the Ids into common bucket sizes. + * + * @param idCollection The collection of Ids values being bound. + */ + public static void padIds(List idCollection) { + + int extraIds = padding(idCollection.size()); + if (extraIds > 0) { + // for performance make up the Id's to the batch size + // so we get the same query (for Ebean and the db) + Object firstId = idCollection.get(0); + for (int i = 0; i < extraIds; i++) { + // just add the first Id again + idCollection.add(firstId); + } + } + } + + /** + * Extra padding on binding id's in order to get better hit ratio on DB prepared statements / query plans. + */ + static int padding(int size) { + if (size == 1) { + return 0; + } + if (size <= 5) { + return 5 - size; + } + if (size <= 10) { + return 10 - size; + } + if (size <= 20) { + return 20 - size; + } + if (size <= 40) { + return 40 - size; + } + if (size <= 50) { + return 50 - size; + } + return size <= 100 ? 100 - size : 0; + } + +} diff --git a/src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java b/src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java index e66c590d2..0a4487c52 100644 --- a/src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java +++ b/src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java @@ -38,54 +38,14 @@ class DefaultBeanLoader { this.onIterateUseExtraTxn = server.getDatabasePlatform().useExtraTransactionOnIterateSecondaryQueries(); } - /** - * Return a batch size that might be less than the requestedBatchSize. - *

- * This means we can have large and variable requestedBatchSizes. - *

- *

- * We want to restrict the number of different batch sizes as we want to - * re-use the query plan cache and get DB statement re-use. - *

- */ - private int getBatchSize(int batchSize) { - - if (batchSize == 1) { - // there is only one bean/collection to load - return 1; - } - if (batchSize <= 5) { - // anything less than 5 becomes 5 - return 5; - } - if (batchSize <= 10) { - return 10; - } - if (batchSize <= 20) { - return 20; - } - if (batchSize <= 50) { - return 50; - } - if (batchSize <= 100) { - return 100; - } - return batchSize; - } - void refreshMany(EntityBean parentBean, String propertyName) { refreshMany(parentBean, propertyName, null); } void loadMany(LoadManyRequest loadRequest) { - List> batch = loadRequest.getBatch(); - - int batchSize = getBatchSize(batch.size()); - - SpiQuery query = loadRequest.createQuery(server, batchSize); + SpiQuery query = loadRequest.createQuery(server); executeQuery(loadRequest, query); - loadRequest.postLoad(); } @@ -97,7 +57,7 @@ class DefaultBeanLoader { loadManyInternal(parentBean, propertyName, null, false, onlyIds); } - void refreshMany(EntityBean parentBean, String propertyName, Transaction t) { + private void refreshMany(EntityBean parentBean, String propertyName, Transaction t) { loadManyInternal(parentBean, propertyName, t, true, false); } @@ -147,8 +107,7 @@ class DefaultBeanLoader { query.setLoadDescription("+lazy", null); } - String idProperty = parentDesc.getIdBinder().getIdProperty(); - query.select(idProperty); + query.select(parentDesc.getIdBinder().getIdProperty()); if (onlyIds) { query.fetch(many.getName(), many.getTargetIdProperty()); @@ -192,9 +151,7 @@ class DefaultBeanLoader { throw new RuntimeException("Nothing in batch?"); } - int batchSize = getBatchSize(batch.size()); - - List idList = loadRequest.getIdList(batchSize); + List idList = loadRequest.getIdList(); if (idList.isEmpty()) { // everything was loaded from cache return; @@ -315,6 +272,5 @@ class DefaultBeanLoader { } desc.resetManyProperties(dbBean); - } } diff --git a/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java b/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java index b7543c76d..4a9fa0625 100644 --- a/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java +++ b/src/main/java/io/ebeaninternal/server/core/OrmQueryRequest.java @@ -115,6 +115,11 @@ public final class OrmQueryRequest extends BeanRequest implements SpiOrmQuery } } + @Override + public boolean isPadInExpression() { + return beanDescriptor.isPadInExpression(); + } + @Override public boolean isMultiValueIdSupported() { return beanDescriptor.isMultiValueIdSupported(); diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java index cb38766e4..ae11750bb 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java @@ -1835,6 +1835,13 @@ public class BeanDescriptor implements BeanType, STreeType { return multiValueSupported && isSimpleId(); } + /** + * Return true if Id IN expression should have bind parameters padded. + */ + public boolean isPadInExpression() { + return !multiValueSupported && isSimpleId(); + } + /** * Return the sql for binding an id. This is the columns with table alias that * make up the id. diff --git a/src/main/java/io/ebeaninternal/server/expression/IdInExpression.java b/src/main/java/io/ebeaninternal/server/expression/IdInExpression.java index e37e9d150..d9d82d4e3 100644 --- a/src/main/java/io/ebeaninternal/server/expression/IdInExpression.java +++ b/src/main/java/io/ebeaninternal/server/expression/IdInExpression.java @@ -5,6 +5,7 @@ import io.ebeaninternal.api.ManyWhereJoins; import io.ebeaninternal.api.SpiExpression; import io.ebeaninternal.api.SpiExpressionRequest; import io.ebeaninternal.api.SpiExpressionValidation; +import io.ebeaninternal.server.core.BindPadding; import io.ebeaninternal.server.deploy.BeanDescriptor; import io.ebeaninternal.server.deploy.id.IdBinder; @@ -46,6 +47,10 @@ public class IdInExpression extends NonPrepareExpression { @Override public void prepareExpression(BeanQueryRequest request) { multiValueIdSupported = request.isMultiValueIdSupported(); + if (!multiValueIdSupported && !idCollection.isEmpty() && request.isPadInExpression()) { + // pad out the ids for better hit ratio on DB query plans + BindPadding.padIds(idCollection); + } } @Override diff --git a/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java b/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java index d919a251c..492a0902c 100644 --- a/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java +++ b/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java @@ -1,5 +1,6 @@ package io.ebeaninternal.server.loadcontext; +import io.ebean.CacheMode; import io.ebean.bean.BeanLoader; import io.ebean.bean.EntityBeanIntercept; import io.ebean.bean.PersistenceContext; @@ -20,6 +21,8 @@ import java.util.Set; */ class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext { + private final boolean cache; + private List bufferList; private LoadBuffer currentBuffer; @@ -29,6 +32,7 @@ class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext { // bufferList only required when using query joins (queryFetch) this.bufferList = (!queryFetch) ? null : new ArrayList<>(); this.currentBuffer = createBuffer(firstBatchSize); + this.cache = (queryProps == null) ? false : queryProps.isCache(); } /** @@ -43,6 +47,9 @@ class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext { private void configureQuery(SpiQuery query, String lazyLoadProperty) { + if (cache) { + query.setBeanCacheMode(CacheMode.ON); + } setLabel(query); parent.propagateQueryState(query, desc.isDocStoreMapped()); query.setParentNode(objectGraphNode); diff --git a/src/test/java/io/ebeaninternal/server/core/BindPaddingTest.java b/src/test/java/io/ebeaninternal/server/core/BindPaddingTest.java new file mode 100644 index 000000000..d0363d2c6 --- /dev/null +++ b/src/test/java/io/ebeaninternal/server/core/BindPaddingTest.java @@ -0,0 +1,51 @@ +package io.ebeaninternal.server.core; + +import org.junit.Test; + +import java.util.ArrayList; +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; + +public class BindPaddingTest { + + @Test + public void padIds() { + + final List input = asList(1, 2); + BindPadding.padIds(input); + assertThat(input).contains(1,2,1,1,1); + assertThat(input).hasSize(5); + } + + private List asList(int... id) { + List list = new ArrayList<>(); + for (int i : id) { + list.add(i); + } + return list; + } + + @Test + public void padding() { + + assertEquals(0, BindPadding.padding(1)); + assertEquals(3, BindPadding.padding(2)); + assertEquals(2, BindPadding.padding(3)); + assertEquals(1, BindPadding.padding(4)); + assertEquals(0, BindPadding.padding(5)); + assertEquals(4, BindPadding.padding(6)); + assertEquals(1, BindPadding.padding(9)); + assertEquals(0, BindPadding.padding(10)); + assertEquals(9, BindPadding.padding(11)); + assertEquals(0, BindPadding.padding(20)); + assertEquals(19, BindPadding.padding(21)); + assertEquals(0, BindPadding.padding(40)); + assertEquals(9, BindPadding.padding(41)); + assertEquals(0, BindPadding.padding(50)); + assertEquals(49, BindPadding.padding(51)); + assertEquals(0, BindPadding.padding(100)); + assertEquals(0, BindPadding.padding(101)); + } +} diff --git a/src/test/java/io/ebeaninternal/server/expression/BaseExpressionTest.java b/src/test/java/io/ebeaninternal/server/expression/BaseExpressionTest.java index a9a8e939a..d1b01da21 100644 --- a/src/test/java/io/ebeaninternal/server/expression/BaseExpressionTest.java +++ b/src/test/java/io/ebeaninternal/server/expression/BaseExpressionTest.java @@ -20,18 +20,18 @@ public abstract class BaseExpressionTest extends BaseTestCase { } protected String hash(SpiExpression expression) { - StringBuilder sb = new StringBuilder(); + StringBuilder sb = new StringBuilder(); if (expression != null) { expression.queryPlanHash(sb); } return sb.toString(); } - protected void same(SpiExpression one, SpiExpression two){ + protected void same(SpiExpression one, SpiExpression two) { assertThat(hash(one)).isEqualTo(hash(two)); } - protected void different(SpiExpression one, SpiExpression two){ + protected void different(SpiExpression one, SpiExpression two) { assertThat(hash(one)).isNotEqualTo(hash(two)); } @@ -50,7 +50,7 @@ public abstract class BaseExpressionTest extends BaseTestCase { } - private static final TDQueryRequest MULTI_VALUE= new TDQueryRequest<>(true); + private static final TDQueryRequest MULTI_VALUE = new TDQueryRequest<>(true); private static final TDQueryRequest NO_MULTI_VALUE = new TDQueryRequest<>(false); static class TDQueryRequest implements BeanQueryRequest { @@ -76,6 +76,11 @@ public abstract class BaseExpressionTest extends BaseTestCase { return null; } + @Override + public boolean isPadInExpression() { + return supported; + } + @Override public boolean isMultiValueIdSupported() { return supported; diff --git a/src/test/java/org/tests/cache/TestBeanCache.java b/src/test/java/org/tests/cache/TestBeanCache.java index 783c300a5..448b824a9 100644 --- a/src/test/java/org/tests/cache/TestBeanCache.java +++ b/src/test/java/org/tests/cache/TestBeanCache.java @@ -76,7 +76,7 @@ public class TestBeanCache extends BaseTestCase { List sql = LoggedSqlCollector.current(); assertThat(sql).hasSize(1); if (isH2()) { - assertThat(sql.get(0)).contains("from o_cached_bean t0 where t0.id in (?,?,?)"); + assertThat(sql.get(0)).contains("from o_cached_bean t0 where t0.id in (?,?,?,?,?)"); } log.info("All hits (3 of 3) ..."); @@ -124,7 +124,7 @@ public class TestBeanCache extends BaseTestCase { assertThat(sql).hasSize(1); if (isH2()) { // fetch the misses from DB - assertThat(sql.get(0)).contains("from o_cached_bean t0 where t0.id in (?,?)"); + assertThat(sql.get(0)).contains("from o_cached_bean t0 where t0.id in (?,?,?,?,?)"); } } diff --git a/src/test/java/org/tests/cache/TestBeanFetchJoinCache.java b/src/test/java/org/tests/cache/TestBeanFetchJoinCache.java new file mode 100644 index 000000000..118d0e114 --- /dev/null +++ b/src/test/java/org/tests/cache/TestBeanFetchJoinCache.java @@ -0,0 +1,45 @@ +package org.tests.cache; + +import io.ebean.BaseTestCase; +import io.ebean.DB; +import org.junit.Test; +import org.tests.model.basic.Customer; +import org.tests.model.basic.Order; +import org.tests.model.basic.ResetBasicData; + +import java.util.List; + +import static io.ebean.CacheMode.ON; + +public class TestBeanFetchJoinCache extends BaseTestCase { + + @Test + public void test() { + + ResetBasicData.reset(); + +// DB.find(Customer.class) +// .setBeanCacheMode(ON) +// .findList(); + + + DB.find(Customer.class) + .where().idIn(1,2,3) + //.setBeanCacheMode(ON) + .findList(); + + List orders = DB.find(Order.class) + .fetchQuery("customer", "name") + .findList(); + + + orders = DB.find(Order.class) + .fetchQuery("customer", "+cache, name") + .findList(); + + for (Order order : orders) { + order.getCustomer().getName(); + } + + } +} diff --git a/src/test/resources/logback-test.xml b/src/test/resources/logback-test.xml index 0e19a7c4b..7dec55bb6 100644 --- a/src/test/resources/logback-test.xml +++ b/src/test/resources/logback-test.xml @@ -80,10 +80,10 @@ - - + + - +