You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2020/01/06 08:11:59 UTC

[calcite] 01/07: In DiffRepository, replace a synchronized Map with a Guava cache

This is an automated email from the ASF dual-hosted git repository.

jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git

commit 8658804e0a1b5c2fe9c78fa8e40655db06531ac8
Author: Julian Hyde <jh...@apache.org>
AuthorDate: Mon Dec 16 15:57:53 2019 -0800

    In DiffRepository, replace a synchronized Map with a Guava cache
---
 .../org/apache/calcite/test/DiffRepository.java    | 62 +++++++++++++++-------
 1 file changed, 44 insertions(+), 18 deletions(-)

diff --git a/core/src/test/java/org/apache/calcite/test/DiffRepository.java b/core/src/test/java/org/apache/calcite/test/DiffRepository.java
index fefa003..462c79f 100644
--- a/core/src/test/java/org/apache/calcite/test/DiffRepository.java
+++ b/core/src/test/java/org/apache/calcite/test/DiffRepository.java
@@ -22,6 +22,10 @@ import org.apache.calcite.util.Sources;
 import org.apache.calcite.util.Util;
 import org.apache.calcite.util.XmlOutput;
 
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
+
 import org.junit.jupiter.api.Assertions;
 import org.opentest4j.AssertionFailedError;
 import org.w3c.dom.CDATASection;
@@ -39,10 +43,9 @@ import java.io.IOException;
 import java.io.Writer;
 import java.net.URL;
 import java.util.ArrayList;
-import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
@@ -161,8 +164,8 @@ public class DiffRepository {
    * the same class to share the same diff-repository: if the repository gets
    * loaded once per test case, then only one diff is recorded.
    */
-  private static final Map<Class, DiffRepository> MAP_CLASS_TO_REPOSITORY =
-      new HashMap<>();
+  private static final LoadingCache<Key, DiffRepository> REPOSITORY_CACHE =
+      CacheBuilder.newBuilder().build(CacheLoader.from(Key::toRepo));
 
   //~ Instance fields --------------------------------------------------------
 
@@ -755,22 +758,11 @@ public class DiffRepository {
    * @param filter    Filters each string returned by the repository
    * @return The diff repository shared between test cases in this class.
    */
-  public static synchronized DiffRepository lookup(
-      Class clazz,
+  public static DiffRepository lookup(Class clazz,
       DiffRepository baseRepository,
       Filter filter) {
-    DiffRepository diffRepository = MAP_CLASS_TO_REPOSITORY.get(clazz);
-    if (diffRepository == null) {
-      final URL refFile = findFile(clazz, ".xml");
-      final String refFilePath = Sources.of(refFile).file().getAbsolutePath();
-      final File logFile =
-          new File(refFilePath.replace(".xml", "_actual.xml"));
-      assert !refFilePath.equals(logFile.getAbsolutePath());
-      diffRepository =
-          new DiffRepository(refFile, logFile, baseRepository, filter);
-      MAP_CLASS_TO_REPOSITORY.put(clazz, diffRepository);
-    }
-    return diffRepository;
+    final Key key = new Key(clazz, baseRepository, filter);
+    return REPOSITORY_CACHE.getUnchecked(key);
   }
 
   /**
@@ -794,4 +786,38 @@ public class DiffRepository {
         String text,
         String expanded);
   }
+
+  /** Cache key. */
+  private static class Key {
+    private final Class clazz;
+    private final DiffRepository baseRepository;
+    private final Filter filter;
+
+    Key(Class clazz, DiffRepository baseRepository, Filter filter) {
+      this.clazz = Objects.requireNonNull(clazz);
+      this.baseRepository = baseRepository;
+      this.filter = filter;
+    }
+
+    @Override public int hashCode() {
+      return Objects.hash(clazz, baseRepository, filter);
+    }
+
+    @Override public boolean equals(Object obj) {
+      return this == obj
+          || obj instanceof Key
+          && clazz.equals(((Key) obj).clazz)
+          && Objects.equals(baseRepository, ((Key) obj).baseRepository)
+          && Objects.equals(filter, ((Key) obj).filter);
+    }
+
+    DiffRepository toRepo() {
+      final URL refFile = findFile(clazz, ".xml");
+      final String refFilePath = Sources.of(refFile).file().getAbsolutePath();
+      final String logFilePath = refFilePath.replace(".xml", "_actual.xml");
+      final File logFile = new File(logFilePath);
+      assert !refFilePath.equals(logFile.getAbsolutePath());
+      return new DiffRepository(refFile, logFile, baseRepository, filter);
+    }
+  }
 }