changeset 16241:c6ebc1997a55

added listener for nodes being added to a graph; consolidated all node event listeners into new NodeEventListener interface and made registering such listeners work in a try-with-resources statement so that de-registration is automatic
author Doug Simon <doug.simon@oracle.com>
date Thu, 26 Jun 2014 13:42:29 +0200
parents ff06fc69249e
children e9998e2be7f5
files graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Graph.java graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/IncrementalCanonicalizerPhase.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/IterativeConditionalEliminationPhase.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/IterativeFlowSensitiveReductionPhase.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/util/HashSetNodeChangeListener.java graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/util/HashSetNodeEventListener.java graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/EffectsPhase.java
diffstat 9 files changed, 207 insertions(+), 173 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Graph.java	Thu Jun 26 10:50:28 2014 +0200
+++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Graph.java	Thu Jun 26 13:42:29 2014 +0200
@@ -74,8 +74,7 @@
      */
     int compressions;
 
-    NodeChangedListener inputChangedListener;
-    NodeChangedListener usagesDroppedToZeroListener;
+    NodeEventListener nodeEventListener;
     private final HashMap<CacheEntry, Node> cachedNodes = new HashMap<>();
 
     /*
@@ -302,59 +301,97 @@
         return node;
     }
 
-    public interface NodeChangedListener {
+    /**
+     * Client interested in one or more node related events.
+     */
+    public interface NodeEventListener {
+
+        /**
+         * Notifies this listener of a change in a node's inputs.
+         *
+         * @param node a node who has had one of its inputs changed
+         */
+        default void inputChanged(Node node) {
+        }
 
-        void nodeChanged(Node node);
+        /**
+         * Notifies this listener of a node becoming unused.
+         *
+         * @param node a node whose {@link Node#usages()} just became empty
+         */
+        default void usagesDroppedToZero(Node node) {
+        }
+
+        /**
+         * Notifies this listener of an added node.
+         *
+         * @param node a node that was just added to the graph
+         */
+        default void nodeAdded(Node node) {
+        }
     }
 
-    private static class ChainedNodeChangedListener implements NodeChangedListener {
+    /**
+     * Registers a given {@link NodeEventListener} with the enclosing graph until this object is
+     * {@linkplain #close() closed}.
+     */
+    public final class NodeEventScope implements AutoCloseable {
+        NodeEventScope(NodeEventListener listener) {
+            if (nodeEventListener == null) {
+                nodeEventListener = listener;
+            } else {
+                nodeEventListener = new ChainedNodeEventListener(listener, nodeEventListener);
+            }
+        }
 
-        NodeChangedListener head;
-        NodeChangedListener next;
+        public void close() {
+            assert nodeEventListener != null;
+            if (nodeEventListener instanceof ChainedNodeEventListener) {
+                nodeEventListener = ((ChainedNodeEventListener) nodeEventListener).next;
+            } else {
+                nodeEventListener = null;
+            }
+        }
+    }
 
-        ChainedNodeChangedListener(NodeChangedListener head, NodeChangedListener next) {
+    private static class ChainedNodeEventListener implements NodeEventListener {
+
+        NodeEventListener head;
+        NodeEventListener next;
+
+        ChainedNodeEventListener(NodeEventListener head, NodeEventListener next) {
             this.head = head;
             this.next = next;
         }
 
-        public void nodeChanged(Node node) {
-            head.nodeChanged(node);
-            next.nodeChanged(node);
+        public void nodeAdded(Node node) {
+            head.nodeAdded(node);
+            next.nodeAdded(node);
         }
-    }
 
-    public void trackInputChange(NodeChangedListener listener) {
-        if (inputChangedListener == null) {
-            inputChangedListener = listener;
-        } else {
-            inputChangedListener = new ChainedNodeChangedListener(listener, inputChangedListener);
+        public void inputChanged(Node node) {
+            head.inputChanged(node);
+            next.inputChanged(node);
+        }
+
+        public void usagesDroppedToZero(Node node) {
+            head.inputChanged(node);
+            next.inputChanged(node);
         }
     }
 
-    public void stopTrackingInputChange() {
-        assert inputChangedListener != null;
-        if (inputChangedListener instanceof ChainedNodeChangedListener) {
-            inputChangedListener = ((ChainedNodeChangedListener) inputChangedListener).next;
-        } else {
-            inputChangedListener = null;
-        }
-    }
-
-    public void trackUsagesDroppedZero(NodeChangedListener listener) {
-        if (usagesDroppedToZeroListener == null) {
-            usagesDroppedToZeroListener = listener;
-        } else {
-            usagesDroppedToZeroListener = new ChainedNodeChangedListener(listener, usagesDroppedToZeroListener);
-        }
-    }
-
-    public void stopTrackingUsagesDroppedZero() {
-        assert usagesDroppedToZeroListener != null;
-        if (usagesDroppedToZeroListener instanceof ChainedNodeChangedListener) {
-            usagesDroppedToZeroListener = ((ChainedNodeChangedListener) usagesDroppedToZeroListener).next;
-        } else {
-            usagesDroppedToZeroListener = null;
-        }
+    /**
+     * Registers a given {@link NodeEventListener} with this graph. This should be used in
+     * conjunction with try-with-resources statement as follows:
+     *
+     * <pre>
+     * try (NodeEventScope nes = graph.trackNodeEvents(listener)) {
+     *     // make changes to the graph
+     * }
+     * </pre>
+     */
+    public NodeEventScope trackNodeEvents(NodeEventListener listener) {
+        return new NodeEventScope(listener);
     }
 
     /**
@@ -811,6 +848,9 @@
         }
 
         node.id = id;
+        if (nodeEventListener != null) {
+            nodeEventListener.nodeAdded(node);
+        }
         logNodeAdded(node);
     }
 
--- a/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java	Thu Jun 26 10:50:28 2014 +0200
+++ b/graal/com.oracle.graal.graph/src/com/oracle/graal/graph/Node.java	Thu Jun 26 13:42:29 2014 +0200
@@ -27,7 +27,6 @@
 import java.lang.annotation.*;
 import java.util.*;
 
-import com.oracle.graal.graph.Graph.NodeChangedListener;
 import com.oracle.graal.graph.NodeClass.NodeClassIterator;
 import com.oracle.graal.graph.NodeClass.Position;
 import com.oracle.graal.graph.iterators.*;
@@ -559,7 +558,7 @@
                     assert assertTrue(result, "not found in usages, old input: %s", oldInput);
                 }
             }
-            maybeNotifyChanged(this);
+            maybeNotifyInputChanged(this);
             if (newInput != null) {
                 if (newInput.recordsUsages()) {
                     newInput.addUsage(this);
@@ -629,7 +628,7 @@
             boolean result = usage.getNodeClass().replaceFirstInput(usage, this, other);
             assert assertTrue(result, "not found in inputs, usage: %s", usage);
             if (other != null) {
-                maybeNotifyChanged(usage);
+                maybeNotifyInputChanged(usage);
                 if (other.recordsUsages()) {
                     other.addUsage(usage);
                 }
@@ -651,7 +650,7 @@
                 boolean result = usage.getNodeClass().replaceFirstInput(usage, this, other);
                 assert assertTrue(result, "not found in inputs, usage: %s", usage);
                 if (other != null) {
-                    maybeNotifyChanged(usage);
+                    maybeNotifyInputChanged(usage);
                     if (other.recordsUsages()) {
                         other.addUsage(usage);
                     }
@@ -685,22 +684,22 @@
         }
     }
 
-    private void maybeNotifyChanged(Node usage) {
+    private void maybeNotifyInputChanged(Node node) {
         if (graph != null) {
             assert !graph.isFrozen();
-            NodeChangedListener listener = graph.inputChangedListener;
+            NodeEventListener listener = graph.nodeEventListener;
             if (listener != null) {
-                listener.nodeChanged(usage);
+                listener.inputChanged(node);
             }
         }
     }
 
-    private void maybeNotifyZeroInputs(Node oldInput) {
+    private void maybeNotifyZeroInputs(Node node) {
         if (graph != null) {
             assert !graph.isFrozen();
-            NodeChangedListener listener = graph.usagesDroppedToZeroListener;
+            NodeEventListener listener = graph.nodeEventListener;
             if (listener != null) {
-                listener.nodeChanged(oldInput);
+                listener.usagesDroppedToZero(node);
             }
         }
     }
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java	Thu Jun 26 10:50:28 2014 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/CanonicalizerPhase.java	Thu Jun 26 13:42:29 2014 +0200
@@ -27,8 +27,7 @@
 import com.oracle.graal.debug.*;
 import com.oracle.graal.debug.Debug.Scope;
 import com.oracle.graal.graph.*;
-import com.oracle.graal.graph.Graph.Mark;
-import com.oracle.graal.graph.Graph.NodeChangedListener;
+import com.oracle.graal.graph.Graph.*;
 import com.oracle.graal.graph.spi.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.nodes.calc.*;
@@ -150,22 +149,26 @@
         }
 
         private void processWorkSet(StructuredGraph graph) {
-            NodeChangedListener nodeChangedListener = new NodeChangedListener() {
+            NodeEventListener listener = new NodeEventListener() {
 
-                @Override
-                public void nodeChanged(Node node) {
+                public void nodeAdded(Node node) {
+                    workList.add(node);
+                }
+
+                public void inputChanged(Node node) {
                     workList.add(node);
                 }
-            };
-            graph.trackInputChange(nodeChangedListener);
-            graph.trackUsagesDroppedZero(nodeChangedListener);
+
+                public void usagesDroppedToZero(Node node) {
+                    workList.add(node);
+                }
 
-            for (Node n : workList) {
-                processNode(n);
+            };
+            try (NodeEventScope nes = graph.trackNodeEvents(listener)) {
+                for (Node n : workList) {
+                    processNode(n);
+                }
             }
-
-            graph.stopTrackingInputChange();
-            graph.stopTrackingUsagesDroppedZero();
         }
 
         private void processNode(Node node) {
@@ -177,7 +180,6 @@
                     return;
                 }
                 StructuredGraph graph = (StructuredGraph) node.graph();
-                Mark mark = graph.getMark();
                 if (!GraphUtil.tryKillUnused(node)) {
                     if (!tryCanonicalize(node, nodeClass)) {
                         if (node instanceof ValueNode) {
@@ -196,12 +198,6 @@
                         }
                     }
                 }
-
-                if (!mark.isCurrent()) {
-                    for (Node newNode : graph.getNewNodes(mark)) {
-                        workList.add(newNode);
-                    }
-                }
             }
         }
 
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/IncrementalCanonicalizerPhase.java	Thu Jun 26 10:50:28 2014 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/IncrementalCanonicalizerPhase.java	Thu Jun 26 13:42:29 2014 +0200
@@ -22,7 +22,7 @@
  */
 package com.oracle.graal.phases.common;
 
-import com.oracle.graal.graph.Graph.Mark;
+import com.oracle.graal.graph.Graph.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.phases.*;
 import com.oracle.graal.phases.common.util.*;
@@ -47,19 +47,13 @@
 
     @Override
     protected void run(StructuredGraph graph, C context) {
-        Mark newNodesMark = graph.getMark();
-
-        HashSetNodeChangeListener listener = new HashSetNodeChangeListener();
-        graph.trackInputChange(listener);
-        graph.trackUsagesDroppedZero(listener);
+        HashSetNodeEventListener listener = new HashSetNodeEventListener();
+        try (NodeEventScope nes = graph.trackNodeEvents(listener)) {
+            super.run(graph, context);
+        }
 
-        super.run(graph, context);
-
-        graph.stopTrackingInputChange();
-        graph.stopTrackingUsagesDroppedZero();
-
-        if (graph.getMark() != newNodesMark || !listener.getChangedNodes().isEmpty()) {
-            canonicalizer.applyIncremental(graph, context, listener.getChangedNodes(), newNodesMark, false);
+        if (!listener.getNodes().isEmpty()) {
+            canonicalizer.applyIncremental(graph, context, listener.getNodes(), null, false);
         }
     }
 }
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/IterativeConditionalEliminationPhase.java	Thu Jun 26 10:50:28 2014 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/IterativeConditionalEliminationPhase.java	Thu Jun 26 13:42:29 2014 +0200
@@ -24,6 +24,7 @@
 
 import com.oracle.graal.api.code.*;
 import com.oracle.graal.graph.*;
+import com.oracle.graal.graph.Graph.*;
 import com.oracle.graal.graph.spi.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.phases.*;
@@ -43,24 +44,22 @@
     @Override
     protected void run(StructuredGraph graph, PhaseContext context) {
         ConditionalEliminationPhase eliminate = new ConditionalEliminationPhase(context.getMetaAccess());
-        HashSetNodeChangeListener listener = new HashSetNodeChangeListener();
+        HashSetNodeEventListener listener = new HashSetNodeEventListener.ExceptForAddedNodes();
         int count = 0;
         while (true) {
-            graph.trackInputChange(listener);
-            graph.trackUsagesDroppedZero(listener);
-            eliminate.apply(graph);
-            graph.stopTrackingInputChange();
-            graph.stopTrackingUsagesDroppedZero();
-            if (listener.getChangedNodes().isEmpty()) {
+            try (NodeEventScope nes = graph.trackNodeEvents(listener)) {
+                eliminate.apply(graph);
+            }
+            if (listener.getNodes().isEmpty()) {
                 break;
             }
             for (Node node : graph.getNodes()) {
                 if (node instanceof Simplifiable) {
-                    listener.getChangedNodes().add(node);
+                    listener.getNodes().add(node);
                 }
             }
-            canonicalizer.applyIncremental(graph, context, listener.getChangedNodes());
-            listener.getChangedNodes().clear();
+            canonicalizer.applyIncremental(graph, context, listener.getNodes());
+            listener.getNodes().clear();
             if (++count > MAX_ITERATIONS) {
                 throw new BailoutException("Number of iterations in ConditionalEliminationPhase phase exceeds " + MAX_ITERATIONS);
             }
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/IterativeFlowSensitiveReductionPhase.java	Thu Jun 26 10:50:28 2014 +0200
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/cfs/IterativeFlowSensitiveReductionPhase.java	Thu Jun 26 13:42:29 2014 +0200
@@ -24,6 +24,7 @@
 
 import com.oracle.graal.api.code.*;
 import com.oracle.graal.graph.*;
+import com.oracle.graal.graph.Graph.*;
 import com.oracle.graal.graph.spi.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.phases.*;
@@ -41,42 +42,28 @@
         this.canonicalizer = canonicalizer;
     }
 
-    public static class CountingListener extends HashSetNodeChangeListener {
-
-        public int count;
-
-        @Override
-        public void nodeChanged(Node node) {
-            super.nodeChanged(node);
-        }
-
-    }
-
     // private Histogram histogram = new Histogram("FSR-");
 
     @Override
     protected void run(StructuredGraph graph, PhaseContext context) {
         FlowSensitiveReductionPhase eliminate = new FlowSensitiveReductionPhase(context.getMetaAccess());
-        CountingListener listener = new CountingListener();
+        HashSetNodeEventListener listener = new HashSetNodeEventListener.ExceptForAddedNodes();
         int count = 1;
         while (true) {
-            listener.count = count;
-            graph.trackInputChange(listener);
-            graph.trackUsagesDroppedZero(listener);
-            eliminate.apply(graph, context);
-            graph.stopTrackingInputChange();
-            graph.stopTrackingUsagesDroppedZero();
-            if (listener.getChangedNodes().isEmpty()) {
+            try (NodeEventScope nes = graph.trackNodeEvents(listener)) {
+                eliminate.apply(graph, context);
+            }
+            if (listener.getNodes().isEmpty()) {
                 // histogram.tick(count);
                 break;
             }
             for (Node node : graph.getNodes()) {
                 if (node instanceof Simplifiable) {
-                    listener.getChangedNodes().add(node);
+                    listener.getNodes().add(node);
                 }
             }
-            canonicalizer.applyIncremental(graph, context, listener.getChangedNodes());
-            listener.getChangedNodes().clear();
+            canonicalizer.applyIncremental(graph, context, listener.getNodes());
+            listener.getNodes().clear();
             if (++count > MAX_ITERATIONS) {
                 // System.out.println("Bailing out IterativeFlowSensitiveReductionPhase for graph: "
                 // + graph);
--- a/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/util/HashSetNodeChangeListener.java	Thu Jun 26 10:50:28 2014 +0200
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,50 +0,0 @@
-/*
- * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
- * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
- *
- * This code is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License version 2 only, as
- * published by the Free Software Foundation.
- *
- * This code is distributed in the hope that it will be useful, but WITHOUT
- * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
- * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
- * version 2 for more details (a copy is included in the LICENSE file that
- * accompanied this code).
- *
- * You should have received a copy of the GNU General Public License version
- * 2 along with this work; if not, write to the Free Software Foundation,
- * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
- *
- * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
- * or visit www.oracle.com if you need additional information or have any
- * questions.
- */
-package com.oracle.graal.phases.common.util;
-
-import java.util.*;
-
-import com.oracle.graal.graph.Graph.NodeChangedListener;
-import com.oracle.graal.graph.*;
-
-/**
- * A simple {@link NodeChangedListener} implementation that accumulates the changed nodes in a
- * {@link HashSet}.
- */
-public class HashSetNodeChangeListener implements NodeChangedListener {
-
-    private final Set<Node> changedNodes;
-
-    public HashSetNodeChangeListener() {
-        this.changedNodes = new HashSet<>();
-    }
-
-    @Override
-    public void nodeChanged(Node node) {
-        changedNodes.add(node);
-    }
-
-    public Set<Node> getChangedNodes() {
-        return changedNodes;
-    }
-}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/graal/com.oracle.graal.phases.common/src/com/oracle/graal/phases/common/util/HashSetNodeEventListener.java	Thu Jun 26 13:42:29 2014 +0200
@@ -0,0 +1,70 @@
+/*
+ * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+package com.oracle.graal.phases.common.util;
+
+import java.util.*;
+
+import com.oracle.graal.graph.Graph.NodeEventListener;
+import com.oracle.graal.graph.*;
+
+/**
+ * A simple {@link NodeEventListener} implementation that accumulates event nodes in a
+ * {@link HashSet}.
+ */
+public class HashSetNodeEventListener implements NodeEventListener {
+
+    /**
+     * Accumulates all node events except for {@link NodeEventListener#nodeAdded(Node) node
+     * additions}.
+     */
+    public static class ExceptForAddedNodes extends HashSetNodeEventListener {
+        @Override
+        public void nodeAdded(Node node) {
+        }
+    }
+
+    private final Set<Node> nodes;
+
+    public HashSetNodeEventListener() {
+        this.nodes = new HashSet<>();
+    }
+
+    public void nodeAdded(Node node) {
+        nodes.add(node);
+    }
+
+    public void inputChanged(Node node) {
+        nodes.add(node);
+    }
+
+    public void usagesDroppedToZero(Node node) {
+        nodes.add(node);
+    }
+
+    /**
+     * Gets the set of nodes that were communicated to this listener.
+     */
+    public Set<Node> getNodes() {
+        return nodes;
+    }
+}
--- a/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/EffectsPhase.java	Thu Jun 26 10:50:28 2014 +0200
+++ b/graal/com.oracle.graal.virtual/src/com/oracle/graal/virtual/phases/ea/EffectsPhase.java	Thu Jun 26 13:42:29 2014 +0200
@@ -29,6 +29,7 @@
 import com.oracle.graal.debug.*;
 import com.oracle.graal.debug.Debug.Scope;
 import com.oracle.graal.graph.*;
+import com.oracle.graal.graph.Graph.*;
 import com.oracle.graal.graph.spi.*;
 import com.oracle.graal.nodes.*;
 import com.oracle.graal.nodes.cfg.*;
@@ -90,12 +91,10 @@
                 }
 
                 // apply the effects collected during this iteration
-                HashSetNodeChangeListener listener = new HashSetNodeChangeListener();
-                graph.trackInputChange(listener);
-                graph.trackUsagesDroppedZero(listener);
-                closure.applyEffects();
-                graph.stopTrackingInputChange();
-                graph.stopTrackingUsagesDroppedZero();
+                HashSetNodeEventListener listener = new HashSetNodeEventListener.ExceptForAddedNodes();
+                try (NodeEventScope nes = graph.trackNodeEvents(listener)) {
+                    closure.applyEffects();
+                }
 
                 if (Debug.isDumpEnabled()) {
                     Debug.dump(graph, "after " + getName() + " iteration");
@@ -103,7 +102,7 @@
 
                 new DeadCodeEliminationPhase().apply(graph);
 
-                Set<Node> changedNodes = listener.getChangedNodes();
+                Set<Node> changedNodes = listener.getNodes();
                 for (Node node : graph.getNodes()) {
                     if (node instanceof Simplifiable) {
                         changedNodes.add(node);