Alternative implementation for stepThrough using HaltingBlock

[carolahp]

This PR is a clean version of PR#17308
It implements step through using a special kind of blocks named halting blocks.

[StevenCostiou]

What is here is nice, I did not look the tests yet because it seems there are parts missing:

• if this is supposed to replace totally the current stepThrough, the stepThrough: method should be rewritten to, else it has to be modified depending on a setting, activating or not these changes
• HaltingBlocks are not used, there is a part where they are installed that is missing
• I wonder if we should use halt or break
• There are missing pragmas: for example in haltIfStepping, the halt will actually stop the execution on that method, which will defeat the purpose of helping user get faster in their block ;)

[StevenCostiou]

Note: the failing tests are just integration tests, easy to fix.

[carolahp]

Thanks a lot for the review @StevenCostiou , after our pair programming session I did the following:

• I have tested the fastStepThrough behavior in the debugger debugging the code below and it worked well :D

DebugSession fastStepThroughMode: true.
StepThroughTest new evalBlock: [ 1+ 1. ^ 2 ] afterLoop: 100

• I identified that the bottle neck in fastStepThrough is in Process>>completeStep:, in particular it's in Break(Halt)>>completeProcess: aProcess with: aContext. I just changed one line (this one) and now fastStepThrough is 29% faster than stepThrough (before changing this line, fastStepThrough was only about 13% faster). The image below shows my experiment:

• The line I changed allows contexts to use cache, and I thought this may be risky. I ran tests related to debugging before and after the change.
  • Before the change:
    • StepOverTest>>testStepOverReturnUnwindBlock threw an Error
    • DebugPointTest>>testTranscriptDebugPoint Failed.
  • After the change:
    • DebugPointTest>>testTranscriptDebugPoint Failed (same as before the change)
    • ObjectWithPrintingRaisingHaltTest>>testInspectingObjectWithPrintOnWithHaltOpenInspector threw an Error
    • StDebuggerTest>>testDynamicVariableEvaluation Failed

To prevent unrelated tests from failing, I could subclass Break and use the subclass in Process>>completeStep:. However this wouldn't fix the tests I mentioned above.
I tried debugging the previous tests, but I couldn't fix them.

Do you think this solution points in the right direction?

[StevenCostiou]

In Halt, I wonder if we could not just stop completing processes having the pragma, since they are hidden in the debugger?
Could you try benchmarking after doing this modification?

Halt >> completeProcess: aProcess with: aContext
	^super completeProcess: aProcess with: aContext

If it is really faster it would need further care in the debugger but I think there is no important impact besides potentially showing more suff in the debugger.

[carolahp]

In Halt, I wonder if we could not just stop completing processes having the pragma, since they are hidden in the debugger? Could you try benchmarking after doing this modification?

Halt >> completeProcess: aProcess with: aContext
	^super completeProcess: aProcess with: aContext

I tried it and the modification makes the FastStepThrough 33% faster than the original StepThrough.

If it is really faster it would need further care in the debugger but I think there is no important impact besides potentially showing more suff in the debugger.

Do you mean we would show irrelevant stack frames to the user in the debugger? If this is the case, is it feasible to hide this information from the final user?

[StevenCostiou]

33% instead of 29%?
hm not sure it's worth it.
Yes, the debugger normally already hides those stack frames.

[carolahp]

Yes, but 29% was reached after changing this line. And I'm afraid that change may be risky. Should we just keep it?

[carolahp]

Replaced by PR#18095