Skip to content
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.

Sign up for GitHub

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

Jump to bottom

service/dap: expand goroutine info in threads response #2233

Merged
merged 3 commits into from
Nov 30, 2020
Merged

service/dap: expand goroutine info in threads response #2233

merged 3 commits into from
Nov 30, 2020

Conversation

polinasok
Copy link
Collaborator

@polinasok polinasok commented Nov 24, 2020 •
edited
Loading

Adding goroutine id, thread id and selected marker to the thread description akin to what is shared with the "goroutines" command.
E.g.

* Goroutine 1 - Start: /usr/local/go/src/runtime/proc.go:113 runtime.main (0x1036080) (thread 1969871)
  Goroutine 2 - Start: /usr/local/go/src/runtime/proc.go:245 runtime.forcegchelper (0x10363f0)
  Goroutine 3 - Start: /usr/local/go/src/runtime/mgcsweep.go:64 runtime.bgsweep (0x10231a0)
  Goroutine 4 - Start: /usr/local/go/src/runtime/mgcscavenge.go:225 runtime.bgscavenge (0x1021710)
  Goroutine 5 - Start: /usr/local/go/src/runtime/mgc.go:1844 runtime.gcBgMarkWorker (0x101c670)
  Goroutine 17 - Start: /usr/local/go/src/runtime/mfinal.go:161 runtime.runfinq (0x1019080)

Screen Shot 2020-11-23 at 9 31 26 PM

The selected goroutine should be highlighted on stop in the UI, but as people click around, expanding the goroutines and viewing their stacks, they might loose track of which one was initially highlighted/expanded, so I think an explicit marker is useful.

Updates #1515

aarzilli
aarzilli reviewed Nov 24, 2020
View reviewed changes
Copy link
Member

@aarzilli aarzilli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See test failure. Aside from that LGTM.

aarzilli
aarzilli approved these changes Nov 25, 2020
View reviewed changes
Copy link
Member

@aarzilli aarzilli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@derekparker derekparker merged commit 98cba03 into go-delve:master Nov 30, 2020
abner-chenc pushed a commit to loongson/delve that referenced this pull request Mar 1, 2024
@polinasok @zhaixiaojuan
service/dap: expand goroutine info in threads response (go-delve#2233)
8110423 
* service/dap: expand goroutine info in threads response

* Fix checks

* Make DeepSource happy

Co-authored-by: Polina Sokolova <polinasok@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aarzilli aarzilli aarzilli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@polinasok @aarzilli @derekparker