changeset 11431:ca53d08b8ef9

removed Node.nodeClass field (GRAAL-359)
author Doug Simon <doug.simon@oracle.com>
date Mon, 26 Aug 2013 21:38:10 +0200
parents a7dd2d728500
children 565724c714a7
files graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/VMToCompilerImpl.java graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedObjectType.java
diffstat 4 files changed, 134 insertions(+), 23 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java	Mon Aug 26 20:16:43 2013 +0200
+++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java	Mon Aug 26 21:38:10 2013 +0200
@@ -121,12 +121,10 @@
     private NodeUsagesList usages;
     private Node predecessor;
     private int modCount;
-    private final NodeClass nodeClass;
 
     public Node() {
         this.graph = null;
         this.id = INITIAL_ID;
-        nodeClass = NodeClass.get(getClass());
     }
 
     protected int id() {
@@ -241,7 +239,7 @@
     }
 
     public final NodeClass getNodeClass() {
-        return nodeClass;
+        return NodeClass.get(getClass());
     }
 
     private boolean checkReplaceWith(Node other) {
--- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java	Mon Aug 26 20:16:43 2013 +0200
+++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java	Mon Aug 26 21:38:10 2013 +0200
@@ -25,38 +25,104 @@
 import java.lang.reflect.*;
 import java.util.*;
 import java.util.Map.Entry;
+import java.util.concurrent.*;
 
 import com.oracle.graal.graph.Graph.DuplicationReplacement;
+import com.oracle.graal.graph.Node.Input;
+import com.oracle.graal.graph.Node.IterableNodeType;
+import com.oracle.graal.graph.Node.Successor;
 import com.oracle.graal.graph.Node.Verbosity;
 
-public class NodeClass extends FieldIntrospection {
+/**
+ * Lazily associated metadata for every {@link Node} type. The metadata includes:
+ * <ul>
+ * <li>The offsets of fields annotated with {@link Input} and {@link Successor} as well as methods
+ * for iterating over such fields.</li>
+ * <li>The identifier for an {@link IterableNodeType} class.</li>
+ * </ul>
+ */
+public final class NodeClass extends FieldIntrospection {
 
-    public static final NodeClass get(Class<?> c) {
-        NodeClass clazz = (NodeClass) allClasses.get(c);
-        if (clazz != null) {
-            return clazz;
+    /**
+     * Maps {@link Class} values (for {@link Node} types) to {@link NodeClass} values.
+     * 
+     * Only a single Registry instance can be created. If a runtime creates a specialized registry,
+     * it must do so before the class initializer of {@link NodeClass} is executed.
+     */
+    public static class Registry {
+
+        private static Registry instance;
+
+        /**
+         * Gets the singleton {@link Registry} instance, creating it first if necessary.
+         */
+        static synchronized Registry instance() {
+            if (instance == null) {
+                return new Registry();
+            }
+            return instance;
+        }
+
+        protected Registry() {
+            assert instance == null : "exactly one registry can be created";
+            instance = this;
         }
 
-        /*
-         * Using putIfAbsent doesn't work here, because the creation of NodeClass needs to be
-         * serialized. (the NodeClass constructor looks at allClasses, and it also uses the static
-         * field nextIterableId)
-         * 
-         * The fact that ConcurrentHashMap.put and .get are used should make the double-checked
-         * locking idiom work, since it internally uses volatile.
+        /**
+         * @return the {@link NodeClass} value for {@code key} or {@code null} if no such mapping
+         *         exists
          */
+        protected NodeClass get(Class<? extends Node> key) {
+            return (NodeClass) allClasses.get(key);
+        }
 
-        synchronized (allClasses) {
-            clazz = (NodeClass) allClasses.get(c);
-            if (clazz == null) {
-                clazz = new NodeClass(c);
-                NodeClass oldClass = (NodeClass) allClasses.putIfAbsent(c, clazz);
-                assert oldClass == null;
+        /**
+         * Same as {@link #get(Class)} except that a {@link NodeClass} is created if no such mapping
+         * exists. The creation of a {@link NodeClass} must be serialized as
+         * {@link NodeClass#NodeClass(Class)} accesses both {@link FieldIntrospection#allClasses}
+         * and {@link NodeClass#nextIterableId}.
+         * <p>
+         * The fact that {@link ConcurrentHashMap#put} {@link ConcurrentHashMap#get} are used should
+         * make the double-checked locking idiom work in the way {@link NodeClass#get(Class)} uses
+         * this method and {@link #get(Class)}.
+         */
+        final synchronized NodeClass make(Class<? extends Node> key) {
+            NodeClass value = (NodeClass) allClasses.get(key);
+            if (value == null) {
+                value = new NodeClass(key);
+                Object old = allClasses.putIfAbsent(key, value);
+                assert old == null;
+                registered(key, value);
             }
-            return clazz;
+            return value;
+        }
+
+        /**
+         * Hook for a subclass to be notified of a new mapping added to the registry.
+         * 
+         * @param key
+         * @param value
+         */
+        protected void registered(Class<? extends Node> key, NodeClass value) {
+
         }
     }
 
+    private static final Registry registry = Registry.instance();
+
+    /**
+     * Gets the {@link NodeClass} associated with a given {@link Class}.
+     */
+    public static NodeClass get(Class<?> c) {
+        @SuppressWarnings("unchecked")
+        Class<? extends Node> key = (Class<? extends Node>) c;
+        NodeClass value = registry.get(key);
+        if (value != null) {
+            return value;
+        }
+        return registry.make(key);
+    }
+
     static final int NOT_ITERABLE = -1;
 
     private static final Class<?> NODE_CLASS = Node.class;
@@ -77,7 +143,7 @@
     private final int iterableId;
     private int[] iterableIds;
 
-    public NodeClass(Class<?> clazz) {
+    private NodeClass(Class<?> clazz) {
         super(clazz);
         assert NODE_CLASS.isAssignableFrom(clazz);
 
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/VMToCompilerImpl.java	Mon Aug 26 20:16:43 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/bridge/VMToCompilerImpl.java	Mon Aug 26 21:38:10 2013 +0200
@@ -258,6 +258,33 @@
             DynamicCounterNode.enabled = true;
         }
         compilerStartTime = System.nanoTime();
+
+        FastNodeClassRegistry.initialize();
+    }
+
+    /**
+     * A fast-path for {@link NodeClass} retrieval using {@link HotSpotResolvedObjectType}.
+     */
+    static class FastNodeClassRegistry extends NodeClass.Registry {
+
+        @SuppressWarnings("unused")
+        static void initialize() {
+            new FastNodeClassRegistry();
+        }
+
+        private static HotSpotResolvedObjectType type(Class<? extends Node> key) {
+            return (HotSpotResolvedObjectType) HotSpotResolvedObjectType.fromClass(key);
+        }
+
+        @Override
+        public NodeClass get(Class<? extends Node> key) {
+            return type(key).getNodeClass();
+        }
+
+        @Override
+        protected void registered(Class<? extends Node> key, NodeClass value) {
+            type(key).setNodeClass(value);
+        }
     }
 
     private final class BenchmarkCountersOutputStream extends CallbackOutputStream {
--- a/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedObjectType.java	Mon Aug 26 20:16:43 2013 +0200
+++ b/graal/com.oracle.graal.hotspot/src/com/oracle/graal/hotspot/meta/HotSpotResolvedObjectType.java	Mon Aug 26 21:38:10 2013 +0200
@@ -33,6 +33,7 @@
 import java.util.*;
 
 import com.oracle.graal.api.meta.*;
+import com.oracle.graal.graph.*;
 import com.oracle.graal.hotspot.*;
 
 /**
@@ -65,6 +66,11 @@
     private final String simpleName;
 
     /**
+     * Used for implemented a lazy binding from a {@link Node} type to a {@link NodeClass} value.
+     */
+    private NodeClass nodeClass;
+
+    /**
      * The instance size (in bytes) for an instance type,
      * {@link HotSpotResolvedObjectType#INTERFACE_SPECIES_VALUE} denoting an interface type or
      * {@link HotSpotResolvedObjectType#ARRAY_SPECIES_VALUE} denoting an array type.
@@ -554,4 +560,18 @@
     public Constant newArray(int length) {
         return Constant.forObject(Array.newInstance(javaMirror, length));
     }
+
+    /**
+     * @return the {@link NodeClass} value (which may be {@code null}) associated with this type
+     */
+    public NodeClass getNodeClass() {
+        return nodeClass;
+    }
+
+    /**
+     * Sets the {@link NodeClass} value associated with this type.
+     */
+    public void setNodeClass(NodeClass nodeClass) {
+        this.nodeClass = nodeClass;
+    }
 }