#1922 - Performance - Too pessimistic synchronized block for lazy loading on reference bean or l2 cache bean

src/main/java/io/ebean/bean/EntityBeanIntercept.java

@@ -1,6 +1,7 @@
 package io.ebean.bean;
 
 import io.ebean.DB;
+import io.ebean.Database;
 import io.ebean.ValuePair;
 
 import javax.persistence.EntityNotFoundException;
@@ -104,9 +105,6 @@ public final class EntityBeanIntercept implements Serializable {
 
   /**
    * Create a intercept with a given entity.
-   * <p>
-   * Refer to agent ProxyConstructor.
-   * </p>
    */
   public EntityBeanIntercept(Object ownerBean) {
     this.owner = (EntityBean) ownerBean;
@@ -821,14 +819,14 @@ public final class EntityBeanIntercept implements Serializable {
 
     synchronized (this) {
       if (beanLoader == null) {
-        BeanLoader serverLoader = (BeanLoader) DB.byName(ebeanServerName);
-        if (serverLoader == null) {
-          throw new PersistenceException("Server [" + ebeanServerName + "] was not found?");
+        final Database database = DB.byName(ebeanServerName);
+        if (database == null) {
+          throw new PersistenceException("Database [" + ebeanServerName + "] was not found?");
         }
 
         // For stand alone reference bean or after deserialisation lazy load
         // using the ebeanServer. Synchronise only on the bean.
-        loadBeanInternal(loadProperty, serverLoader);
+        loadBeanInternal(loadProperty, database.getPluginApi());
         return;
       }
     }

src/main/java/io/ebean/bean/SingleBeanLoader.java

@@ -0,0 +1,48 @@
+package io.ebean.bean;
+
+import io.ebean.Database;
+
+/**
+ * BeanLoader used when single beans are loaded (which is usually not ideal / N+1).
+ */
+public abstract class SingleBeanLoader implements BeanLoader {
+
+  protected final Database database;
+
+  SingleBeanLoader(Database database) {
+    this.database = database;
+  }
+
+  @Override
+  public String getName() {
+    return database.getName();
+  }
+
+  /**
+   * Single bean lazy loaded when bean from L2 cache.
+   */
+  public static class L2 extends SingleBeanLoader {
+    public L2(Database database) {
+      super(database);
+    }
+
+    @Override
+    public void loadBean(EntityBeanIntercept ebi) {
+      database.getPluginApi().loadBeanL2(ebi);
+    }
+  }
+
+  /**
+   * Single bean lazy loaded when a reference bean.
+   */
+  public static class Ref extends SingleBeanLoader {
+    public Ref(Database database) {
+      super(database);
+    }
+
+    @Override
+    public void loadBean(EntityBeanIntercept ebi) {
+      database.getPluginApi().loadBeanRef(ebi);
+    }
+  }
+}

src/main/java/io/ebean/plugin/SpiServer.java

@@ -1,6 +1,8 @@
 package io.ebean.plugin;
 
 import io.ebean.EbeanServer;
+import io.ebean.bean.BeanLoader;
+import io.ebean.bean.EntityBeanIntercept;
 import io.ebean.config.ServerConfig;
 import io.ebean.config.dbplatform.DatabasePlatform;
 
@@ -10,7 +12,7 @@ import java.util.List;
 /**
  * Extensions to Database API made available to plugins.
  */
-public interface SpiServer extends EbeanServer {
+public interface SpiServer extends EbeanServer, BeanLoader {
 
   /**
    * Return the serverConfig.
@@ -52,4 +54,19 @@ public interface SpiServer extends EbeanServer {
    */
   DataSource getReadOnlyDataSource();
 
+  /**
+   * Invoke lazy loading on this single bean (reference bean).
+   */
+  void loadBeanRef(EntityBeanIntercept ebi);
+
+  /**
+   * Invoke lazy loading on this single bean (L2 cache bean).
+   */
+  void loadBeanL2(EntityBeanIntercept ebi);
+
+  /**
+   * Invoke lazy loading on this single bean when no BeanLoader is set.
+   * Typically due to serialisation or multiple stateless updates.
+   */
+  void loadBean(EntityBeanIntercept ebi);
 }

src/main/java/io/ebeaninternal/api/ExtraMetrics.java

@@ -2,6 +2,7 @@ package io.ebeaninternal.api;
 
 import io.ebean.meta.MetricType;
 import io.ebean.meta.MetricVisitor;
+import io.ebean.metric.CountMetric;
 import io.ebean.metric.MetricFactory;
 import io.ebean.metric.TimedMetric;
 
@@ -12,6 +13,9 @@ public class ExtraMetrics {
 
   private final TimedMetric bindCapture;
   private final TimedMetric planCollect;
+  private final CountMetric loadOneL2;
+  private final CountMetric loadOneRef;
+  private final CountMetric loadOneNoLoader;
 
   /**
    * Create the extra metrics.
@@ -20,6 +24,9 @@ public class ExtraMetrics {
     final MetricFactory factory = MetricFactory.get();
     this.bindCapture = factory.createTimedMetric(MetricType.ORM, "ebean.queryplan.bindcapture");
     this.planCollect = factory.createTimedMetric(MetricType.ORM, "ebean.queryplan.collect");
+    this.loadOneL2 = factory.createCountMetric(MetricType.ORM, "loadone.l2");
+    this.loadOneRef = factory.createCountMetric(MetricType.ORM, "loadone.ref");
+    this.loadOneNoLoader = factory.createCountMetric(MetricType.ORM, "loadone.noloader");
   }
 
   /**
@@ -36,11 +43,38 @@ public class ExtraMetrics {
     return planCollect;
   }
 
+  /**
+   * Increment counter for lazy loading one bean from L2 cache.
+   * All good when lazy loading also hits L2 cache.
+   */
+  public void incrementLoadOneL2() {
+    loadOneL2.increment();
+  }
+
+  /**
+   * Increment counter for lazy loading on reference bean.
+   * We ought to be able to avoid this by changing to a tuned query.
+   */
+  public void incrementLoadOneRef() {
+    loadOneRef.increment();
+  }
+
+  /**
+   * Increment counter for lazy loading one bean due to no loader.
+   * Likely due to multiple stateless updates or serialisation.
+   */
+  public void incrementLoadOneNoLoader() {
+    loadOneNoLoader.increment();
+  }
+
   /**
    * Collect the metrics.
    */
   public void visitMetrics(MetricVisitor visitor) {
     bindCapture.visit(visitor);
     planCollect.visit(visitor);
+    loadOneL2.visit(visitor);
+    loadOneRef.visit(visitor);
+    loadOneNoLoader.visit(visitor);
   }
 }

src/main/java/io/ebeaninternal/api/SpiEbeanServer.java

@@ -31,7 +31,7 @@ import java.util.function.Predicate;
 /**
  * Service Provider extension to EbeanServer.
  */
-public interface SpiEbeanServer extends ExtendedServer, EbeanServer, BeanLoader, BeanCollectionLoader {
+public interface SpiEbeanServer extends ExtendedServer, EbeanServer, BeanCollectionLoader {
 
   /**
    * Return the log manager.

src/main/java/io/ebeaninternal/server/core/DefaultServer.java

@@ -580,6 +580,19 @@ public final class DefaultServer implements SpiServer, SpiEbeanServer {
   @Override
   public void loadBean(EntityBeanIntercept ebi) {
     beanLoader.loadBean(ebi);
+    extraMetrics.incrementLoadOneNoLoader();
+  }
+
+  @Override
+  public void loadBeanRef(EntityBeanIntercept ebi) {
+    beanLoader.loadBean(ebi);
+    extraMetrics.incrementLoadOneRef();
+  }
+
+  @Override
+  public void loadBeanL2(EntityBeanIntercept ebi) {
+    beanLoader.loadBean(ebi);
+    extraMetrics.incrementLoadOneL2();
   }
 
   @Override

src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java

@@ -10,6 +10,7 @@ import io.ebean.bean.BeanCollection;
 import io.ebean.bean.EntityBean;
 import io.ebean.bean.EntityBeanIntercept;
 import io.ebean.bean.PersistenceContext;
+import io.ebean.bean.SingleBeanLoader;
 import io.ebean.cache.QueryCacheEntry;
 import io.ebean.config.EncryptKey;
 import io.ebean.config.ServerConfig;
@@ -2040,7 +2041,7 @@ public class BeanDescriptor<T> implements BeanType<T>, STreeType {
       if (disableLazyLoad) {
         ebi.setDisableLazyLoad(true);
       } else {
-        ebi.setBeanLoader(ebeanServer);
+        ebi.setBeanLoader(refBeanLoader());
       }
       ebi.setReference(idPropertyIndex);
       if (Boolean.TRUE == readOnly) {
@@ -2058,6 +2059,14 @@ public class BeanDescriptor<T> implements BeanType<T>, STreeType {
     }
   }
 
+  SingleBeanLoader refBeanLoader() {
+    return new SingleBeanLoader.Ref(ebeanServer);
+  }
+
+  SingleBeanLoader l2BeanLoader() {
+    return new SingleBeanLoader.L2(ebeanServer);
+  }
+
   /**
    * Create a non read only reference bean without checking cacheSharableBeans.
    */
@@ -2072,7 +2081,7 @@ public class BeanDescriptor<T> implements BeanType<T>, STreeType {
       EntityBean eb = createEntityBean();
       id = convertSetId(id, eb);
       EntityBeanIntercept ebi = eb._ebean_getIntercept();
-      ebi.setBeanLoader(ebeanServer);
+      ebi.setBeanLoader(refBeanLoader());
       ebi.setReference(idPropertyIndex);
       if (pc != null) {
         contextPut(pc, id, eb);

src/main/java/io/ebeaninternal/server/deploy/BeanDescriptorCacheHelp.java

@@ -690,7 +690,7 @@ final class BeanDescriptorCacheHelp<T> {
 
       EntityBeanIntercept ebi = bean._ebean_getIntercept();
       // Not using loadContext here so no batch lazy loading for these beans
-      ebi.setBeanLoader(desc.getEbeanServer());
+      ebi.setBeanLoader(desc.l2BeanLoader());
       if (Boolean.TRUE.equals(readOnly)) {
         ebi.setReadOnly(true);
       }

src/test/java/io/ebeaninternal/api/TDSpiEbeanServer.java

@@ -323,10 +323,6 @@ public class TDSpiEbeanServer implements SpiEbeanServer {
   public void loadMany(BeanCollection<?> collection, boolean onlyIds) {
   }
 
-  @Override
-  public void loadBean(EntityBeanIntercept ebi) {
-  }
-
   @Override
   public void shutdown(boolean shutdownDataSource, boolean deregisterDriver) {
   }

src/test/java/io/ebeaninternal/server/deploy/TestReferenceWithConstructorProperties.java

@@ -3,13 +3,17 @@ package io.ebeaninternal.server.deploy;
 import io.ebean.BaseTestCase;
 import io.ebean.BeanState;
 import io.ebean.Ebean;
+import org.junit.Test;
 import org.tests.model.basic.Order;
 import org.tests.model.basic.ResetBasicData;
-import org.junit.Assert;
-import org.junit.Test;
 
 import java.util.Set;
 
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertTrue;
+
 public class TestReferenceWithConstructorProperties extends BaseTestCase {
 
   /**
@@ -24,14 +28,14 @@ public class TestReferenceWithConstructorProperties extends BaseTestCase {
     BeanState beanState = Ebean.getBeanState(order);
     Set<String> loadedProps = beanState.getLoadedProps();
 
-    Assert.assertEquals(1, loadedProps.size());
-    Assert.assertTrue(beanState.isReference());
+    assertEquals(1, loadedProps.size());
+    assertTrue(beanState.isReference());
 
     // read the status invokes lazy loading
     order.getStatus();
 
-    Assert.assertFalse(beanState.isReference());
-
+    assertFalse(beanState.isReference());
+    assertNotNull(order.getCustomer());
   }
 
 }

src/test/java/org/tests/cache/TestCacheCollectionIds.java

@@ -9,16 +9,24 @@ import io.ebean.Update;
 import io.ebean.cache.ServerCache;
 import io.ebean.cache.ServerCacheManager;
 import io.ebeaninternal.server.cache.CachedManyIds;
-import org.junit.Assert;
 import org.junit.Ignore;
 import org.junit.Test;
-import org.tests.model.basic.*;
+import org.tests.model.basic.Contact;
+import org.tests.model.basic.Country;
+import org.tests.model.basic.Customer;
+import org.tests.model.basic.OBeanChild;
+import org.tests.model.basic.OCachedBean;
+import org.tests.model.basic.OCachedBeanChild;
+import org.tests.model.basic.Order;
+import org.tests.model.basic.OrderDetail;
+import org.tests.model.basic.ResetBasicData;
 
 import java.util.ArrayList;
 import java.util.List;
 
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
 public class TestCacheCollectionIds extends BaseTestCase {
@@ -88,7 +96,7 @@ public class TestCacheCollectionIds extends BaseTestCase {
     List<Contact> contacts2 = customer2.getContacts();
 
     for (Contact contact : contacts2) {
-      contact.getFirstName();
+      assertNotNull(contact.getFirstName());
       contact.getEmail();
     }
     return contacts2.size();
@@ -297,7 +305,7 @@ public class TestCacheCollectionIds extends BaseTestCase {
     // assert that the cache contains the expected entry
     assertEquals("countries cache now loaded with 1 entry", 1, cachedBeanCountriesCache.size());
     CachedManyIds dummyEntry = (CachedManyIds) cachedBeanCountriesCache.get(dummyLoad.getId());
-    Assert.assertNotNull(dummyEntry);
+    assertNotNull(dummyEntry);
     assertEquals("2 ids in the entry", 2, dummyEntry.getIdList().size());
     assertTrue(dummyEntry.getIdList().contains("NZ"));
     assertTrue(dummyEntry.getIdList().contains("AU"));

src/test/java/org/tests/cache/TestL2CacheWithSharedBean.java

@@ -12,6 +12,8 @@ import org.tests.model.basic.FeatureDescription;
 import org.junit.Assert;
 import org.junit.Test;
 
+import static org.junit.Assert.assertEquals;
+
 public class TestL2CacheWithSharedBean extends BaseTestCase {
 
   private TunedQueryInfo createTunedQueryInfo(OrmQueryDetail tunedDetail) {
@@ -25,7 +27,7 @@ public class TestL2CacheWithSharedBean extends BaseTestCase {
 
     FeatureDescription f1 = new FeatureDescription();
     f1.setName("one");
-    f1.setDescription(null);
+    f1.setDescription("helloOne");
 
     Ebean.save(f1);
 
@@ -44,19 +46,20 @@ public class TestL2CacheWithSharedBean extends BaseTestCase {
 
     FeatureDescription fd2 = query.findOne(); // LOAD cache
 
-    fd2.getDescription(); // invoke lazy load (this fails)
+    String description0 = fd2.getDescription(); // invoke lazy load
+    assertEquals("helloOne", description0);
 
     // load the cache
     FeatureDescription fetchOne = Ebean.find(FeatureDescription.class, f1.getId());
     Assert.assertNotNull(fetchOne);
-    Assert.assertEquals(1, beanCache.getStatistics(false).getSize());
+    assertEquals(1, beanCache.getStatistics(false).getSize());
 
     FeatureDescription fetchTwo = Ebean.find(FeatureDescription.class, f1.getId());
     FeatureDescription fetchThree = Ebean.find(FeatureDescription.class, f1.getId());
     Assert.assertSame(fetchTwo, fetchThree);
 
-    String description = fetchThree.getDescription();
-    Assert.assertNull(description);
+    String description1 = fetchThree.getDescription();
+    assertEquals("helloOne", description1);
   }
 
 }