From 6db9cdb107917c10a0ce9846a338673e20aeeb9b Mon Sep 17 00:00:00 2001
From: Florian Angerer <florian.angerer@oracle.com>
Date: Thu, 20 Jul 2017 17:27:50 +0200
Subject: [PATCH] Refactored debug handling to cleanup bindings if not needed
any longer.
---
.../nodes/builtin/helpers/DebugHandling.java | 196 ++++++++++++------
.../instrument/InstrumentationState.java | 10 +-
.../truffle/r/test/ExpectedTestOutput.test | 72 +++++++
.../library/utils/TestInteractiveDebug.java | 11 +
4 files changed, 227 insertions(+), 62 deletions(-)
diff --git a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/helpers/DebugHandling.java b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/helpers/DebugHandling.java
index e9d18f142d..be71516f86 100644
--- a/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/helpers/DebugHandling.java
+++ b/com.oracle.truffle.r.nodes.builtin/src/com/oracle/truffle/r/nodes/builtin/helpers/DebugHandling.java
@@ -42,7 +42,6 @@ import com.oracle.truffle.api.nodes.RootNode;
import com.oracle.truffle.api.source.Source;
import com.oracle.truffle.api.source.SourceSection;
import com.oracle.truffle.api.utilities.CyclicAssumption;
-import com.oracle.truffle.r.launcher.ConsoleHandler;
import com.oracle.truffle.r.nodes.control.AbstractLoopNode;
import com.oracle.truffle.r.nodes.function.FunctionDefinitionNode;
import com.oracle.truffle.r.nodes.instrumentation.RInstrumentation;
@@ -110,8 +109,6 @@ public class DebugHandling {
/**
* Attach the DebugHandling instrument to the FunctionStatementsNode and all syntactic nodes.
- *
- * @param implicit TODO
*/
public static boolean enableDebug(RFunction func, Object text, Object condition, boolean once, boolean implicit) {
FunctionStatementsEventListener fbr = getFunctionStatementsEventListener(func);
@@ -120,18 +117,21 @@ public class DebugHandling {
} else {
fbr.enable();
fbr.setParentListener(null);
+
+ if (!fbr.isAttached()) {
+ fbr.attach();
+ }
}
return true;
}
public static boolean undebug(RFunction func) {
- FunctionStatementsEventListener fbr = getFunctionStatementsEventListener(func);
- if (fbr == null) {
- return false;
- } else {
- fbr.disable();
+ FunctionStatementsEventListener fsel = getFunctionStatementsEventListener(func);
+ if (fsel != null && !fsel.disabled()) {
+ fsel.dispose();
return true;
}
+ return false;
}
public static boolean isDebugged(RFunction func) {
@@ -139,12 +139,19 @@ public class DebugHandling {
return fser != null && (!fser.disabled() || fser.parentListener != null && !fser.parentListener.disabled());
}
- private static FunctionStatementsEventListener getFunctionStatementsEventListener(RFunction func) {
- return (FunctionStatementsEventListener) RContext.getInstance().stateInstrumentation.getDebugListener(RInstrumentation.getSourceSection(func));
+ /**
+ *
+ */
+ public static void dispose() {
+ for (ExecutionEventListener l : RContext.getInstance().stateInstrumentation.getDebugListeners()) {
+ if (l instanceof DebugEventListener) {
+ ((DebugEventListener) l).dispose();
+ }
+ }
}
- private static LineBreakpointEventListener getLineBreakpointEventListener(Source source, int lineNr) {
- return (LineBreakpointEventListener) RContext.getInstance().stateInstrumentation.getDebugListener(source.createSection(lineNr));
+ private static FunctionStatementsEventListener getFunctionStatementsEventListener(RFunction func) {
+ return (FunctionStatementsEventListener) RContext.getInstance().stateInstrumentation.getDebugListener(RInstrumentation.getSourceSection(func));
}
private static FunctionStatementsEventListener getFunctionStatementsEventListener(FunctionDefinitionNode fdn) {
@@ -158,48 +165,10 @@ public class DebugHandling {
@TruffleBoundary
private static FunctionStatementsEventListener attachDebugHandler(FunctionDefinitionNode fdn, Object text, Object condition, boolean once, boolean implicit) {
FunctionStatementsEventListener fser = new FunctionStatementsEventListener(fdn, text, condition, once, implicit);
- // First attach the main listener on the START_FUNCTION
- Instrumenter instrumenter = RInstrumentation.getInstrumenter();
- SourceSectionFilter.Builder functionBuilder = RInstrumentation.createFunctionFilter(fdn, StandardTags.RootTag.class);
- instrumenter.attachListener(functionBuilder.build(), fser);
- // Next attach statement handler to all STATEMENTs except LOOPs
- SourceSectionFilter.Builder statementBuilder = RInstrumentation.createFunctionStatementFilter(fdn);
- statementBuilder.tagIsNot(RSyntaxTags.LoopTag.class);
- instrumenter.attachListener(statementBuilder.build(), fser.getStatementListener());
- // Finally attach loop listeners to all loop nodes
- SourceSectionFilter.Builder loopBuilder = RInstrumentation.createFunctionFilter(fdn, RSyntaxTags.LoopTag.class);
- new RSyntaxVisitor<Void>() {
-
- @Override
- protected Void visit(RSyntaxCall element) {
- if (element instanceof AbstractLoopNode) {
- instrumenter.attachListener(loopBuilder.build(), fser.getLoopStatementReceiver((AbstractLoopNode) element));
- }
- accept(element.getSyntaxLHS());
- for (RSyntaxElement arg : element.getSyntaxArguments()) {
- if (arg != null) {
- accept(arg);
- }
- }
- return null;
- }
- @Override
- protected Void visit(RSyntaxConstant element) {
- return null;
- }
-
- @Override
- protected Void visit(RSyntaxLookup element) {
- return null;
- }
+ // First attach the main listener on the START_FUNCTION
+ fser.attach();
- @Override
- protected Void visit(RSyntaxFunction element) {
- accept(element.getSyntaxBody());
- return null;
- }
- }.accept(fdn);
return fser;
}
@@ -208,14 +177,17 @@ public class DebugHandling {
Instrumenter instrumenter = RInstrumentation.getInstrumenter();
SourceSection lineSourceSection = fdn.createSection(line);
SourceSectionFilter.Builder functionBuilder = RInstrumentation.createLineFilter(fdn, line, StandardTags.StatementTag.class);
- instrumenter.attachListener(functionBuilder.build(), new LineBreakpointEventListener(lineSourceSection));
+ LineBreakpointEventListener listener = new LineBreakpointEventListener(lineSourceSection);
+ listener.setBinding(instrumenter.attachListener(functionBuilder.build(), listener));
+ RContext.getInstance().stateInstrumentation.putDebugListener(lineSourceSection, listener);
}
@TruffleBoundary
public static void disableLineDebug(Source fdn, int line) {
- LineBreakpointEventListener l = getLineBreakpointEventListener(fdn, line);
+ LineBreakpointEventListener l = (LineBreakpointEventListener) RContext.getInstance().stateInstrumentation.getDebugListener(fdn.createSection(line));
if (l != null) {
- l.disable();
+ l.dispose();
+
if (l.fser != null) {
l.fser.setParentListener(null);
l.fser.disable();
@@ -234,6 +206,9 @@ public class DebugHandling {
// record initial state was disabled for undo
fser.enabledForStepInto = true;
}
+ if (!fser.isAttached()) {
+ fser.attach();
+ }
}
fser.setParentListener(parentListener);
return fser;
@@ -241,6 +216,8 @@ public class DebugHandling {
private abstract static class DebugEventListener implements ExecutionEventListener {
+ private EventBinding<? extends DebugEventListener> binding;
+
@TruffleBoundary
protected static void print(String msg, boolean nl) {
try {
@@ -271,6 +248,23 @@ public class DebugHandling {
disabled = newState;
}
}
+
+ public EventBinding<? extends DebugEventListener> getBinding() {
+ return binding;
+ }
+
+ public void setBinding(EventBinding<? extends DebugEventListener> binding) {
+ assert binding != null;
+ assert this.binding == null;
+ this.binding = binding;
+ }
+
+ public void dispose() {
+ if (binding != null && !binding.isDisposed()) {
+ binding.dispose();
+ binding = null;
+ }
+ }
}
private abstract static class InteractingDebugEventListener extends DebugEventListener {
@@ -348,6 +342,12 @@ public class DebugHandling {
stepIntoInstrument = null;
}
}
+
+ @Override
+ public void dispose() {
+ super.dispose();
+ clearStepInstrument();
+ }
}
/**
@@ -360,7 +360,7 @@ public class DebugHandling {
private static class FunctionStatementsEventListener extends InteractingDebugEventListener {
private final StatementEventListener statementListener;
- ArrayList<LoopStatementEventListener> loopStatementListeners = new ArrayList<>();
+ private final ArrayList<LoopStatementEventListener> loopStatementListeners = new ArrayList<>();
/**
* Denotes the {@code debugOnce} function, debugging disabled after one execution, or a
@@ -403,16 +403,84 @@ public class DebugHandling {
parentListener = l;
}
- StatementEventListener getStatementListener() {
- return statementListener;
+ void attachStatementListener() {
+ if (statementListener.getBinding() == null) {
+ SourceSectionFilter.Builder statementBuilder = RInstrumentation.createFunctionStatementFilter(functionDefinitionNode);
+ statementBuilder.tagIsNot(RSyntaxTags.LoopTag.class);
+ Instrumenter instrumenter = RInstrumentation.getInstrumenter();
+ statementListener.setBinding(instrumenter.attachListener(statementBuilder.build(), statementListener));
+ }
}
- LoopStatementEventListener getLoopStatementReceiver(RSyntaxNode loopNode) {
+ LoopStatementEventListener getLoopStatementReceiver(SourceSectionFilter ssf, RSyntaxNode loopNode) {
LoopStatementEventListener lser = new LoopStatementEventListener(functionDefinitionNode, text, condition, loopNode, this);
loopStatementListeners.add(lser);
+ Instrumenter instrumenter = RInstrumentation.getInstrumenter();
+ lser.setBinding(instrumenter.attachListener(ssf, lser));
return lser;
}
+ @Override
+ public void dispose() {
+ disable();
+ super.dispose();
+ statementListener.dispose();
+
+ for (LoopStatementEventListener lser : loopStatementListeners) {
+ lser.dispose();
+ }
+ loopStatementListeners.clear();
+ }
+
+ public boolean isAttached() {
+ return getBinding() != null && !getBinding().isDisposed();
+ }
+
+ public void attach() {
+
+ Instrumenter instrumenter = RInstrumentation.getInstrumenter();
+ SourceSectionFilter.Builder functionBuilder = RInstrumentation.createFunctionFilter(functionDefinitionNode, StandardTags.RootTag.class);
+ setBinding(instrumenter.attachListener(functionBuilder.build(), this));
+
+ // Next attach statement handler to all STATEMENTs except LOOPs
+ attachStatementListener();
+
+ // Finally attach loop listeners to all loop nodes
+ SourceSectionFilter.Builder loopBuilder = RInstrumentation.createFunctionFilter(functionDefinitionNode, RSyntaxTags.LoopTag.class);
+ new RSyntaxVisitor<Void>() {
+
+ @Override
+ protected Void visit(RSyntaxCall element) {
+ if (element instanceof AbstractLoopNode) {
+ getLoopStatementReceiver(loopBuilder.build(), (AbstractLoopNode) element);
+ }
+ accept(element.getSyntaxLHS());
+ for (RSyntaxElement arg : element.getSyntaxArguments()) {
+ if (arg != null) {
+ accept(arg);
+ }
+ }
+ return null;
+ }
+
+ @Override
+ protected Void visit(RSyntaxConstant element) {
+ return null;
+ }
+
+ @Override
+ protected Void visit(RSyntaxLookup element) {
+ return null;
+ }
+
+ @Override
+ protected Void visit(RSyntaxFunction element) {
+ accept(element.getSyntaxBody());
+ return null;
+ }
+ }.accept(functionDefinitionNode);
+ }
+
@Override
void disable() {
super.disable();
@@ -586,7 +654,6 @@ public class DebugHandling {
LineBreakpointEventListener(SourceSection ss) {
this.ss = ss;
- RContext.getInstance().stateInstrumentation.putDebugListener(ss, this);
}
@Override
@@ -620,6 +687,15 @@ public class DebugHandling {
fser.enableChildren();
}
}
+
+ @Override
+ public void dispose() {
+ super.dispose();
+ disable();
+ if (fser != null) {
+ fser.dispose();
+ }
+ }
}
/**
diff --git a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/instrument/InstrumentationState.java b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/instrument/InstrumentationState.java
index 8c91575ef9..10873939fb 100644
--- a/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/instrument/InstrumentationState.java
+++ b/com.oracle.truffle.r.runtime/src/com/oracle/truffle/r/runtime/instrument/InstrumentationState.java
@@ -24,6 +24,7 @@ package com.oracle.truffle.r.runtime.instrument;
import java.io.PrintStream;
import java.util.ArrayList;
+import java.util.Collection;
import java.util.HashSet;
import java.util.Map;
import java.util.WeakHashMap;
@@ -203,8 +204,8 @@ public final class InstrumentationState implements RContext.ContextState {
}
@TruffleBoundary
- public void putDebugListener(SourceSection ss, ExecutionEventListener listener) {
- debugListenerMap.put(ss, listener);
+ public void putDebugListener(SourceSection sourceSection, ExecutionEventListener listener) {
+ debugListenerMap.put(sourceSection, listener);
}
@TruffleBoundary
@@ -215,6 +216,11 @@ public final class InstrumentationState implements RContext.ContextState {
}
+ @TruffleBoundary
+ public Collection<ExecutionEventListener> getDebugListeners() {
+ return debugListenerMap.values();
+ }
+
@TruffleBoundary
public ExecutionEventListener getDebugListener(SourceSection ss) {
return debugListenerMap.get(ss);
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
index f6a1d11272..8e0463ffec 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/ExpectedTestOutput.test
@@ -147946,6 +147946,45 @@ debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#6: bar("Wo
debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#7: print(x)
[1] 10
+##com.oracle.truffle.r.test.library.utils.TestInteractiveDebug.testSetBreakpoint#Output.IgnoreDebugCallString#Output.IgnoreDebugPath#
+#source('tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r'); setBreakpoint('tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r', 4, verbose=F); fun(10)<<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>>setBreakpoint('tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r', 4, verbose=F, clear=T); fun(10)<<<NEWLINE>>>setBreakpoint('tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r', 4, verbose=F); invisible(fun(10))<<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>>
+debug.r#4
+Called from: fun(10)
+debug: print("Hello")
+[1] "Hello"
+debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#5: for (i in seq(3)) print(i)
+debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#5: print(i)
+[1] 1
+debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#5: print(i)
+[1] 2
+debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#5: print(i)
+[1] 3
+debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#6: bar("World")
+[1] "World"
+debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#7: print(x)
+[1] 10
+[1] "Hello"
+[1] 1
+[1] 2
+[1] 3
+[1] "World"
+[1] 10
+debug.r#4
+Called from: fun(10)
+debug: print("Hello")
+[1] "Hello"
+debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#5: for (i in seq(3)) print(i)
+debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#5: print(i)
+[1] 1
+debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#5: print(i)
+[1] 2
+debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#5: print(i)
+[1] 3
+debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#6: bar("World")
+[1] "World"
+debug at tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r#7: print(x)
+[1] 10
+
##com.oracle.truffle.r.test.library.utils.TestInteractiveDebug.testSetBreakpoint#
#source('tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r'); setBreakpoint('tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r', 4, verbose=F); setBreakpoint('tmptest/com.oracle.truffle.r.test.library.utils.rsrc/debug.r', 4, verbose=F, clear=T); fun(10)<<<NEWLINE>>>
[1] "Hello"
@@ -147972,6 +148011,39 @@ debug at #4: t
exiting from: f(5)
[1] 6
+##com.oracle.truffle.r.test.library.utils.TestInteractiveDebug.testSimple#
+#f <- function(x) {<<<NEWLINE>>> t <- x + 1<<<NEWLINE>>> print(t)<<<NEWLINE>>> t}<<<NEWLINE>>>debug(f)<<<NEWLINE>>>f(5)<<<NEWLINE>>>x<<<NEWLINE>>>n<<<NEWLINE>>>n<<<NEWLINE>>>t<<<NEWLINE>>>n<<<NEWLINE>>>n<<<NEWLINE>>>undebug(f)<<<NEWLINE>>>f(3)<<<NEWLINE>>>debug(f)<<<NEWLINE>>>f(5)<<<NEWLINE>>>x<<<NEWLINE>>>n<<<NEWLINE>>>n<<<NEWLINE>>>t<<<NEWLINE>>>n<<<NEWLINE>>>n
+debugging in: f(5)
+debug at #1: {
+ t <- x + 1
+ print(t)
+ t
+}
+[1] 5
+debug at #2: t <- x + 1
+debug at #3: print(t)
+[1] 6
+[1] 6
+debug at #4: t
+exiting from: f(5)
+[1] 6
+[1] 4
+[1] 4
+debugging in: f(5)
+debug at #1: {
+ t <- x + 1
+ print(t)
+ t
+}
+[1] 5
+debug at #2: t <- x + 1
+debug at #3: print(t)
+[1] 6
+[1] 6
+debug at #4: t
+exiting from: f(5)
+[1] 6
+
##com.oracle.truffle.r.test.library.utils.TestInteractiveDebug.testStepInto#
#bar <- function(x) { cat(x); cat('\n') }; foo <- function(x) { cat('foo entry\n'); bar(x); cat('foo exit\n') }; debug(foo); foo(3)<<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>>s<<<NEWLINE>>>n<<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>><<<NEWLINE>>>
debugging in: foo(3)
diff --git a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/library/utils/TestInteractiveDebug.java b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/library/utils/TestInteractiveDebug.java
index c9b996f590..f497f3ba31 100644
--- a/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/library/utils/TestInteractiveDebug.java
+++ b/com.oracle.truffle.r.test/src/com/oracle/truffle/r/test/library/utils/TestInteractiveDebug.java
@@ -27,16 +27,24 @@ import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardOpenOption;
+import org.junit.After;
import org.junit.BeforeClass;
import org.junit.Test;
+import com.oracle.truffle.r.nodes.builtin.helpers.DebugHandling;
import com.oracle.truffle.r.test.TestBase;
// Checkstyle: stop line length check
public class TestInteractiveDebug extends TestBase {
+ @After
+ public void cleanupDebugListeners() {
+ DebugHandling.dispose();
+ }
+
@Test
public void testSimple() {
assertEval("f <- function(x) {\n t <- x + 1\n print(t)\n t}\ndebug(f)\nf(5)\nx\nn\nn\nt\nn\nn");
+ assertEval("f <- function(x) {\n t <- x + 1\n print(t)\n t}\ndebug(f)\nf(5)\nx\nn\nn\nt\nn\nn\nundebug(f)\nf(3)\ndebug(f)\nf(5)\nx\nn\nn\nt\nn\nn");
}
@Test
@@ -117,5 +125,8 @@ public class TestInteractiveDebug extends TestBase {
public void testSetBreakpoint() {
assertEval(Output.IgnoreDebugCallString, Output.IgnoreDebugPath, String.format("source('%s'); setBreakpoint('%s', 4, verbose=F); fun(10)\n\n\n\n\n\n\n\n", debugFile, debugFile));
assertEval(String.format("source('%s'); setBreakpoint('%s', 4, verbose=F); setBreakpoint('%s', 4, verbose=F, clear=T); fun(10)\n", debugFile, debugFile, debugFile));
+ assertEval(Output.IgnoreDebugCallString, Output.IgnoreDebugPath, String.format(
+ "source('%s'); setBreakpoint('%s', 4, verbose=F); fun(10)\n\n\n\n\n\n\n\n\nsetBreakpoint('%s', 4, verbose=F, clear=T); fun(10)\nsetBreakpoint('%s', 4, verbose=F); invisible(fun(10))\n\n\n\n\n\n\n\n\n",
+ debugFile, debugFile, debugFile, debugFile));
}
}
--
GitLab