Refactor Handling of Stepping #153
Conversation
smarr
added
the
enhancement
smarr
force-pushed
the
refactor-stepping-strategy
branch
from
fd916a2
to
6d9d74b
Compare
|
|
||
| @SerializedName("stepToPromiseResolver") | ||
| STEP_TO_PROMISE_RESOLVER("stepToPromiseResolver", "Step to Promise Resolver", | ||
| Group.ACTOR_STEPPING, "msg-white", |
There was a problem hiding this comment.
I should probably find more intuitive icons for each stepping type, they have to be different for each command.
| } | ||
| }, | ||
|
|
||
| @SerializedName("stepToMessageRcvr") | ||
| STEP_TO_RECEIVER_MESSAGE("stepToMessageRcvr", "Step to Msg Receiver", Group.ACTOR_STEPPING, "msg-open", new Class[] {EventualMessageSend.class}) { | ||
| @Override public void process(final Suspension susp) { | ||
| STEP_TO_MESSAGE_RECEIVER("stepToMessageRcvr", "Step to Msg Receiver", |
There was a problem hiding this comment.
thanks for renaming this command, I realize I had a typo there
| activity.setStepToJoin(true); | ||
| } else if (activityThread.isStepping(SteppingType.STEP_TO_NEXT_TURN)) { | ||
| activity.setStepToNextTurn(true); | ||
| } else if (activityThread.isStepping(SteppingType.RETURN_FROM_TURN_TO_PROMISE_RESOLUTION)) { |
There was a problem hiding this comment.
I think I understand what you mean here, is similar to the step-to-next-turn command in the sense that should be able to trigger from anywhere in the code. And I only have it implemented for the SendNode and the primitives of PromisePrims
| @@ -100,11 +109,8 @@ public synchronized void addOrUpdateBeforeExpression(final SectionBreakpoint bId | |||
|
|
|||
| public synchronized void addOrUpdateAfterExpression(final SectionBreakpoint bId) { | |||
| // TODO: does this work??? | |||
There was a problem hiding this comment.
what happens here? I saw that is used for transactions and fork/join but don't we have tests for them?
There was a problem hiding this comment.
That's an old comment, I think it mostly works.
But no, it isn't tested.
| this.breakpoint = breakpoint; | ||
| } | ||
|
|
||
| @Specialization(assumptions = "bpUnchanged", guards = "breakpoint.isDisabled()") | ||
| public final boolean breakpointDisabled( | ||
| @Cached("breakpoint.getAssumption()") final Assumption bpUnchanged) { | ||
| return false; | ||
| return ActivityThread.isSteppingType(breakpoint.getSteppingType()); |
There was a problem hiding this comment.
then the disabled state of the breakpoint will depend on the stepping. I think is a good approach. It fits better in this location than what I implemented, I think.
|
The whole refactoring hinges on the realization that there is a mapping between breakpoints and stepping operations. It is important to realize however that there is connection between breakpoints and stepping targets. Sometimes stepping operations also don't use the breakpoints for what should really be the same source location. |
Signed-off-by: Stefan Marr <git@stefan-marr.de>
- encode relevant SteppingType in BreakpointEnabling - this approach ensures that there is no overhead when we don’t have any breakpoints in the system Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
This is identical to how step-to-next-turn is handled. Signed-off-by: Stefan Marr <git@stefan-marr.de>
These methods don’t serve any obvious purpose, they are already implemented by the breakpoint implementation. Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
smarr
force-pushed
the
refactor-stepping-strategy
branch
from
6d9d74b
to
f623e3d
Compare
This manages all non-truffle breakpoints in a single hashmap. This removes a lot of trivially duplicated code. - standard breakpoint type now creates breakpoint implicitly - remove generic parameter from BreakpointNode, not needed Signed-off-by: Stefan Marr <git@stefan-marr.de>
- rename execute method to `executeShouldHalt()` - remove unnecessary `isDisabled()` - adapt class comments Signed-off-by: Stefan Marr <git@stefan-marr.de>
Signed-off-by: Stefan Marr <git@stefan-marr.de>
smarr
force-pushed
the
refactor-stepping-strategy
branch
from
efee94e
to
b317ca5
Compare
The goal of this change is to simplify the implementation of stepping and unify it with breakpoints where possible.
Thus, in most cases, a potential breakpoint is also a stepping target.
This means, a
BreakpointNodeshould be able to do the test whether a relevant stepping strategy is activated.This change also removes the boilerplate code from
SteppingStrategy. There doesn't seem to be a need for this complexity. A simple check for theSteppingTypeseems sufficient.This PR also adds the missing step-to-promise-resolver stepping.