From 8ad8f47c6b9c791da28659798b783210bd78434a Mon Sep 17 00:00:00 2001 From: Noemi Szemenyei Date: Tue, 15 Mar 2022 10:24:36 +0100 Subject: [PATCH 1/3] Created failing test case (cherry picked from commit 2b21588c903aa526c5586648ea35a5955673bfb9) --- .../TestWithCacheAndLazyLoad.java | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/ebean-test/src/test/java/org/tests/model/lazywithcache/TestWithCacheAndLazyLoad.java b/ebean-test/src/test/java/org/tests/model/lazywithcache/TestWithCacheAndLazyLoad.java index 380f82c5d..8c8066dbc 100644 --- a/ebean-test/src/test/java/org/tests/model/lazywithcache/TestWithCacheAndLazyLoad.java +++ b/ebean-test/src/test/java/org/tests/model/lazywithcache/TestWithCacheAndLazyLoad.java @@ -2,9 +2,14 @@ package org.tests.model.lazywithcache; import io.ebean.BaseTestCase; import io.ebean.DB; +import io.ebean.Transaction; + import org.junit.jupiter.api.Test; import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertNotNull; + +import java.util.List; /** * Test with bean cache and lazy loaded property. @@ -47,5 +52,37 @@ public class TestWithCacheAndLazyLoad extends BaseTestCase{ assertThat(temp.getAddress()).isEqualTo("Address"); assertThat(temp.getName()).isEqualTo("Child With Cache"); } + + @Test + public void testBatch() throws Exception { + + for (int i = 0; i < 2; i++) { + ChildWithCache child = new ChildWithCache(); + child.setId(1000L+i); + child.setName("Child With Cache "+ i); + child.setAddress("Address"); + DB.save(child); + + ParentA parentA = new ParentA(); + parentA.setId(1000L+i); + parentA.setName("Parent A"); + parentA.setChild(child); + DB.save(parentA); + } + // 0. load cache + ParentA temp = DB.find(ParentA.class, 1000L); + temp.getChild().getName(); + + try (Transaction txn = DB.beginTransaction()) { + // 1. FindList + List list = DB.find(ParentA.class).where().ge("id", 1000L).orderBy("id").findList(); + // 2. FindById (first element from list) + temp = DB.find(ParentA.class, 1000L); + assertNotNull(temp.getChild().getName()); + // 3. access elements in list + assertNotNull(list.get(0).getChild().getAddress()); + assertNotNull(list.get(1).getChild().getAddress()); + } + } } From 826b507b773afd33bc3328ae0bfc83e921b80022 Mon Sep 17 00:00:00 2001 From: Noemi Szemenyei Date: Tue, 15 Mar 2022 10:36:36 +0100 Subject: [PATCH 2/3] Suggested fix (cherry picked from commit 52ca271a13906971f0769e98c465c5cc87cb06d0) --- .../io/ebeaninternal/server/loadcontext/DLoadBeanContext.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java b/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java index 427d59de2..54c4dd84c 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java @@ -190,7 +190,7 @@ final class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext // lazy load property was a Many return; } - if (list.isEmpty()) { + if (!list.contains(ebi)) { // re-add to the batch and lazy load from DB skipping l2 cache list.add(ebi); } else if (context.hitCache) { From edb6a0ebe2d5171157949dd01beaf91934cbd5c0 Mon Sep 17 00:00:00 2001 From: Noemi Szemenyei Date: Tue, 15 Mar 2022 10:46:22 +0100 Subject: [PATCH 3/3] performance boost list --> set (cherry picked from commit 2478aded70357e38efe98307c9362e240bcbed2b) --- .../io/ebeaninternal/api/LoadBeanBuffer.java | 4 +-- .../io/ebeaninternal/api/LoadBeanRequest.java | 4 +-- .../server/core/DefaultBeanLoader.java | 3 ++- .../server/deploy/BeanDescriptor.java | 4 +-- .../deploy/BeanDescriptorCacheHelp.java | 4 +-- .../server/loadcontext/DLoadBeanContext.java | 27 ++++++++++--------- 6 files changed, 24 insertions(+), 22 deletions(-) diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/LoadBeanBuffer.java b/ebean-core/src/main/java/io/ebeaninternal/api/LoadBeanBuffer.java index 3b999bc3c..e19b2d3c0 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/LoadBeanBuffer.java +++ b/ebean-core/src/main/java/io/ebeaninternal/api/LoadBeanBuffer.java @@ -4,7 +4,7 @@ import io.ebean.bean.EntityBeanIntercept; import io.ebean.bean.PersistenceContext; import io.ebeaninternal.server.deploy.BeanDescriptor; -import java.util.List; +import java.util.Set; /** * A buffer of beans for batch lazy loading and secondary query loading. @@ -13,7 +13,7 @@ public interface LoadBeanBuffer { int batchSize(); - List batch(); + Set batch(); BeanDescriptor descriptor(); diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/LoadBeanRequest.java b/ebean-core/src/main/java/io/ebeaninternal/api/LoadBeanRequest.java index ffe28462c..092050e7e 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/LoadBeanRequest.java +++ b/ebean-core/src/main/java/io/ebeaninternal/api/LoadBeanRequest.java @@ -16,7 +16,7 @@ import java.util.Set; */ public final class LoadBeanRequest extends LoadRequest { - private final List batch; + private final Set batch; private final LoadBeanBuffer loadBuffer; private final String lazyLoadProperty; private final boolean loadCache; @@ -58,7 +58,7 @@ public final class LoadBeanRequest extends LoadRequest { /** * Return the batch of beans to actually load. */ - public List batch() { + public Set batch() { return batch; } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java index bc03d253c..4a2ffb68c 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/core/DefaultBeanLoader.java @@ -17,6 +17,7 @@ import org.slf4j.Logger; import javax.persistence.EntityNotFoundException; import java.util.List; +import java.util.Set; /** * Helper to handle lazy loading and refreshing of beans. @@ -121,7 +122,7 @@ final class DefaultBeanLoader { * Load a batch of beans for +query or +lazy loading. */ void loadBean(LoadBeanRequest loadRequest) { - List batch = loadRequest.batch(); + Set batch = loadRequest.batch(); if (batch.isEmpty()) { throw new RuntimeException("Nothing in batch?"); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java index c06dfce3e..761e2e3a5 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java @@ -1310,8 +1310,8 @@ public class BeanDescriptor implements BeanType, STreeType, SpiBeanType { * Hit the bean cache trying to load a list/batch of entities. * Return the set of entities that were successfully loaded from L2 cache. */ - public Set cacheBeanLoadAll(List list, PersistenceContext persistenceContext, int lazyLoadProperty, String propertyName) { - return cacheHelp.beanCacheLoadAll(list, persistenceContext, lazyLoadProperty, propertyName); + public Set cacheBeanLoadAll(Set batch, PersistenceContext persistenceContext, int lazyLoadProperty, String propertyName) { + return cacheHelp.beanCacheLoadAll(batch, persistenceContext, lazyLoadProperty, propertyName); } /** diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java index 901f8dff3..725a850e2 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java @@ -693,9 +693,9 @@ final class BeanDescriptorCacheHelp { /** * Load a batch of entities from L2 bean cache checking the lazy loaded property is loaded. */ - Set beanCacheLoadAll(List list, PersistenceContext context, int lazyLoadProperty, String propertyName) { + Set beanCacheLoadAll(Set batch, PersistenceContext context, int lazyLoadProperty, String propertyName) { Map ebis = new HashMap<>(); - for (EntityBeanIntercept ebi : list) { + for (EntityBeanIntercept ebi : batch) { ebis.put(desc.cacheKeyForBean(ebi.getOwner()), ebi); } diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java b/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java index 54c4dd84c..a8533956c 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/loadcontext/DLoadBeanContext.java @@ -15,6 +15,7 @@ import io.ebeaninternal.server.deploy.BeanPropertyAssocMany; import io.ebeaninternal.server.querydefn.OrmQueryProperties; import java.util.ArrayList; +import java.util.HashSet; import java.util.List; import java.util.Set; import java.util.concurrent.locks.Lock; @@ -91,7 +92,7 @@ final class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext try { if (bufferList != null) { for (LoadBuffer loadBuffer : bufferList) { - if (!loadBuffer.list.isEmpty()) { + if (!loadBuffer.batch.isEmpty()) { parent.getEbeanServer().loadBean(new LoadBeanRequest(loadBuffer, parentRequest)); } if (forEach) { @@ -115,13 +116,13 @@ final class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext private final ReentrantLock bufferLock = new ReentrantLock(); private final DLoadBeanContext context; private final int batchSize; - private final List list; + private final Set batch; private PersistenceContext persistenceContext; LoadBuffer(DLoadBeanContext context, int batchSize) { this.context = context; this.batchSize = batchSize; - this.list = new ArrayList<>(batchSize); + this.batch = new HashSet<>(Math.max((int) (batchSize/.75f) + 1, 16)); } @Override @@ -139,7 +140,7 @@ final class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext * Return true if the buffer is full. */ public boolean isFull() { - return batchSize == list.size(); + return batchSize == batch.size(); } /** @@ -150,12 +151,12 @@ final class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext // get persistenceContext from first loaded bean into the buffer persistenceContext = ebi.getPersistenceContext(); } - list.add(ebi); + batch.add(ebi); } @Override - public List batch() { - return list; + public Set batch() { + return batch; } @Override @@ -190,13 +191,13 @@ final class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext // lazy load property was a Many return; } - if (!list.contains(ebi)) { + if (!batch.contains(ebi)) { // re-add to the batch and lazy load from DB skipping l2 cache - list.add(ebi); + batch.add(ebi); } else if (context.hitCache) { - Set hits = context.desc.cacheBeanLoadAll(list, persistenceContext, ebi.getLazyLoadPropertyIndex(), ebi.getLazyLoadProperty()); - list.removeAll(hits); - if (list.isEmpty() || hits.contains(ebi)) { + Set hits = context.desc.cacheBeanLoadAll(batch, persistenceContext, ebi.getLazyLoadPropertyIndex(), ebi.getLazyLoadProperty()); + batch.removeAll(hits); + if (batch.isEmpty() || hits.contains(ebi)) { // successfully hit the L2 cache so don't invoke DB lazy loading return; } @@ -204,7 +205,7 @@ final class DLoadBeanContext extends DLoadBaseContext implements LoadBeanContext LoadBeanRequest req = new LoadBeanRequest(this, ebi, context.hitCache); context.desc.ebeanServer().loadBean(req); - list.clear(); + batch.clear(); } }