changeset 19490:745ecef4c9cd

Truffle/Instrumentation: clean up the use of Assumptions in the Probe (and attached Instruments)
author Michael Van De Vanter <michael.van.de.vanter@oracle.com>
date Wed, 18 Feb 2015 18:07:48 -0800
parents c99c7a4cda7d
children 4d66c000d253
files graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Probe.java graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/ProbeNode.java
diffstat 2 files changed, 103 insertions(+), 117 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Probe.java	Wed Feb 18 16:16:38 2015 -0800
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Probe.java	Wed Feb 18 18:07:48 2015 -0800
@@ -106,12 +106,6 @@
     }
 
     /**
-     * The tag trap is a global setting; it only affects {@linkplain Probe probes} with the
-     * {@linkplain SyntaxTag tag} specified .
-     */
-    @CompilationFinal private static SyntaxTagTrap globalTagTrap = null;
-
-    /**
      * Enables instrumentation at selected nodes in all subsequently constructed ASTs.
      */
     public static void registerASTProber(ASTProber prober) {
@@ -157,8 +151,8 @@
     }
 
     /**
-     * Returns all {@link Probe}s holding a particular {@link SyntaxTag}, or the whole collection if
-     * the specified tag is {@code null}.
+     * Returns all {@link Probe}s holding a particular {@link SyntaxTag}, or the whole collection of
+     * probes if the specified tag is {@code null}.
      *
      * @return A collection of probes containing the given tag.
      */
@@ -176,46 +170,21 @@
     }
 
     /**
-     * Sets the current "tag trap". This causes a callback to be triggered whenever execution
-     * reaches a {@link Probe} (either existing or subsequently created) with the specified tag.
-     * There can only be one tag trap set at a time.
+     * Sets the current "tag trap"; there can be no more than one set at a time.
+     * <ul>
+     * <li>A non-null trap sets a callback to be triggered whenever execution reaches a
+     * {@link Probe} (either existing or subsequently created) with the specified tag.</li>
+     * <li>Setting the trap to null clears the existing trap.</li>
+     * <li>Setting a non-null trap when one is already set will clear the previously set trap.</li>
+     * </ul>
      *
      * @param newTagTrap The {@link SyntaxTagTrap} to set.
-     * @throws IllegalStateException if a trap is currently set.
      */
-    public static void setTagTrap(SyntaxTagTrap newTagTrap) throws IllegalStateException {
-        assert newTagTrap != null;
-        if (globalTagTrap != null) {
-            throw new IllegalStateException("trap already set");
-        }
-        globalTagTrap = newTagTrap;
-
-        final SyntaxTag newTag = newTagTrap.getTag();
+    public static void setTagTrap(SyntaxTagTrap newTagTrap) {
         for (WeakReference<Probe> ref : probes) {
             final Probe probe = ref.get();
-            if (probe != null && probe.tags.contains(newTag)) {
-                probe.trapActive = true;
-                probe.probeStateUnchanged.invalidate();
-            }
-        }
-    }
-
-    /**
-     * Clears the current {@link SyntaxTagTrap}.
-     *
-     * @throws IllegalStateException if no trap is currently set.
-     */
-    public static void clearTagTrap() {
-        if (globalTagTrap == null) {
-            throw new IllegalStateException("no trap set");
-        }
-        globalTagTrap = null;
-
-        for (WeakReference<Probe> ref : probes) {
-            final Probe probe = ref.get();
-            if (probe != null && probe.trapActive) {
-                probe.trapActive = false;
-                probe.probeStateUnchanged.invalidate();
+            if (probe != null) {
+                probe.setTrap(newTagTrap);
             }
         }
     }
@@ -223,12 +192,22 @@
     private final SourceSection sourceSection;
     private final ArrayList<SyntaxTag> tags = new ArrayList<>();
     private final List<WeakReference<ProbeNode>> probeNodeClones = new ArrayList<>();
-    private final CyclicAssumption probeStateUnchanged = new CyclicAssumption("Probe state unchanged");
+
+    /*
+     * Invalidated whenever something changes in the Probe and its Instrument chain, so need deopt
+     */
+    private final CyclicAssumption probeStateUnchangedCyclic = new CyclicAssumption("Probe state unchanged");
 
-    /**
-     * {@code true} iff the global trap is set and this probe has the matching tag.
+    /*
+     * The assumption that nothing had changed in this probe, the last time anybody checked (when
+     * there may have been a deopt). Every time a check fails, gets replaced by a new unchanged
+     * assumption.
      */
-    private boolean trapActive = false;
+    @CompilationFinal private Assumption probeStateUnchangedAssumption = probeStateUnchangedCyclic.getAssumption();
+
+    // Must invalidate whenever either of these these changes.
+    @CompilationFinal private SyntaxTagTrap tagTrap = null;
+    @CompilationFinal private boolean isTrapActive = false;
 
     /**
      * Intended for use only by {@link ProbeNode}.
@@ -262,10 +241,10 @@
             for (ProbeListener listener : probeListeners) {
                 listener.probeTaggedAs(this, tag, tagValue);
             }
-            if (globalTagTrap != null && tag == globalTagTrap.getTag()) {
-                this.trapActive = true;
+            if (tagTrap != null && tag == tagTrap.getTag()) {
+                this.isTrapActive = true;
+                invalidateProbeUnchanged();
             }
-            probeStateUnchanged.invalidate();
         }
     }
 
@@ -289,7 +268,7 @@
                 probeNode.addInstrument(instrument);
             }
         }
-        probeStateUnchanged.invalidate();
+        invalidateProbeUnchanged();
     }
 
     /**
@@ -306,29 +285,6 @@
     }
 
     /**
-     * Receives notification that a new clone of the instrument chain associated with this
-     * {@link Probe} has been created as a side-effect of AST cloning.
-     */
-    void registerProbeNodeClone(ProbeNode probeNode) {
-        probeNodeClones.add(new WeakReference<>(probeNode));
-    }
-
-    /**
-     * Gets the currently active {@linkplain SyntaxTagTrap tagTrap}; {@code null} if not set.
-     */
-    SyntaxTagTrap getTrap() {
-        return trapActive ? globalTagTrap : null;
-    }
-
-    /**
-     * Gets the {@link Assumption} that the instrumentation-related state of this {@link Probe} has
-     * not changed since this method was last called.
-     */
-    Assumption getUnchangedAssumption() {
-        return probeStateUnchanged.getAssumption();
-    }
-
-    /**
      * Internal method for removing and rendering inert a specific instrument previously attached at
      * this Probe.
      *
@@ -343,7 +299,58 @@
                 probeNode.removeInstrument(instrument);
             }
         }
-        probeStateUnchanged.invalidate();
+        invalidateProbeUnchanged();
+    }
+
+    /**
+     * Receives notification that a new clone of the instrument chain associated with this
+     * {@link Probe} has been created as a side-effect of AST cloning.
+     */
+    void registerProbeNodeClone(ProbeNode probeNode) {
+        probeNodeClones.add(new WeakReference<>(probeNode));
+    }
+
+    /**
+     * Gets the currently active {@linkplain SyntaxTagTrap tagTrap}; {@code null} if not set.
+     */
+    SyntaxTagTrap getTrap() {
+        checkProbeUnchanged();
+        return isTrapActive ? tagTrap : null;
+    }
+
+    /**
+     * To be called wherever in the Probe/Instrument chain there are dependencies on the probe
+     * state's @CompilatonFinal fields.
+     */
+    void checkProbeUnchanged() {
+        try {
+            probeStateUnchangedAssumption.check();
+        } catch (InvalidAssumptionException ex) {
+            // Failure creates an implicit deoptimization
+            // Get the assumption associated with the new probe state
+            this.probeStateUnchangedAssumption = probeStateUnchangedCyclic.getAssumption();
+        }
+    }
+
+    private void invalidateProbeUnchanged() {
+        probeStateUnchangedCyclic.invalidate();
+    }
+
+    private void setTrap(SyntaxTagTrap newTagTrap) {
+        // No-op if same trap; traps are immutable
+        if (this.tagTrap != newTagTrap) {
+            if (newTagTrap == null) {
+                this.tagTrap = null;
+                if (isTrapActive) {
+                    isTrapActive = false;
+                    invalidateProbeUnchanged();
+                }
+            } else { // new trap is non-null
+                this.tagTrap = newTagTrap;
+                this.isTrapActive = this.isTaggedAs(newTagTrap.getTag());
+                invalidateProbeUnchanged();
+            }
+        }
     }
 
     private String getTagsDescription() {
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/ProbeNode.java	Wed Feb 18 16:16:38 2015 -0800
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/ProbeNode.java	Wed Feb 18 18:07:48 2015 -0800
@@ -24,7 +24,6 @@
  */
 package com.oracle.truffle.api.instrument;
 
-import com.oracle.truffle.api.*;
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.frame.*;
@@ -176,13 +175,6 @@
         // Never changed once set.
         @CompilationFinal private Probe probe = null;
 
-        /**
-         * An assumption that the state of the {@link Probe} with which this chain is associated has
-         * not changed since the last time checking such an assumption failed and a reference to a
-         * new assumption (associated with a new state of the {@link Probe} was retrieved.
-         */
-        @CompilationFinal private Assumption probeUnchangedAssumption;
-
         private ProbeFullNode() {
             this.firstInstrument = null;
         }
@@ -201,17 +193,6 @@
 
         private void setProbe(Probe probe) {
             this.probe = probe;
-            this.probeUnchangedAssumption = probe.getUnchangedAssumption();
-        }
-
-        private void checkProbeUnchangedAssumption() {
-            try {
-                probeUnchangedAssumption.check();
-            } catch (InvalidAssumptionException ex) {
-                // Failure creates an implicit deoptimization
-                // Get the assumption associated with the new probe state
-                this.probeUnchangedAssumption = probe.getUnchangedAssumption();
-            }
         }
 
         @Override
@@ -235,43 +216,41 @@
             }
         }
 
-        public void enter(Node node, VirtualFrame frame) {
+        public void enter(Node node, VirtualFrame vFrame) {
+            this.probe.checkProbeUnchanged();
             final SyntaxTagTrap trap = probe.getTrap();
             if (trap != null) {
-                checkProbeUnchangedAssumption();
-                trap.tagTrappedAt(((WrapperNode) this.getParent()).getChild(), frame.materialize());
+                trap.tagTrappedAt(((WrapperNode) this.getParent()).getChild(), vFrame.materialize());
             }
             if (firstInstrument != null) {
-                checkProbeUnchangedAssumption();
-                firstInstrument.enter(node, frame);
+                firstInstrument.enter(node, vFrame);
             }
         }
 
-        public void returnVoid(Node node, VirtualFrame frame) {
+        public void returnVoid(Node node, VirtualFrame vFrame) {
+            this.probe.checkProbeUnchanged();
             if (firstInstrument != null) {
-                checkProbeUnchangedAssumption();
-                firstInstrument.returnVoid(node, frame);
+                firstInstrument.returnVoid(node, vFrame);
             }
         }
 
-        public void returnValue(Node node, VirtualFrame frame, Object result) {
+        public void returnValue(Node node, VirtualFrame vFrame, Object result) {
+            this.probe.checkProbeUnchanged();
             if (firstInstrument != null) {
-                checkProbeUnchangedAssumption();
-                firstInstrument.returnValue(node, frame, result);
+                firstInstrument.returnValue(node, vFrame, result);
             }
         }
 
-        public void returnExceptional(Node node, VirtualFrame frame, Exception exception) {
+        public void returnExceptional(Node node, VirtualFrame vFrame, Exception exception) {
+            this.probe.checkProbeUnchanged();
             if (firstInstrument != null) {
-                checkProbeUnchangedAssumption();
-                firstInstrument.returnExceptional(node, frame, exception);
+                firstInstrument.returnExceptional(node, vFrame, exception);
             }
         }
 
         public String instrumentationInfo() {
             return "Standard probe";
         }
-
     }
 
     /**
@@ -305,20 +284,20 @@
             throw new IllegalStateException("Instruments may not be removed at a \"lite-probed\" location");
         }
 
-        public void enter(Node node, VirtualFrame frame) {
-            eventListener.enter(node, frame);
+        public void enter(Node node, VirtualFrame vFrame) {
+            eventListener.enter(node, vFrame);
         }
 
-        public void returnVoid(Node node, VirtualFrame frame) {
-            eventListener.returnVoid(node, frame);
+        public void returnVoid(Node node, VirtualFrame vFrame) {
+            eventListener.returnVoid(node, vFrame);
         }
 
-        public void returnValue(Node node, VirtualFrame frame, Object result) {
-            eventListener.returnValue(node, frame, result);
+        public void returnValue(Node node, VirtualFrame vFrame, Object result) {
+            eventListener.returnValue(node, vFrame, result);
         }
 
-        public void returnExceptional(Node node, VirtualFrame frame, Exception exception) {
-            eventListener.returnExceptional(node, frame, exception);
+        public void returnExceptional(Node node, VirtualFrame vFrame, Exception exception) {
+            eventListener.returnExceptional(node, vFrame, exception);
         }
 
         public String instrumentationInfo() {