-
Notifications
You must be signed in to change notification settings - Fork 617
dap: add stack traces with next and continue functionality #3279
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
1da2de0 to
611fbf0
Compare
ktock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM.
| } | ||
|
|
||
| func (p *idPool) Put(x int64) { | ||
| // noop |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: It's unlikely happen but it's safe to return an error or panic if the id is exhausted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was mostly just adding this struct in case we ever wanted to reuse ids for some reason. That's the main reason why this function exists. I can also just remove this entirely and we can add that later if it's ever used.
It is now possible to send next and continue as separate signals. When executing a build, the debug adapter will divide the LLB graph into regions. Each region corresponds to an uninterrupted chain of instructions. It will also record which regions depend on which other ones. This determines the execution order and it also determines what the stack traces look like. When continue is used, we will attempt to evaluate the last leaf node (the head). If we push next, we will determine which digest would be the next one to be processed. In the future, this will be used to also support breakpoints. Signed-off-by: Jonathan A. Sternberg <[email protected]>
ktock
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
|
|
||
| for _, src := range t.def.Source.Infos { | ||
| fname := filepath.Join(t.sourcePath, src.Filename) | ||
| t.sourceMap.Put(ctx, fname, src.Data) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might need to check for duplicates here in the future as Filename is not guaranteed to be unique.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Source map has some code in it that will deduplicate things. It checks the filename and the data and doesn't emit an event when the data doesn't change.
| } | ||
|
|
||
| def := t.def.ToPB() | ||
| def.Def[len(def.Def)-1] = dt |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks wasteful. We should only send the ops that actually participate in the build.
It is now possible to send next and continue as separate signals. When
executing a build, the debug adapter will divide the LLB graph into
regions. Each region corresponds to an uninterrupted chain of
instructions. It will also record which regions depend on which other
ones.
This determines the execution order and it also determines what the
stack traces look like.
When continue is used, we will attempt to evaluate the last leaf node
(the head). If we push next, we will determine which digest would be the
next one to be processed.
In the future, this will be used to also support breakpoints.