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 2016/01/15 00:05:29 UTC
[07/13] calcite git commit: [CALCITE-1053] CPU spin in
ReflectiveRelMetadataProvider.apply -> HashMap.get
[CALCITE-1053] 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/2712d7da
Tree: http://git-wip-us.apache.org/repos/asf/calcite/tree/2712d7da
Diff: http://git-wip-us.apache.org/repos/asf/calcite/diff/2712d7da
Branch: refs/heads/branch-1.6
Commit: 2712d7dafc1b724c33b0c29846666c1903d5afb2
Parents: 5323d8d
Author: Vladimir Sitnikov <si...@gmail.com>
Authored: Wed Jan 13 11:36:19 2016 +0300
Committer: Julian Hyde <jh...@apache.org>
Committed: Wed Jan 13 08:42:52 2016 -0800
----------------------------------------------------------------------
.../rel/metadata/ReflectiveRelMetadataProvider.java | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/calcite/blob/2712d7da/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();