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.

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

cmd/trace: add a name/desc to goroutines which are inactive when get a pprof trace #49994

Closed
yifhao opened this issue Dec 6, 2021 · 7 comments
Closed

cmd/trace: add a name/desc to goroutines which are inactive when get a pprof trace #49994

yifhao opened this issue Dec 6, 2021 · 7 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Backlog

Comments

@yifhao
Copy link

yifhao commented Dec 6, 2021

What version of Go are you using (go version)?

$ go version
nothing to do with go version

Does this issue reproduce with the latest release?

yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
nothing to do with go env

What did you do?

I got a pprof trace, then watch it on browser. I saw the last line 'N=xxx' with no link or name.
it's easy to make a mistake about what it is meaning, many people may think it's the total number of goroutines.

What did you expect to see?

I read the source code, and then find: the last line is the num of goroutines which are created before trace taken, and inactive during the trace.

I think may add a name/desc to it, like: inactive when trace.
in httpGoroutines func add:
image
and then see:
image

What did you see instead?

image
@seankhliao seankhliao changed the title cmd/trace/goroutines.go: add a name/desc to goroutines which are inactive when get a pprof trace cmd/trace: add a name/desc to goroutines which are inactive when get a pprof trace Dec 6, 2021
@seankhliao
Copy link
Member

cc @mknyszek @prattmic

@seankhliao seankhliao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Dec 6, 2021
@yifhao
Copy link
Author

yifhao commented Dec 6, 2021 •
edited

cmd/trace/goroutines.go

var templGoroutines = template.Must(template.New("").Parse(`
<html>
<body>
Goroutines: <br>
{{range $}}
  <a href="/goroutine?id={{.ID}}">{{.Name}}</a> N={{.N}} <br>
{{end}}
</body>
</html>
`))

image
a href tag without text which means no action in tracing will display a line like this in browser .
image

@prattmic prattmic added this to the Go1.19 milestone Dec 6, 2021
@prattmic prattmic added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Dec 6, 2021
@prattmic
Copy link
Member

prattmic commented Dec 6, 2021

Thanks for the report. I think your suggested fix is reasonable, feel free to send a CL (note we are in freeze and thus won't submit before ~Feb).

Ideally we'd do even better and actually attach each goroutine's stack trace to the initial EvGoCreate. This should be possible since the world is stopped, but I could see that making starting tracing take a lot longer, especially with 5000+ goroutines, which I imagine is why it was avoided in the first place.

@prattmic
Copy link
Member

prattmic commented Dec 6, 2021 •
edited

Ideally we'd do even better and actually attach each goroutine's stack trace to the initial EvGoCreate. This should be possible since the world is stopped, but I could see that making starting tracing take a lot longer, especially with 5000+ goroutines, which I imagine is why it was avoided in the first place.

Actually, we already have gp.startpc with the initial goroutine entrypoint (in fact TraceStart even uses it as the "stack id" for EvGoCreate). If the cost is low enough, we could findfunc(gp.startpc) and include a func name string id along with the EvGoCreate event.

@ianlancetaylor
Copy link
Contributor

Moving to Backlog. Please comment if you disagree. Thanks.

@ianlancetaylor ianlancetaylor modified the milestones: Go1.19, Backlog Jun 24, 2022
@seankhliao seankhliao mentioned this issue Aug 13, 2022
Closed
@hitzhangjie hitzhangjie mentioned this issue Aug 21, 2022
Closed
@gopherbot
Copy link

Change https://go.dev/cl/425042 mentions this issue: cmd/trace: display goroutines (PC=0) with clearer description

gopherbot pushed a commit that referenced this issue Aug 25, 2022
@hitzhangjie @prattmic
cmd/trace: display goroutines (PC=0) with clearer description
04eb359 
This PR fixes: #54425 #49994

Change-Id: Id60a3ba6930f8e29b12b6d8f80945decd2ce31bc
GitHub-Last-Rev: 60a040a
GitHub-Pull-Request: #54575
Reviewed-on: https://go-review.googlesource.com/c/go/+/425042
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
@zikaeroh
Copy link
Contributor

The above CL had a malformed "fixes" line; this issue should be closed now that it's merged.

rajbarik pushed a commit to rajbarik/go that referenced this issue Sep 1, 2022
@hitzhangjie
cmd/trace: display goroutines (PC=0) with clearer description
d9deeea 
This PR fixes: golang#54425 golang#49994

Change-Id: Id60a3ba6930f8e29b12b6d8f80945decd2ce31bc
GitHub-Last-Rev: 60a040a
GitHub-Pull-Request: golang#54575
Reviewed-on: https://go-review.googlesource.com/c/go/+/425042
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
@golang golang locked and limited conversation to collaborators Aug 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
Release Status & Release Blockers
Status: Done
Milestone
Backlog
Development

No branches or pull requests
6 participants
@prattmic @ianlancetaylor @gopherbot @seankhliao @yifhao @zikaeroh