Mercurial > hg > truffle
changeset 22475:c434681cd164
Truffle/REPL debugger: rewrite the management of breakpoint information that permits pre-execution setting of breakpoints, no longer depends directly on the Breakpoint classes.
author | Michael Van De Vanter <michael.van.de.vanter@oracle.com> |
---|---|
date | Mon, 07 Dec 2015 18:21:03 -0800 |
parents | c04f6f51b9bf |
children | 4a83dc15e774 |
files | truffle/com.oracle.truffle.tools.debug.shell/src/com/oracle/truffle/tools/debug/shell/server/REPLHandler.java truffle/com.oracle.truffle.tools.debug.shell/src/com/oracle/truffle/tools/debug/shell/server/REPLServer.java |
diffstat | 2 files changed, 194 insertions(+), 207 deletions(-) [+] |
line wrap: on
line diff
--- a/truffle/com.oracle.truffle.tools.debug.shell/src/com/oracle/truffle/tools/debug/shell/server/REPLHandler.java Mon Dec 07 18:19:25 2015 -0800 +++ b/truffle/com.oracle.truffle.tools.debug.shell/src/com/oracle/truffle/tools/debug/shell/server/REPLHandler.java Mon Dec 07 18:21:03 2015 -0800 @@ -29,7 +29,6 @@ import java.util.Collection; import java.util.List; -import com.oracle.truffle.api.debug.Breakpoint; import com.oracle.truffle.api.debug.FrameDebugDescription; import com.oracle.truffle.api.frame.Frame; import com.oracle.truffle.api.frame.FrameSlot; @@ -43,6 +42,7 @@ import com.oracle.truffle.api.source.SourceSection; import com.oracle.truffle.api.vm.PolyglotEngine.Language; import com.oracle.truffle.tools.debug.shell.REPLMessage; +import com.oracle.truffle.tools.debug.shell.server.REPLServer.BreakpointInfo; import com.oracle.truffle.tools.debug.shell.server.REPLServer.Context; /** @@ -108,15 +108,15 @@ return replies; } - protected static final REPLMessage createBreakpointInfoMessage(Breakpoint breakpoint, REPLServer replServer) { + protected static final REPLMessage createBreakpointInfoMessage(BreakpointInfo info) { final REPLMessage infoMessage = new REPLMessage(REPLMessage.OP, REPLMessage.BREAKPOINT_INFO); - infoMessage.put(REPLMessage.BREAKPOINT_ID, Integer.toString(replServer.getBreakpointID(breakpoint))); - infoMessage.put(REPLMessage.BREAKPOINT_STATE, breakpoint.getState().getName()); - infoMessage.put(REPLMessage.BREAKPOINT_HIT_COUNT, Integer.toString(breakpoint.getHitCount())); - infoMessage.put(REPLMessage.BREAKPOINT_IGNORE_COUNT, Integer.toString(breakpoint.getIgnoreCount())); - infoMessage.put(REPLMessage.INFO_VALUE, breakpoint.getLocationDescription().toString()); - if (breakpoint.getCondition() != null) { - infoMessage.put(REPLMessage.BREAKPOINT_CONDITION, breakpoint.getCondition().getCode()); + infoMessage.put(REPLMessage.BREAKPOINT_ID, Integer.toString(info.getID())); + infoMessage.put(REPLMessage.BREAKPOINT_STATE, info.describeState()); + infoMessage.put(REPLMessage.BREAKPOINT_HIT_COUNT, Integer.toString(info.getHitCount())); + infoMessage.put(REPLMessage.BREAKPOINT_IGNORE_COUNT, Integer.toString(info.getIgnoreCount())); + infoMessage.put(REPLMessage.INFO_VALUE, info.describeLocation()); + if (info.getCondition() != null) { + infoMessage.put(REPLMessage.BREAKPOINT_CONDITION, info.getCondition()); } infoMessage.put(REPLMessage.STATUS, REPLMessage.SUCCEEDED); return infoMessage; @@ -199,7 +199,7 @@ if (source == null) { return finishReplyFailed(reply, fileName + " not found"); } - Integer lineNumber = request.getIntValue(REPLMessage.LINE_NUMBER); + final Integer lineNumber = request.getIntValue(REPLMessage.LINE_NUMBER); if (lineNumber == null) { return finishReplyFailed(reply, "missing line number"); } @@ -207,15 +207,10 @@ if (ignoreCount == null) { ignoreCount = 0; } - Breakpoint breakpoint; - try { - breakpoint = replServer.setLineBreakpoint(DEFAULT_IGNORE_COUNT, source.createLineLocation(lineNumber), false); - } catch (IOException ex) { - return finishReplyFailed(reply, ex); - } + final BreakpointInfo breakpointInfo = replServer.setLineBreakpoint(DEFAULT_IGNORE_COUNT, source.createLineLocation(lineNumber), false); reply.put(REPLMessage.SOURCE_NAME, fileName); reply.put(REPLMessage.FILE_PATH, source.getPath()); - reply.put(REPLMessage.BREAKPOINT_ID, Integer.toString(replServer.getBreakpointID(breakpoint))); + reply.put(REPLMessage.BREAKPOINT_ID, Integer.toString(breakpointInfo.getID())); reply.put(REPLMessage.LINE_NUMBER, Integer.toString(lineNumber)); reply.put(REPLMessage.BREAKPOINT_IGNORE_COUNT, ignoreCount.toString()); return finishReplySucceeded(reply, "Breakpoint set"); @@ -239,19 +234,14 @@ if (source == null) { return finishReplyFailed(reply, fileName + " not found"); } - Integer lineNumber = request.getIntValue(REPLMessage.LINE_NUMBER); + final Integer lineNumber = request.getIntValue(REPLMessage.LINE_NUMBER); if (lineNumber == null) { return finishReplyFailed(reply, "missing line number"); } - Breakpoint breakpoint; - try { - breakpoint = replServer.setLineBreakpoint(DEFAULT_IGNORE_COUNT, source.createLineLocation(lineNumber), true); - } catch (IOException ex) { - return finishReplyFailed(reply, ex); - } + final BreakpointInfo breakpointInfo = replServer.setLineBreakpoint(DEFAULT_IGNORE_COUNT, source.createLineLocation(lineNumber), true); reply.put(REPLMessage.SOURCE_NAME, fileName); reply.put(REPLMessage.FILE_PATH, source.getPath()); - reply.put(REPLMessage.BREAKPOINT_ID, Integer.toString(replServer.getBreakpointID(breakpoint))); + reply.put(REPLMessage.BREAKPOINT_ID, Integer.toString(breakpointInfo.getID())); reply.put(REPLMessage.LINE_NUMBER, Integer.toString(lineNumber)); return finishReplySucceeded(reply, "One-shot line breakpoint set"); } @@ -291,8 +281,8 @@ public REPLMessage[] receive(REPLMessage request, REPLServer replServer) { final REPLMessage reply = createReply(); final ArrayList<REPLMessage> infoMessages = new ArrayList<>(); - for (Breakpoint breakpoint : replServer.getBreakpoints()) { - infoMessages.add(createBreakpointInfoMessage(breakpoint, replServer)); + for (BreakpointInfo breakpointInfo : replServer.getBreakpoints()) { + infoMessages.add(createBreakpointInfoMessage(breakpointInfo)); } if (infoMessages.size() > 0) { return infoMessages.toArray(new REPLMessage[0]); @@ -338,15 +328,15 @@ @Override public REPLMessage[] receive(REPLMessage request, REPLServer replServer) { final REPLMessage reply = createReply(); - Integer breakpointNumber = request.getIntValue(REPLMessage.BREAKPOINT_ID); + final Integer breakpointNumber = request.getIntValue(REPLMessage.BREAKPOINT_ID); if (breakpointNumber == null) { return finishReplyFailed(reply, "missing breakpoint number"); } - final Breakpoint breakpoint = replServer.findBreakpoint(breakpointNumber); - if (breakpoint == null) { + final BreakpointInfo breakpointInfo = replServer.findBreakpoint(breakpointNumber); + if (breakpointInfo == null) { return finishReplyFailed(reply, "no breakpoint number " + breakpointNumber); } - replServer.clearBreakpoint(breakpoint); + breakpointInfo.dispose(); reply.put(REPLMessage.BREAKPOINT_ID, Integer.toString(breakpointNumber)); return finishReplySucceeded(reply, "Breakpoint " + breakpointNumber + " cleared"); } @@ -367,12 +357,12 @@ @Override public REPLMessage[] receive(REPLMessage request, REPLServer replServer) { final REPLMessage reply = createReply(); - final Collection<Breakpoint> breakpoints = replServer.getBreakpoints(); + final Collection<BreakpointInfo> breakpoints = replServer.getBreakpoints(); if (breakpoints.isEmpty()) { return finishReplyFailed(reply, "no breakpoints to delete"); } - for (Breakpoint breakpoint : breakpoints) { - breakpoint.dispose(); + for (BreakpointInfo breakpointInfo : breakpoints) { + breakpointInfo.dispose(); } return finishReplySucceeded(reply, Integer.toString(breakpoints.size()) + " breakpoints deleted"); } @@ -387,11 +377,11 @@ if (breakpointNumber == null) { return finishReplyFailed(reply, "missing breakpoint number"); } - final Breakpoint breakpoint = replServer.findBreakpoint(breakpointNumber); - if (breakpoint == null) { + final BreakpointInfo breakpointInfo = replServer.findBreakpoint(breakpointNumber); + if (breakpointInfo == null) { return finishReplyFailed(reply, "no breakpoint number " + breakpointNumber); } - breakpoint.setEnabled(false); + breakpointInfo.setEnabled(false); reply.put(REPLMessage.BREAKPOINT_ID, Integer.toString(breakpointNumber)); return finishReplySucceeded(reply, "Breakpoint " + breakpointNumber + " disabled"); } @@ -406,11 +396,11 @@ if (breakpointNumber == null) { return finishReplyFailed(reply, "missing breakpoint number"); } - final Breakpoint breakpoint = replServer.findBreakpoint(breakpointNumber); - if (breakpoint == null) { + final BreakpointInfo breakpointInfo = replServer.findBreakpoint(breakpointNumber); + if (breakpointInfo == null) { return finishReplyFailed(reply, "no breakpoint number " + breakpointNumber); } - breakpoint.setEnabled(true); + breakpointInfo.setEnabled(true); reply.put(REPLMessage.BREAKPOINT_ID, Integer.toString(breakpointNumber)); return finishReplySucceeded(reply, "Breakpoint " + breakpointNumber + " enabled"); } @@ -628,8 +618,8 @@ return finishReplyFailed(message, "missing breakpoint number"); } message.put(REPLMessage.BREAKPOINT_ID, Integer.toString(breakpointNumber)); - final Breakpoint breakpoint = replServer.findBreakpoint(breakpointNumber); - if (breakpoint == null) { + final BreakpointInfo breakpointInfo = replServer.findBreakpoint(breakpointNumber); + if (breakpointInfo == null) { return finishReplyFailed(message, "no breakpoint number " + breakpointNumber); } final String expr = request.get(REPLMessage.BREAKPOINT_CONDITION); @@ -637,7 +627,7 @@ return finishReplyFailed(message, "missing condition for " + breakpointNumber); } try { - breakpoint.setCondition(expr); + breakpointInfo.setCondition(expr); } catch (IOException ex) { return finishReplyFailed(message, "invalid condition for " + breakpointNumber); } catch (UnsupportedOperationException ex) { @@ -735,12 +725,12 @@ return finishReplyFailed(message, "missing breakpoint number"); } message.put(REPLMessage.BREAKPOINT_ID, Integer.toString(breakpointNumber)); - final Breakpoint breakpoint = replServer.findBreakpoint(breakpointNumber); - if (breakpoint == null) { + final BreakpointInfo breakpointInfo = replServer.findBreakpoint(breakpointNumber); + if (breakpointInfo == null) { return finishReplyFailed(message, "no breakpoint number " + breakpointNumber); } try { - breakpoint.setCondition(null); + breakpointInfo.setCondition(null); } catch (Exception ex) { return finishReplyFailed(message, ex); }
--- a/truffle/com.oracle.truffle.tools.debug.shell/src/com/oracle/truffle/tools/debug/shell/server/REPLServer.java Mon Dec 07 18:19:25 2015 -0800 +++ b/truffle/com.oracle.truffle.tools.debug.shell/src/com/oracle/truffle/tools/debug/shell/server/REPLServer.java Mon Dec 07 18:21:03 2015 -0800 @@ -36,12 +36,11 @@ import java.util.WeakHashMap; import com.oracle.truffle.api.debug.Breakpoint; +import com.oracle.truffle.api.debug.Breakpoint.State; import com.oracle.truffle.api.debug.Debugger; import com.oracle.truffle.api.debug.ExecutionEvent; import com.oracle.truffle.api.debug.FrameDebugDescription; -import com.oracle.truffle.api.debug.LineBreakpoint; import com.oracle.truffle.api.debug.SuspendedEvent; -import com.oracle.truffle.api.debug.TagBreakpoint; import com.oracle.truffle.api.frame.MaterializedFrame; import com.oracle.truffle.api.instrument.StandardSyntaxTag; import com.oracle.truffle.api.instrument.SyntaxTag; @@ -64,6 +63,13 @@ */ public final class REPLServer { + enum BreakpointKind { + LINE, + TAG + } + + private static int nextBreakpointUID = 0; + // Language-agnostic private final PolyglotEngine engine; private Debugger db; @@ -87,19 +93,7 @@ private PolyglotEngine.Language defaultLanguage; private final Visualizer visualizer; - private int nextBreakpointUID = 0; - - /** - * Map: breakpoints => breakpoint UID. - * <ul> - * <li>Contains only pending breakpoints when the Debugger not yet available.</li> - * <li>When the Debugger becomes available, each pending breakpoint is replaced with a Debugger - * breakpoint, keeping same UID.</li> - * <li>Contains no disposed breakpoints.</li> - * <li>UIDs for disposed breakpoints are never reused.</li> - * </ul> - */ - private Map<Breakpoint, Integer> breakpoints = new WeakHashMap<>(); + private Map<Integer, BreakpointInfo> breakpoints = new WeakHashMap<>(); public REPLServer(String defaultMIMEType, Visualizer visualizer) { this.visualizer = visualizer == null ? new DefaultVisualizer() : visualizer; @@ -135,27 +129,8 @@ protected void on(ExecutionEvent event) { if (db == null) { db = event.getDebugger(); - if (!breakpoints.isEmpty()) { - ArrayList<? extends Breakpoint> pendingBreakpoints = new ArrayList<>(breakpoints.keySet()); - try { - for (Breakpoint pending : pendingBreakpoints) { - - Integer uid = breakpoints.get(pending); - pending.dispose(); - Breakpoint replacement = null; - if (pending instanceof PendingLineBreakpoint) { - final PendingLineBreakpoint lineBreak = (PendingLineBreakpoint) pending; - replacement = db.setLineBreakpoint(lineBreak.getIgnoreCount(), lineBreak.getLineLocation(), lineBreak.isOneShot()); - - } else if (pending instanceof PendingTagBreakpoint) { - final PendingTagBreakpoint tagBreak = (PendingTagBreakpoint) pending; - replacement = db.setTagBreakpoint(tagBreak.getIgnoreCount(), tagBreak.getTag(), tagBreak.isOneShot()); - } - breakpoints.put(replacement, uid); - } - } catch (IOException e) { - throw new IllegalStateException("pending breakpoints should all be valid"); - } + for (BreakpointInfo breakpointInfo : breakpoints.values()) { + breakpointInfo.activate(db); } } if (currentServerContext.steppingInto) { @@ -500,168 +475,190 @@ return language.getMimeTypes().iterator().next(); } - Breakpoint setLineBreakpoint(int ignoreCount, LineLocation lineLocation, boolean oneShot) throws IOException { - Breakpoint breakpoint; - if (db == null) { - breakpoint = new PendingLineBreakpoint(ignoreCount, lineLocation, oneShot); - } else { - breakpoint = db.setLineBreakpoint(ignoreCount, lineLocation, oneShot); - } - registerNewBreakpoint(breakpoint); - return breakpoint; + BreakpointInfo setLineBreakpoint(int ignoreCount, LineLocation lineLocation, boolean oneShot) { + return new BreakpointInfo(db, lineLocation, ignoreCount, oneShot); } - Breakpoint setTagBreakpoint(int ignoreCount, StandardSyntaxTag tag, boolean oneShot) throws IOException { - Breakpoint breakpoint; - if (db == null) { - breakpoint = new PendingTagBreakpoint(ignoreCount, tag, oneShot); - } else { - breakpoint = db.setTagBreakpoint(ignoreCount, tag, oneShot); - } - registerNewBreakpoint(breakpoint); - return breakpoint; + BreakpointInfo setTagBreakpoint(int ignoreCount, StandardSyntaxTag tag, boolean oneShot) { + return new BreakpointInfo(db, tag, ignoreCount, oneShot); } - private synchronized void registerNewBreakpoint(Breakpoint breakpoint) { - breakpoints.put(breakpoint, nextBreakpointUID++); - } - - synchronized Breakpoint findBreakpoint(int id) { - for (Map.Entry<Breakpoint, Integer> entrySet : breakpoints.entrySet()) { - if (id == entrySet.getValue()) { - return entrySet.getKey(); - } - } - return null; + synchronized BreakpointInfo findBreakpoint(int id) { + return breakpoints.get(id); } /** * Gets a list of the currently existing breakpoints. */ - Collection<Breakpoint> getBreakpoints() { - return new ArrayList<>(breakpoints.keySet()); - } - - synchronized int getBreakpointID(Breakpoint breakpoint) { - final Integer id = breakpoints.get(breakpoint); - return id == null ? -1 : id; + Collection<BreakpointInfo> getBreakpoints() { + return new ArrayList<>(breakpoints.values()); } - void clearBreakpoint(Breakpoint breakpoint) { - breakpoint.dispose(); - breakpoints.remove(breakpoint); - } + final class BreakpointInfo { + + private final BreakpointKind kind; + + /** Null before created in debugger or after disposal. */ + private Breakpoint breakpoint; - /** - * The intention to create a line breakpoint. - */ - private final class PendingLineBreakpoint extends LineBreakpoint { + /** Non-null only when breakpoint == null. */ + private State state; // non-null iff haven't "activated" yet + + private final int uid; + + private boolean oneShot; + + private int ignoreCount; private Source conditionSource; - PendingLineBreakpoint(int ignoreCount, LineLocation lineLocation, boolean oneShot) { - super(Breakpoint.State.ENABLED_UNRESOLVED, lineLocation, ignoreCount, oneShot); + private final LineLocation lineLocation; + + private final SyntaxTag tag; + + private BreakpointInfo(Debugger debugger, LineLocation lineLocation, int ignoreCount, boolean oneShot) { + this.kind = BreakpointKind.LINE; + this.lineLocation = lineLocation; + this.tag = null; + this.ignoreCount = ignoreCount; + this.oneShot = oneShot; + this.uid = nextBreakpointUID++; + if (debugger == null) { + this.state = State.ENABLED_UNRESOLVED; + } else { + activate(debugger); + } + breakpoints.put(uid, this); } - @Override - public void setEnabled(boolean enabled) { - switch (getState()) { - case ENABLED_UNRESOLVED: - if (!enabled) { - setState(State.DISABLED_UNRESOLVED); - } - break; - case DISABLED_UNRESOLVED: - if (enabled) { - setState(State.ENABLED_UNRESOLVED); - } - break; - case DISPOSED: - throw new IllegalStateException("Disposed breakpoints must stay disposed"); - default: - throw new IllegalStateException("Unexpected breakpoint state"); + private BreakpointInfo(Debugger debugger, SyntaxTag tag, int ignoreCount, boolean oneShot) { + this.kind = BreakpointKind.TAG; + this.lineLocation = null; + this.tag = tag; + this.ignoreCount = ignoreCount; + this.oneShot = oneShot; + this.uid = nextBreakpointUID++; + if (debugger == null) { + this.state = State.ENABLED_UNRESOLVED; + } else { + activate(debugger); + } + breakpoints.put(uid, this); + } + + private void activate(Debugger debugger) { + if (breakpoint != null) { + throw new IllegalStateException("Breakpoint already activated"); + } + if (state == State.DISPOSED) { + throw new IllegalStateException("Breakpoint already disposed"); + } + try { + switch (kind) { + case LINE: + breakpoint = debugger.setLineBreakpoint(ignoreCount, lineLocation, oneShot); + break; + case TAG: + breakpoint = debugger.setTagBreakpoint(ignoreCount, tag, oneShot); + break; + default: + throw new IllegalStateException("Unexpected breakpoint kind"); + } + if (conditionSource != null) { + breakpoint.setCondition(conditionSource.getCode()); + conditionSource = null; + } + if (state == State.DISABLED_UNRESOLVED) { + breakpoint.setEnabled(false); + } + state = null; + } catch (IOException ex) { + throw new IllegalStateException("Failure to activate breakpoint " + uid + ": " + ex.getMessage()); } } - @Override - public boolean isEnabled() { - return getState() == State.ENABLED_UNRESOLVED; + int getID() { + return uid; } - @Override - public void setCondition(String expr) throws IOException { - - this.conditionSource = expr == null ? null : Source.fromText(expr, "breakpoint condition from text: " + expr); - } - - @Override - public Source getCondition() { - return conditionSource; + String describeState() { + return (breakpoint == null ? state : breakpoint.getState()).getName(); } - @Override - public void dispose() { - if (getState() == State.DISPOSED) { - throw new IllegalStateException("Breakpoint already disposed"); + String describeLocation() { + if (breakpoint == null) { + switch (kind) { + case LINE: + return "Line: " + lineLocation.getShortDescription(); + case TAG: + return "Tag " + tag.name(); + default: + throw new IllegalStateException("Unexpected breakpoint state"); + } } - setState(State.DISPOSED); - breakpoints.remove(this); - } - } - - /** - * The intention to create a line breakpoint. - */ - private final class PendingTagBreakpoint extends TagBreakpoint { - - private Source conditionSource; - - PendingTagBreakpoint(int ignoreCount, SyntaxTag tag, boolean oneShot) { - super(Breakpoint.State.ENABLED_UNRESOLVED, tag, ignoreCount, oneShot); + return breakpoint.getLocationDescription(); } - @Override - public void setEnabled(boolean enabled) { - switch (getState()) { - case ENABLED_UNRESOLVED: - if (!enabled) { - setState(State.DISABLED_UNRESOLVED); - } - break; - case DISABLED_UNRESOLVED: - if (enabled) { - setState(State.ENABLED_UNRESOLVED); - } - break; - case DISPOSED: - throw new IllegalStateException("Disposed breakpoints must stay disposed"); - default: - throw new IllegalStateException("Unexpected breakpoint state"); + void setEnabled(boolean enabled) { + if (breakpoint == null) { + switch (state) { + case ENABLED_UNRESOLVED: + if (!enabled) { + state = State.DISABLED_UNRESOLVED; + } + break; + case DISABLED_UNRESOLVED: + if (enabled) { + state = State.ENABLED_UNRESOLVED; + } + break; + case DISPOSED: + throw new IllegalStateException("Disposed breakpoints must stay disposed"); + default: + throw new IllegalStateException("Unexpected breakpoint state"); + } + } else { + breakpoint.setEnabled(enabled); } } - @Override - public boolean isEnabled() { - return getState() == State.ENABLED_UNRESOLVED; + boolean isEnabled() { + return breakpoint == null ? (state == State.ENABLED_UNRESOLVED) : breakpoint.isEnabled(); } - @Override - public void setCondition(String expr) throws IOException { - this.conditionSource = Source.fromText(expr, "breakpoint condition from text: " + expr); + void setCondition(String expr) throws IOException { + if (breakpoint == null) { + conditionSource = expr == null ? null : Source.fromText(expr, "breakpoint condition from text: " + expr); + } else { + breakpoint.setCondition(expr); + } + } + + String getCondition() { + final Source source = breakpoint == null ? conditionSource : breakpoint.getCondition(); + return source == null ? null : source.getCode(); } - @Override - public Source getCondition() { - return conditionSource; + int getIgnoreCount() { + return breakpoint == null ? ignoreCount : breakpoint.getIgnoreCount(); + } + + int getHitCount() { + return breakpoint == null ? 0 : breakpoint.getHitCount(); } - @Override - public void dispose() { - if (getState() == State.DISPOSED) { - throw new IllegalStateException("Breakpoint already disposed"); + void dispose() { + if (breakpoint == null) { + if (state == State.DISPOSED) { + throw new IllegalStateException("Breakpoint already disposed"); + } + } else { + breakpoint.dispose(); + breakpoint = null; } - setState(State.DISPOSED); - breakpoints.remove(this); + state = State.DISPOSED; + breakpoints.remove(uid); + conditionSource = null; } } }