Nested breakpoints: break on instruction after another instruction breakpoint has triggered

[pausan]

VS Code already supports breakpoints on expressions or by hit count, ..., but sometimes it is not enough.

Proposal
I'd like to propose a complimentary approach to debug breakpoints, which as far as I know, has never been proposed before, nor I know of any debugger doing this nowadays, but I find the concept really useful myself.

The idea is to have a debugger that allows to break code after another breakpoint had been reached. The reason is that sometimes method X is called, and you know that method fails after gazillion calls to this method, but it is not easy to break by hit count, nor by an expression, plus, if you create a breakpoint with an expression condition a performance sensitive function you'll have to wait a long time because expression has to evaluate and that's not cheap, plus you have to find the right condition that makes it stop when you want, which is also not straightforward sometimes.

Background info
The thing is, you know that the problem arises only when going through certain code path (which usually is non-performance critical, but does not really matter), so nowadays, a quick-and-dirty approach to do what I propose, manually, would be to create a breakpoint on an instruction you want to break in... since that instruction gets called gazillion times, you look at the stack when the problem arises and put a breakpoint on a caller ancestor (not necessarily the caller) and then you would disable the breakpoint on the children method that gets called a gazillion times. Now you run the debugger again only with the 'caller' breakpoint enabled and the 'children' disabled. Once the 'caller' breaks, you enable the children breakpoints (sometimes you'd like to break at different points after the 'parent' broke) and run again on each of the children.

Current manual approach makes you enable/disable a bunch of breakpoints manually everytime you launch the app, which consumes time...

Yes, you can also investigate which expression or hit-count will make it trigger, althought not always is clear or deterministic. I believe this other method would make things simpler and quicker to debug on certain situations and it is complimentary to the existing approaches.

Implementation ideas:
Instead of a list of breakpoints, you can have a tree of breakpoints where breakpoints can be nested, and a nested breakpoint only triggers after the parent has been activated (in fact, the parent can break or not, depending if it is enabled or not... more on that later). One breakpoint could have 0 children, so it would behave as behaves right now, or N children, so all children would get activated after the parent triggered. In this case, it would be handy to be able to set as well if you want the parent to break or not. A possible solution would be to add an extra flag to set is as an "activator only breakpoint" or just, by convention, if parent is disabled, but has enabled children, the parent acts as an activator. In this case it would be good to have a context menu, by right-clicking on a parent, to disable or re-enable all children all at once. Probably having an extra flag, for not really stopping the debugger would be easier to keep all enabled/disabled children without losing the flags...

Extra considerations:
Using this new approach, I'm sure it will also be helpful to be able to have a breakpoint on the same instruction in the root of the tree or in anothe code paths, so effectively having several breakpoints on the same instructions on different "conditions". Said another way, it would be handy to support breakpoint on the same instruction triggering on different conditions.

Example:
Old approach: debugging session starts:

Breakpoints
  [x] whatever.py                   182  <-- non-critical
  [ ] whatever.py                   247  <-- critical, get called a lot
  [ ] whatever.py                   256  <-- critical, get called a lot
  [x] another.py                   618

Old approach: breaks on line 182 of whatever.py

Breakpoints
  [ ] whatever.py                   182  <-- non-critical
  [x] whatever.py                   247  <-- critical, get called a lot
  [x] whatever.py                   256  <-- critical, get called a lot
  [x] another.py                   618

Old approach: you continue debugging.

New approach would be
Case A:

Breakpoints
  [x] whatever.py                   182  <-- non-critical
      [x] whatever.py               247  <-- critical, get called a lot
      [x] whatever.py               256  <-- critical, get called a lot
  [x] another.py                   618

First it would stop on line 182 and only after that, it will stop on every call or effectively evaluating the conditions and breaking on lines 247 and 256.

Case B:

Breakpoints
  [ ] whatever.py                   182  <-- non-critical
      [x] whatever.py               247  <-- critical, get called a lot
      [x] whatever.py               256  <-- critical, get called a lot
  [x] another.py                   618

If parent is disabled, but children not, the only difference would be that it will never break into the debugger in line 182, but will act as a flag so if it breaks on lines 247 and 256 you know like 182 got executed (internally the debugger has to break, but I mean it won't have to give the control back to the user, and will still activate children if they are enabled, if children or descendants are not enabled, then there is no need to actually break even internally on the parent).

Feel free to use this concept as it is or improve/iterate on it.

If you reached this far, thanks a lot for reading 😁

[weinand]

Great idea!
And it could be done generically via the Debug Adapter Protocol.

Any chance that you could submit a PR for this?

[pausan]

I've just made an attempt over the weekend to compile VS Code on my machine and understand how everything is working.

I could kind of make sense of everything and even made a rough proof of concept by modifying:

Frontend/UI

• src/vs/workbench/contrib/debug/browser/breakpointsView.ts: Added drag&drop functionality to the list
• src/vs/workbench/contrib/debug/browser/media/debugViewlet.css: Added a couple of CSS indentations (to simulate a tree/children). You cannot show/hide children, they are always displayed.

App

• src/vs/workbench/contrib/debug/common/debug.ts: add parent/children to IBaseBreakpoint + children on IBreakpointUpdateData
• src/vs/workbench/contrib/debug/common/debugModel.ts: add several methods to Breakpoint to deal with parent/children, etc..
• src/vs/workbench/contrib/debug/common/debugSession.ts: updated "onDidStop" event to catch breakpoints in order to enable children breakpoints if the current bpx where it stopped has children, or normal flow if not.

I'm not entirely sure my modifications align with "Debug Adapter Protocol" you mentioned. To be honest I'm not familiarized with that protocol so I'm not sure the way I'm doing it aligns with the way you would expect it to be done.

The main idea of the approach I've opted for is to modify the UI and the underlying model/service so it stays agnostic to the rest of debugger servers/services and not requiring any changes to any protocol or any new functionality to be added elsewhere. I believe it can be implemented directly on VS Code and work as a layer on top/agnostic of any debugger service/server being used, so it would work straightaway with all debuggers (I hope).

Let me explain the approach for the UI and then for the internal model & debug service.

UI
I initially though on using a Tree to represent the new breakpoints, but that would have been tricky and costly because of all the changes that would have been required to keep existing things working, plus at the moment all breakpoints are visible and I think it would be useful to continue like that. So then I though, maybe a list would work, so I started experimenting with drag&drop list items and adding indentation to children. And I belive UI/UX feels pretty decent, although more work would have to be done to treat special cases like moving a child to the root level (right now I can move anything anywhere many times, but not to the root level anymore unless I remove the breakpoint, that is not good UX, but there is a work around it...but I don't like it. Another possibility would be to leave an empty space at the bottom and use it like a "root" marker, so if you move it there, it goes to the root again).
In the CSS I added some margin, depending on the number of ancestors, so that checkboxes+text are aligned and show as a children as expected.
I've been thinking on limiting the number of ancestors to 5 or 6.

Internal model/code
I've added parent breakpoint and a list of children breakpoints to IBreakpoint interface. Both can be undefined, if so, everything will behave like it behaves right now. When a BP is moved to another BP, I adjust the parent and children list accordingly (it behaves like a normal tree implementation).
To sort BPs and make them appear sorted in the UI I use a combination of URI + Line number of all ancestors concatenated together, that way I know they will be displayed in order by the UI and still work as it currently works if there are no nested BPs.

When the debugger session starts, all children breakpoints (all breakpoints that have a parent) will get disabled, then, when the debugger stops I find if it has stopped in a breakpoint that has children, and if so, all children breakpoints are re-enabled and the user can continue the session however he wants. I've also experimented by continuing the session automatically but I believe it feels more natural/user friendly if the debugger just stops as it is supposed to do. This kind of works, but you cannot keep some breakpoints enabled/disabled because all of them would get re-enabled again, which won't feel natural. So probably an extra flag would be needed and make that flag work well with the UI/service would be tricky.

Final words
Whereas my implementation kind of works, I've only focused my code on Breakpoints, not on Function Breakpoints, ... and there are several edge cases and situations that would require testing, more polishing and refactoring the code a little bit.

Overall, after having dedicated only one day on this and taking into account I hadn't compiled VS Code before, nor I was familiarized with its structure, classes, UI, ... I'm quite happy of the progress I've made. It proved to me that this approach can work, but my code is just a proof of concept full of hacks and it has many known bugs/know issues already.

Honestly, I'm not sure I'll be able to implement this feature on my own on my free time. One thing I learnt is that this will require more work and is more complex than I initially though.

Extra help would be appreciated (mainly because of my lack of free time). I would also appreciate thoughts on the approach I've taken as well, by somebody more experience in this code, maybe there are simpler ways of doing this.

PS: I have not pushed my code anywhere yet

[isidorn]

@pausan thanks for looking into this. However the changes needed for this are not so simple as you might have encountered.
The true change needed is that the breakpoints view no longer uses the list but starts to use a Tree. This would be a huge refactoring and for now we are not open to making this change.
Thus I will remove help-wanted and leave this open as a feature request so we might tackle it sometimes in the future.

I appologize if I did not communicate this before so you would not invest so much effor here.

[pausan]

@isidorn thanks for pointing it out. Haven't spent more time on it this week, but I think it is worth to share an image and a gif animation on what it looks like by using drag and drop on lists and 10 lines of CSS:

Here is a gif video to show how it would work in practice:

Again, there are some unsolved issues since once you add a breakpoint as a children you cannot move it to the root level, but that would be easily fixable by introducing a single line saying 'root level' or similar when drag&drop starts.

This is why I mentioned that even though there might be some hacks to make this work properly, I believe the final look & feel is kind of acceptable without requiring a major rewrite for creating tree breakpoints for the visual representation (internal structure would be a tree). It also preserves the existing look and feel when the developers are not using any nested breakpoints, so I believe it feels natural in that sense.

Let me know your thoughts.

[vscodebot]

This issue is being closed to keep the number of issues in our inbox on a manageable level, we are closing issues that are not going to be addressed in the foreseeable future: We look at the number of votes the issue has received and the number of duplicate issues filed. More details here. If you disagree and feel that this issue is crucial: We are happy to listen and to reconsider.

If you wonder what we are up to, please see our roadmap and issue reporting guidelines.

Thanks for your understanding and happy coding!