calcite-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jh...@apache.org
Subject calcite git commit: [CALCITE-1594] ConventionTraitDef.getConversionData() is not thread-safe
Date Fri, 20 Jan 2017 02:17:34 GMT
Repository: calcite
Updated Branches:
  refs/heads/master e5c9f2ed3 -> 83e517d51


[CALCITE-1594] ConventionTraitDef.getConversionData() is not thread-safe

Symptom is a hang in the test suite, with many threads in
ConventionTraitDef.getConversionData() and waiting to lock a WeakHashMap.
The solution is to replace the WeakHashMap with a LoadingCache, which is
thread-safe.


Project: http://git-wip-us.apache.org/repos/asf/calcite/repo
Commit: http://git-wip-us.apache.org/repos/asf/calcite/commit/83e517d5
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/83e517d5
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/83e517d5

Branch: refs/heads/master
Commit: 83e517d51f2601a4795ec386a2e17f9a6e3c7303
Parents: e5c9f2e
Author: Julian Hyde <jhyde@apache.org>
Authored: Thu Jan 19 13:40:49 2017 -0800
Committer: Julian Hyde <jhyde@apache.org>
Committed: Thu Jan 19 15:51:04 2017 -0800

----------------------------------------------------------------------
 .../apache/calcite/plan/ConventionTraitDef.java | 30 +++++++++++---------
 1 file changed, 16 insertions(+), 14 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/83e517d5/core/src/main/java/org/apache/calcite/plan/ConventionTraitDef.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/plan/ConventionTraitDef.java b/core/src/main/java/org/apache/calcite/plan/ConventionTraitDef.java
index 0ecca03..eddbb89 100644
--- a/core/src/main/java/org/apache/calcite/plan/ConventionTraitDef.java
+++ b/core/src/main/java/org/apache/calcite/plan/ConventionTraitDef.java
@@ -26,18 +26,22 @@ import org.apache.calcite.util.graph.DefaultEdge;
 import org.apache.calcite.util.graph.DirectedGraph;
 import org.apache.calcite.util.graph.Graphs;
 
+import com.google.common.cache.CacheBuilder;
+import com.google.common.cache.CacheLoader;
+import com.google.common.cache.LoadingCache;
 import com.google.common.collect.HashMultimap;
 import com.google.common.collect.Multimap;
 
 import java.util.List;
-import java.util.WeakHashMap;
+import javax.annotation.Nonnull;
 
 /**
  * Definition of the the convention trait.
  * A new set of conversion information is created for
  * each planner that registers at least one {@link ConverterRule} instance.
  *
- * <p>Conversion data is held in a {@link WeakHashMap} so that the JVM's garbage
+ * <p>Conversion data is held in a {@link LoadingCache}
+ * with weak keys so that the JVM's garbage
  * collector may reclaim the conversion data after the planner itself has been
  * garbage collected. The conversion information consists of a graph of
  * conversions (from one calling convention to another) and a map of graph arcs
@@ -52,12 +56,16 @@ public class ConventionTraitDef extends RelTraitDef<Convention>
{
   //~ Instance fields --------------------------------------------------------
 
   /**
-   * Weak-key map of RelOptPlanner to ConversionData. The idea is that when
-   * the planner goes away, so does the map entry.
+   * Weak-key cache of RelOptPlanner to ConversionData. The idea is that when
+   * the planner goes away, so does the cache entry.
    */
-  private final WeakHashMap<RelOptPlanner, ConversionData>
-  plannerConversionMap =
-      new WeakHashMap<RelOptPlanner, ConversionData>();
+  private final LoadingCache<RelOptPlanner, ConversionData> conversionCache =
+      CacheBuilder.newBuilder().weakKeys().build(
+          new CacheLoader<RelOptPlanner, ConversionData>() {
+            public ConversionData load(@Nonnull RelOptPlanner key) {
+              return new ConversionData();
+            }
+          });
 
   //~ Constructors -----------------------------------------------------------
 
@@ -200,13 +208,7 @@ public class ConventionTraitDef extends RelTraitDef<Convention>
{
   }
 
   private ConversionData getConversionData(RelOptPlanner planner) {
-    ConversionData conversionData = plannerConversionMap.get(planner);
-    if (conversionData == null) {
-      // Create new, empty ConversionData
-      conversionData = new ConversionData();
-      plannerConversionMap.put(planner, conversionData);
-    }
-    return conversionData;
+    return conversionCache.getUnchecked(planner);
   }
 
   //~ Inner Classes ----------------------------------------------------------


Mime
View raw message