Mercurial > hg > truffle

changeset 19494:c162000dd30c

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
Merge.
author Thomas Wuerthinger <thomas.wuerthinger@oracle.com>
date Thu, 19 Feb 2015 11:36:53 +0100
parents 45c90acd813e (current diff) 5a91549293df (diff)
children 910c4f1006c9 068256ee3b90
files
diffstat 4 files changed, 155 insertions(+), 124 deletions(-) [+]
line wrap: on
line diff
--- a/graal/com.oracle.graal.truffle.test/src/com/oracle/graal/truffle/test/InstrumentationPartialEvaluationTest.java	Wed Feb 18 23:34:48 2015 +0100
+++ b/graal/com.oracle.graal.truffle.test/src/com/oracle/graal/truffle/test/InstrumentationPartialEvaluationTest.java	Thu Feb 19 11:36:53 2015 +0100
@@ -25,9 +25,11 @@
 import org.junit.*;
 
 import com.oracle.graal.truffle.test.nodes.*;
+import com.oracle.truffle.api.*;
 import com.oracle.truffle.api.frame.*;
 import com.oracle.truffle.api.instrument.*;
 import com.oracle.truffle.api.instrument.impl.*;
+import com.oracle.truffle.api.nodes.*;
 
 /**
  * Tests for a single simple PE test with various combinations of instrumentation attached. None of
@@ -48,7 +50,6 @@
         assertPartialEvalEquals("constant42", root);
     }
 
-    @Ignore
     @Test
     public void constantValueProbedNoInstruments() {
         FrameDescriptor fd = new FrameDescriptor();
@@ -59,7 +60,6 @@
         assertPartialEvalEquals("constant42", root);
     }
 
-    @Ignore
     @Test
     public void constantValueProbedNullInstrument() {
         FrameDescriptor fd = new FrameDescriptor();
@@ -72,7 +72,6 @@
         assertPartialEvalEquals("constant42", root);
     }
 
-    @Ignore
     @Test
     public void constantValueProbedNullInstrumentDisposed() {
         FrameDescriptor fd = new FrameDescriptor();
@@ -86,7 +85,6 @@
         assertPartialEvalEquals("constant42", root);
     }
 
-    @Ignore
     @Test
     public void constantValueProbedTwoNullInstruments() {
         FrameDescriptor fd = new FrameDescriptor();
@@ -101,7 +99,6 @@
         assertPartialEvalEquals("constant42", root);
     }
 
-    @Ignore
     @Test
     public void constantValueProbedThreeNullInstruments() {
         FrameDescriptor fd = new FrameDescriptor();
@@ -118,7 +115,6 @@
         assertPartialEvalEquals("constant42", root);
     }
 
-    @Ignore
     @Test
     public void constantValueProbedThreeNullInstrumentsOneDisposed() {
         FrameDescriptor fd = new FrameDescriptor();
@@ -135,4 +131,57 @@
         instrument2.dispose();
         assertPartialEvalEquals("constant42", root);
     }
+
+    @Test
+    public void instrumentDeopt() {
+        final FrameDescriptor fd = new FrameDescriptor();
+        final AbstractTestNode result = new ConstantTestNode(42);
+        final RootTestNode root = new RootTestNode(fd, "constantValue", result);
+        final Probe[] probe = new Probe[1];
+        final int[] count = {1};
+        count[0] = 0;
+        // Register a "prober" that will get applied when CallTarget gets created.
+        Probe.registerASTProber(new ASTProber() {
+
+            @Override
+            public void probeAST(Node node) {
+                node.accept(new NodeVisitor() {
+
+                    @Override
+                    public boolean visit(Node visitedNode) {
+                        if (visitedNode instanceof ConstantTestNode) {
+                            probe[0] = visitedNode.probe();
+                        }
+                        return true;
+                    }
+
+                });
+            }
+        });
+        final RootCallTarget callTarget = Truffle.getRuntime().createCallTarget(root);
+
+        // The CallTarget has one Probe, attached to the ConstantTestNode, ready to run
+        Assert.assertEquals(42, callTarget.call()); // Correct result
+        Assert.assertEquals(0, count[0]);           // Didn't count anything
+
+        // Add a counting instrument; this changes the "Probe state" and should cause a deopt
+        final Instrument countingInstrument = Instrument.create(new DefaultEventListener() {
+
+            @Override
+            public void enter(Node node, VirtualFrame frame) {
+                count[0] = count[0] + 1;
+            }
+        });
+        probe[0].attach(countingInstrument);
+
+        Assert.assertEquals(42, callTarget.call()); // Correct result
+        Assert.assertEquals(1, count[0]);           // Counted the first call
+
+        // Remove the counting instrument; this changes the "Probe state" and should cause a deopt
+        countingInstrument.dispose();
+
+        Assert.assertEquals(42, callTarget.call()); // Correct result
+        Assert.assertEquals(1, count[0]);           // Didn't count this time
+
+    }
 }
--- a/graal/com.oracle.graal.truffle.test/src/com/oracle/graal/truffle/test/nodes/RootTestNode.java	Wed Feb 18 23:34:48 2015 +0100
+++ b/graal/com.oracle.graal.truffle.test/src/com/oracle/graal/truffle/test/nodes/RootTestNode.java	Thu Feb 19 11:36:53 2015 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 2015, 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
@@ -23,6 +23,7 @@
 package com.oracle.graal.truffle.test.nodes;
 
 import com.oracle.truffle.api.frame.*;
+import com.oracle.truffle.api.instrument.*;
 import com.oracle.truffle.api.nodes.*;
 
 @NodeInfo
@@ -43,6 +44,11 @@
     }
 
     @Override
+    public void applyInstrumentation() {
+        Probe.applyASTProbers(node);
+    }
+
+    @Override
     public String toString() {
         return name;
     }
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Probe.java	Wed Feb 18 23:34:48 2015 +0100
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Probe.java	Thu Feb 19 11:36:53 2015 +0100
@@ -28,6 +28,7 @@
 import java.util.*;
 
 import com.oracle.truffle.api.*;
+import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
 import com.oracle.truffle.api.nodes.*;
 import com.oracle.truffle.api.source.*;
 import com.oracle.truffle.api.utilities.*;
@@ -80,6 +81,8 @@
      */
     private static final List<WeakReference<Probe>> probes = new ArrayList<>();
 
+    @CompilationFinal private static SyntaxTagTrap tagTrap = null;
+
     private static final class FindSourceVisitor implements NodeVisitor {
 
         Source source = null;
@@ -105,12 +108,6 @@
     }
 
     /**
-     * The tag trap is a global setting; it only affects {@linkplain Probe probes} with the
-     * {@linkplain SyntaxTag tag} specified .
-     */
-    private static SyntaxTagTrap globalTagTrap = null;
-
-    /**
      * Enables instrumentation at selected nodes in all subsequently constructed ASTs.
      */
     public static void registerASTProber(ASTProber prober) {
@@ -156,8 +153,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.
      */
@@ -175,46 +172,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.notifyTrapSet();
             }
         }
     }
@@ -222,12 +194,21 @@
     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 this changes.
+    @CompilationFinal private boolean isTrapActive = false;
 
     /**
      * Intended for use only by {@link ProbeNode}.
@@ -261,10 +242,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();
         }
     }
 
@@ -288,7 +269,7 @@
                 probeNode.addInstrument(instrument);
             }
         }
-        probeStateUnchanged.invalidate();
+        invalidateProbeUnchanged();
     }
 
     /**
@@ -305,29 +286,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.
      *
@@ -342,7 +300,46 @@
                 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 notifyTrapSet() {
+        this.isTrapActive = tagTrap != null && this.isTaggedAs(tagTrap.getTag());
+        invalidateProbeUnchanged();
     }
 
     private String getTagsDescription() {
--- a/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/ProbeNode.java	Wed Feb 18 23:34:48 2015 +0100
+++ b/graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/ProbeNode.java	Thu Feb 19 11:36:53 2015 +0100
@@ -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.
-         */
-        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() {