# HG changeset patch # User Doug Simon # Date 1412378759 -7200 # Node ID 1778c3208bc56a51865c53252a29d327bb84eff5 # Parent 8043c997764d71967b72735ff66ca02389adc261 reduce or eliminate redundant writes during Node cloning diff -r 8043c997764d -r 1778c3208bc5 graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Edges.java --- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Edges.java Fri Oct 03 23:44:49 2014 +0200 +++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Edges.java Sat Oct 04 01:25:59 2014 +0200 @@ -160,6 +160,7 @@ * @param toNode the node to which the edges should be copied. */ public void copy(Node fromNode, Node toNode) { + assert fromNode != toNode; assert fromNode.getNodeClass().getClazz() == toNode.getNodeClass().getClazz(); int index = 0; while (index < getDirectCount()) { @@ -168,12 +169,12 @@ } while (index < getCount()) { NodeList list = getNodeList(toNode, index); - if (list == null) { - NodeList fromList = getNodeList(fromNode, index); + NodeList fromList = getNodeList(fromNode, index); + if (list == null || list == fromList) { list = type == Edges.Type.Inputs ? new NodeInputList<>(toNode, fromList) : new NodeSuccessorList<>(toNode, fromList); initializeList(toNode, index, list); } else { - list.copy(getNodeList(fromNode, index)); + list.copy(fromList); } index++; } diff -r 8043c997764d -r 1778c3208bc5 graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Graph.java --- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Graph.java Fri Oct 03 23:44:49 2014 +0200 +++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Graph.java Sat Oct 04 01:25:59 2014 +0200 @@ -29,6 +29,7 @@ import com.oracle.graal.compiler.common.*; import com.oracle.graal.debug.*; +import com.oracle.graal.debug.internal.*; import com.oracle.graal.graph.Node.ValueNumberable; import com.oracle.graal.graph.iterators.*; import com.oracle.graal.options.*; @@ -845,9 +846,13 @@ } + private static final DebugTimer DuplicateGraph = Debug.timer("DuplicateGraph"); + @SuppressWarnings("all") public Map addDuplicates(Iterable newNodes, final Graph oldGraph, int estimatedNodeCount, DuplicationReplacement replacements) { - return NodeClass.addGraphDuplicate(this, oldGraph, estimatedNodeCount, newNodes, replacements); + try (TimerCloseable s = DuplicateGraph.start()) { + return NodeClass.addGraphDuplicate(this, oldGraph, estimatedNodeCount, newNodes, replacements); + } } public boolean isFrozen() { diff -r 8043c997764d -r 1778c3208bc5 graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java --- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java Fri Oct 03 23:44:49 2014 +0200 +++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java Sat Oct 04 01:25:59 2014 +0200 @@ -28,6 +28,8 @@ import java.lang.annotation.*; import java.util.*; +import sun.misc.*; + import com.oracle.graal.compiler.common.*; import com.oracle.graal.graph.Graph.NodeEventListener; import com.oracle.graal.graph.iterators.*; @@ -58,7 +60,7 @@ public abstract class Node implements Cloneable, Formattable { public final static boolean USE_GENERATED_NODES = Boolean.parseBoolean(System.getProperty("graal.useGeneratedNodes", "true")); - public final static boolean USE_UNSAFE_CLONE = Boolean.parseBoolean(System.getProperty("graal.useUnsafeClone", "true")); + public final static boolean USE_UNSAFE_TO_CLONE = Boolean.parseBoolean(System.getProperty("graal.useUnsafeToClone", "true")); static final int DELETED_ID_START = -1000000000; static final int INITIAL_ID = -1; @@ -515,7 +517,7 @@ assert graph == null || !graph.isFrozen(); if (oldSuccessor != newSuccessor) { if (oldSuccessor != null) { - assert assertTrue(oldSuccessor.predecessor == this, "wrong predecessor in old successor (%s): %s", oldSuccessor, oldSuccessor.predecessor); + assert assertTrue(oldSuccessor.predecessor == this, "wrong predecessor in old successor (%s): %s, should be %s", oldSuccessor, oldSuccessor.predecessor, this); oldSuccessor.predecessor = null; } if (newSuccessor != null) { @@ -716,25 +718,13 @@ } public final Node copyWithInputs() { - return copyWithInputs(true); - } - - public final Node copyWithInputs(boolean addToGraph) { - Node newNode = clone(addToGraph ? graph : null); - NodeClass clazz = getNodeClass(); - clazz.getEdges(Inputs).copy(this, newNode); - if (addToGraph) { - for (Node input : inputs()) { - input.addUsage(newNode); - } + Node newNode = clone(graph, WithOnlyInputEdges); + for (Node input : inputs()) { + input.addUsage(newNode); } return newNode; } - public final Node clone(Graph into) { - return clone(into, true); - } - /** * Must be overridden by subclasses that implement {@link Simplifiable}. The implementation in * {@link Node} exists to obviate the need to cast a node before invoking @@ -746,9 +736,43 @@ throw new UnsupportedOperationException(); } - final Node clone(Graph into, boolean clearInputsAndSuccessors) { + /** + * @param newNode the result of cloning this node or {@link Unsafe#allocateInstance(Class) raw + * allocating} a copy of this node + * @param type the type of edges to process + * @param edgesToCopy if {@code type} is in this set, the edges are copied otherwise they are + * cleared + */ + private void copyOrClearEdgesForClone(NodeClass nodeClass, Node newNode, Edges.Type type, EnumSet edgesToCopy) { + if (edgesToCopy.contains(type)) { + nodeClass.getEdges(type).copy(this, newNode); + } else { + if (USE_UNSAFE_TO_CLONE) { + // The direct edges are already null + nodeClass.getEdges(type).initializeLists(newNode, this); + } else { + nodeClass.getEdges(type).clear(newNode); + } + } + } + + public static final EnumSet WithNoEdges = EnumSet.noneOf(Edges.Type.class); + public static final EnumSet WithAllEdges = EnumSet.allOf(Edges.Type.class); + public static final EnumSet WithOnlyInputEdges = EnumSet.of(Inputs); + public static final EnumSet WithOnlySucessorEdges = EnumSet.of(Successors); + + /** + * Makes a copy of this node in(to) a given graph. + * + * @param into the graph in which the copy will be registered (which may be this node's graph) + * @param edgesToCopy specifies the edges to be copied. The edges not specified in this set are + * initialized to their default value (i.e., {@code null} for a direct edge, an empty + * list for an edge list) + * @return the copy of this node + */ + final Node clone(Graph into, EnumSet edgesToCopy) { NodeClass nodeClass = getNodeClass(); - if (into != null && nodeClass.valueNumberable() && nodeClass.isLeafNode()) { + if (nodeClass.valueNumberable() && nodeClass.isLeafNode()) { Node otherNode = into.findNodeInCache(this); if (otherNode != null) { return otherNode; @@ -757,38 +781,29 @@ Node newNode = null; try { - if (USE_UNSAFE_CLONE) { + if (USE_UNSAFE_TO_CLONE) { newNode = (Node) UnsafeAccess.unsafe.allocateInstance(getClass()); nodeClass.getData().copy(this, newNode); - if (clearInputsAndSuccessors) { - nodeClass.getEdges(Inputs).initializeLists(newNode, this); - nodeClass.getEdges(Successors).initializeLists(newNode, this); - } else { - nodeClass.getEdges(Inputs).copy(this, newNode); - nodeClass.getEdges(Successors).copy(this, newNode); - } + copyOrClearEdgesForClone(nodeClass, newNode, Inputs, edgesToCopy); + copyOrClearEdgesForClone(nodeClass, newNode, Successors, edgesToCopy); } else { newNode = (Node) this.clone(); - if (clearInputsAndSuccessors) { - nodeClass.getEdges(Inputs).clear(newNode); - nodeClass.getEdges(Successors).clear(newNode); - } newNode.typeCacheNext = null; newNode.usage0 = null; newNode.usage1 = null; newNode.predecessor = null; + copyOrClearEdgesForClone(nodeClass, newNode, Inputs, edgesToCopy); + copyOrClearEdgesForClone(nodeClass, newNode, Successors, edgesToCopy); } } catch (Exception e) { throw new GraalGraphInternalError(e).addContext(this); } newNode.graph = into; newNode.id = INITIAL_ID; - if (into != null) { - into.register(newNode); - } + into.register(newNode); newNode.extraUsages = NO_NODES; - if (into != null && nodeClass.valueNumberable() && nodeClass.isLeafNode()) { + if (nodeClass.valueNumberable() && nodeClass.isLeafNode()) { into.putNodeIntoCache(newNode); } newNode.afterClone(this); diff -r 8043c997764d -r 1778c3208bc5 graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java --- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java Fri Oct 03 23:44:49 2014 +0200 +++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/NodeClass.java Sat Oct 04 01:25:59 2014 +0200 @@ -260,7 +260,7 @@ * * 
      *     if (node.getNodeClass().is(BeginNode.class)) { ... }
-     *
+     * 
      *     // Due to generated Node classes, the test below
      *     // is *not* the same as the test above:
      *     if (node.getClass() == BeginNode.class) { ... }
@@ -786,7 +786,7 @@
                     assert replacement != null;
                     newNodes.put(node, replacement);
                 } else {
-                    Node newNode = node.clone(graph, false);
+                    Node newNode = node.clone(graph, WithAllEdges);
                     assert newNode.inputs().isEmpty() || newNode.usages().isEmpty();
                     assert newNode.getClass() == node.getClass();
                     newNodes.put(node, newNode);