You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by vl...@apache.org on 2016/01/13 09:36:27 UTC

calcite git commit: Fix CPU spin in ReflectiveRelMetadataProvider.apply -> HashMap.get [Forced Update!]

Repository: calcite
Updated Branches:
  refs/heads/master 9a17c178a -> fe91d4676 (forced update)


Fix CPU spin in ReflectiveRelMetadataProvider.apply -> HashMap.get

The map in question is shared between multiple threads and updated at the same time.
Thus the map should be a ConcurrentMap.

Problem was reported here: https://mail-archives.apache.org/mod_mbox/calcite-dev/201601.mbox/%3C56952E8F.7090705%40gmail.com%3E


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

Branch: refs/heads/master
Commit: fe91d46761f460927b21ef334fee5e686b54e397
Parents: 5323d8d
Author: Vladimir Sitnikov <si...@gmail.com>
Authored: Wed Jan 13 11:36:19 2016 +0300
Committer: Vladimir Sitnikov <si...@gmail.com>
Committed: Wed Jan 13 11:36:19 2016 +0300

----------------------------------------------------------------------
 .../rel/metadata/ReflectiveRelMetadataProvider.java    | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/calcite/blob/fe91d467/core/src/main/java/org/apache/calcite/rel/metadata/ReflectiveRelMetadataProvider.java
----------------------------------------------------------------------
diff --git a/core/src/main/java/org/apache/calcite/rel/metadata/ReflectiveRelMetadataProvider.java b/core/src/main/java/org/apache/calcite/rel/metadata/ReflectiveRelMetadataProvider.java
index beaedfe..8cdb55a 100644
--- a/core/src/main/java/org/apache/calcite/rel/metadata/ReflectiveRelMetadataProvider.java
+++ b/core/src/main/java/org/apache/calcite/rel/metadata/ReflectiveRelMetadataProvider.java
@@ -39,10 +39,11 @@ import java.lang.reflect.Proxy;
 import java.lang.reflect.UndeclaredThrowableException;
 import java.util.ArrayList;
 import java.util.Arrays;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentMap;
 
 /**
  * Implementation of the {@link RelMetadataProvider} interface that dispatches
@@ -60,7 +61,7 @@ public class ReflectiveRelMetadataProvider
     implements RelMetadataProvider, ReflectiveVisitor {
 
   //~ Instance fields --------------------------------------------------------
-  private final Map<Class<RelNode>, UnboundMetadata> map;
+  private final ConcurrentMap<Class<RelNode>, UnboundMetadata> map;
   private final Class<? extends Metadata> metadataClass0;
 
   //~ Constructors -----------------------------------------------------------
@@ -72,7 +73,7 @@ public class ReflectiveRelMetadataProvider
    * @param metadataClass0 Metadata class
    */
   protected ReflectiveRelMetadataProvider(
-      Map<Class<RelNode>, UnboundMetadata> map,
+      ConcurrentMap<Class<RelNode>, UnboundMetadata> map,
       Class<? extends Metadata> metadataClass0) {
     assert !map.isEmpty() : "are your methods named wrong?";
     this.map = map;
@@ -135,7 +136,11 @@ public class ReflectiveRelMetadataProvider
       }
     }
 
-    final Map<Class<RelNode>, UnboundMetadata> methodsMap = new HashMap<>();
+    // This needs to be a councurrent map since RelMetadataProvider are cached in static
+    // fields, thus the map is subject to concurrent modifications later.
+    // See map.put in org.apache.calcite.rel.metadata.ReflectiveRelMetadataProvider.apply(
+    // java.lang.Class<? extends org.apache.calcite.rel.RelNode>)
+    final ConcurrentMap<Class<RelNode>, UnboundMetadata> methodsMap = new ConcurrentHashMap<>();
     for (Class<RelNode> key : classes) {
       ImmutableNullableList.Builder<Method> builder =
           ImmutableNullableList.builder();