Performance improvement in OrmQueryProperties for FetchGroup reuse (#3761)

ebean-bench/pom.xml

@@ -0,0 +1,92 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+  xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+  <modelVersion>4.0.0</modelVersion>
+  <parent>
+    <artifactId>ebean-parent</artifactId>
+    <groupId>io.ebean</groupId>
+    <version>16.5.0</version>
+  </parent>
+
+  <artifactId>ebean-bench</artifactId>
+  <packaging>jar</packaging>
+
+  <name>ebean-bench</name>
+  <description>JMH benchmarks for ebean internals</description>
+
+  <properties>
+    <maven.deploy.skip>true</maven.deploy.skip>
+    <jmh.version>1.37</jmh.version>
+  </properties>
+
+  <dependencies>
+    <dependency>
+      <groupId>io.ebean</groupId>
+      <artifactId>ebean-core</artifactId>
+      <version>${project.version}</version>
+    </dependency>
+    <dependency>
+      <groupId>org.openjdk.jmh</groupId>
+      <artifactId>jmh-core</artifactId>
+      <version>${jmh.version}</version>
+    </dependency>
+    <dependency>
+      <groupId>org.openjdk.jmh</groupId>
+      <artifactId>jmh-generator-annprocess</artifactId>
+      <version>${jmh.version}</version>
+      <scope>provided</scope>
+    </dependency>
+  </dependencies>
+
+  <build>
+    <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <configuration>
+          <annotationProcessorPaths>
+            <path>
+              <groupId>org.openjdk.jmh</groupId>
+              <artifactId>jmh-generator-annprocess</artifactId>
+              <version>${jmh.version}</version>
+            </path>
+          </annotationProcessorPaths>
+        </configuration>
+      </plugin>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-shade-plugin</artifactId>
+        <executions>
+          <execution>
+            <phase>package</phase>
+            <goals>
+              <goal>shade</goal>
+            </goals>
+            <configuration>
+              <finalName>benchmarks</finalName>
+              <createDependencyReducedPom>false</createDependencyReducedPom>
+              <transformers>
+                <transformer implementation="org.apache.maven.plugins.shade.resource.ManifestResourceTransformer">
+                  <mainClass>org.openjdk.jmh.Main</mainClass>
+                </transformer>
+                <transformer implementation="org.apache.maven.plugins.shade.resource.ServicesResourceTransformer"/>
+              </transformers>
+              <filters>
+                <filter>
+                  <artifact>*:*</artifact>
+                  <excludes>
+                    <exclude>META-INF/*.SF</exclude>
+                    <exclude>META-INF/*.DSA</exclude>
+                    <exclude>META-INF/*.RSA</exclude>
+                  </excludes>
+                </filter>
+              </filters>
+            </configuration>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
+  </build>
+
+</project>

ebean-bench/src/main/java/io/ebean/bench/OrmQueryDetailCopyBenchmark.java

@@ -0,0 +1,50 @@
+package io.ebean.bench;
+
+import io.ebeaninternal.server.querydefn.OrmQueryDetail;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Warmup;
+
+import java.util.concurrent.TimeUnit;
+
+@BenchmarkMode(Mode.Throughput)
+@OutputTimeUnit(TimeUnit.MILLISECONDS)
+@Warmup(iterations = 5, time = 1)
+@Measurement(iterations = 10, time = 1)
+@Fork(2)
+@State(Scope.Thread)
+public class OrmQueryDetailCopyBenchmark {
+
+  private OrmQueryDetail source;
+  private OrmQueryDetail existing;
+
+  @Setup
+  public void setup() {
+    source = new OrmQueryDetail();
+    source.select("id,name,status,version");
+    source.fetch("billingAddress", "line1,city,country", null);
+    source.fetch("shippingAddress", "line1,city,country", null);
+    source.fetch("contacts", "firstName,lastName,email", null);
+    source.fetch("contacts.phoneNumbers", "number,type", null);
+
+    existing = new OrmQueryDetail();
+    existing.fetch("contacts", "firstName,lastName", null);
+  }
+
+  @Benchmark
+  public OrmQueryDetail copyNull() {
+    return source.copy(null);
+  }
+
+  @Benchmark
+  public OrmQueryDetail copyExisting() {
+    return source.copy(existing);
+  }
+}

ebean-core/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryDetail.java

@@ -45,7 +45,7 @@ public final class OrmQueryDetail implements Serializable {
     for (Map.Entry<String, OrmQueryProperties> entry : fetchPaths.entrySet()) {
       copy.fetchPaths.put(entry.getKey(), entry.getValue().copy());
     }
-    if (existing != null) {
+    if (existing != null && !existing.fetchPaths.isEmpty()) {
       // transfer any existing filterMany expressions
       for (Map.Entry<String, OrmQueryProperties> entry : existing.fetchPaths.entrySet()) {
         var filterMany = entry.getValue().getFilterMany();

ebean-core/src/main/java/io/ebeaninternal/server/querydefn/OrmQueryProperties.java

@@ -15,6 +15,7 @@ import io.ebeaninternal.server.expression.FilterExpressionList;
 
 import java.io.Serializable;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
@@ -87,7 +88,7 @@ public final class OrmQueryProperties implements Serializable {
     this.parentPath = SplitName.parent(path);
     OrmQueryPropertiesParser.Response response = OrmQueryPropertiesParser.parse(rawProperties);
     this.allProperties = response.allProperties;
-    this.included = response.included;
+    this.included = immutableIncluded(response.included);
     if (fetchConfig != null) {
       this.fetchConfig = fetchConfig;
       this.cache = fetchConfig.isCache();
@@ -104,7 +105,7 @@ public final class OrmQueryProperties implements Serializable {
   OrmQueryProperties(String path, Set<String> included, FetchConfig fetchConfig) {
     this.path = path;
     this.parentPath = SplitName.parent(path);
-    this.included = included;
+    this.included = immutableIncluded(included);
     this.allProperties = false;
     this.fetchConfig = fetchConfig;
     this.cache = fetchConfig.isCache();
@@ -130,7 +131,14 @@ public final class OrmQueryProperties implements Serializable {
     this.cache = source.cache;
     this.filterMany = source.filterMany;
     this.markForQueryJoin = source.markForQueryJoin;
-    this.included = (source.included == null) ? null : new LinkedHashSet<>(source.included);
+    this.included = source.included;
+  }
+
+  private static Set<String> immutableIncluded(Set<String> included) {
+    if (included == null) {
+      return null;
+    }
+    return Collections.unmodifiableSet(new LinkedHashSet<>(included));
   }
 
   /**

ebean-core/src/test/java/io/ebeaninternal/server/querydefn/OrmQueryPropertiesTest.java

@@ -5,6 +5,7 @@ import org.junit.jupiter.api.Test;
 import java.util.LinkedHashSet;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertSame;
 
 public class OrmQueryPropertiesTest {
 
@@ -46,6 +47,26 @@ public class OrmQueryPropertiesTest {
     assertThat(p1.allProperties()).isFalse();
   }
 
+  @Test
+  public void construct_with_propertySet_expect_defensiveCopy() {
+
+    LinkedHashSet<String> set = new LinkedHashSet<>();
+    set.add("name");
+    OrmQueryProperties p1 = new OrmQueryProperties(null, set);
+    set.add("status");
+
+    assertThat(p1.getIncluded()).containsOnly("name");
+  }
+
+  @Test
+  public void copy_expect_reuseIncludedSet() {
+
+    OrmQueryProperties p1 = new OrmQueryProperties(null, "id,name");
+    OrmQueryProperties p2 = p1.copy();
+
+    assertSame(p1.getIncluded(), p2.getIncluded());
+  }
+
   @Test
   public void append_when_empty() {
 

pom.xml

@@ -101,6 +101,12 @@
   </modules>
 
   <profiles>
+    <profile>
+      <id>bench</id>
+      <modules>
+        <module>ebean-bench</module>
+      </modules>
+    </profile>
     <profile>
       <id>ea</id>
     </profile>
@@ -138,4 +144,3 @@
   </profiles>
 
 </project>
-