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 2017/01/20 02:17:34 UTC
calcite git commit: [CALCITE-1594]
ConventionTraitDef.getConversionData() is not thread-safe
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 <jh...@apache.org>
Authored: Thu Jan 19 13:40:49 2017 -0800
Committer: Julian Hyde <jh...@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 ----------------------------------------------------------