# HG changeset patch # User Thomas Wuerthinger # Date 1424342213 -3600 # Node ID c162000dd30c48c65d8cc975e04de93ca0a00382 # Parent 45c90acd813e0238369684c9663ff553f4561633# Parent 5a91549293dfb54fba426946858ed21ad7a49a7b Merge. diff -r 45c90acd813e -r c162000dd30c graal/com.oracle.graal.truffle.test/src/com/oracle/graal/truffle/test/InstrumentationPartialEvaluationTest.java --- 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 + + } } diff -r 45c90acd813e -r c162000dd30c graal/com.oracle.graal.truffle.test/src/com/oracle/graal/truffle/test/nodes/RootTestNode.java --- 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; } diff -r 45c90acd813e -r c162000dd30c graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/Probe.java --- 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> 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. + * * * @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 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 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 tags = new ArrayList<>(); private final List> 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() { diff -r 45c90acd813e -r c162000dd30c graal/com.oracle.truffle.api/src/com/oracle/truffle/api/instrument/ProbeNode.java --- 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() {