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

cmd/trace/v2: goroutine analysis page doesn't identify goroutines consistently [1.22 backport] #65577

Closed
gopherbot opened this issue Feb 7, 2024 · 3 comments
Closed

cmd/trace/v2: goroutine analysis page doesn't identify goroutines consistently [1.22 backport] #65577

gopherbot opened this issue Feb 7, 2024 · 3 comments
Labels
CherryPickApproved Used during the release process for point releases
Milestone
Go1.22.1

Comments

@gopherbot
Copy link
Contributor

@mknyszek requested issue #65574 to be considered for backport to the next 1.22 minor release.

@gopherbot Please open backport issues for Go 1.21 and Go 1.22. This makes the cmd/trace UI harder to interpret with no easy workaround. The fix is very safe because it only touches debug tooling, specifically go tool trace, not any Go code someone would run in production.
@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Feb 7, 2024
@gopherbot gopherbot mentioned this issue Feb 7, 2024
Closed
@gopherbot gopherbot added this to the Go1.22.1 milestone Feb 7, 2024
@dmitshur dmitshur added CherryPickApproved Used during the release process for point releases and removed CherryPickCandidate Used during the release process for point releases labels Feb 7, 2024
@mknyszek
Copy link
Contributor

mknyszek commented Feb 8, 2024

This backport only needs to happen for Go 1.22. The relevant codepaths don't exist in Go 1.21. The corresponding 1.21 issue was closed as not planned.

@gopherbot
Copy link
Contributor Author

Change https://go.dev/cl/562558 mentions this issue: [release-branch.go1.22] cmd/trace/v2,internal/trace: use correct frame for identifying goroutines

gopherbot pushed a commit that referenced this issue Feb 27, 2024
@nsrip-dd @cagedmantis
[release-branch.go1.22] cmd/trace/v2,internal/trace: use correct fram…
c33adf4 
…e for identifying goroutines

To determine the identity of a goroutine for displaying in the trace UI,
we should use the root frame from a call stack. This will be the
starting function for the goroutine and is the same for each call stack
from a given goroutine. The new tracer no longer includes starting PCs
for goroutines which existed at the start of tracing, so we can't use a
PC for grouping together goroutines any more. Instead, we just use the
name of the entry function for grouping.

For #65574
Fixes #65577

Change-Id: I5324653316f1acf0ab90c30680f181060ea45dd7
Reviewed-on: https://go-review.googlesource.com/c/go/+/562455
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit a517131)
Reviewed-on: https://go-review.googlesource.com/c/go/+/562558
Reviewed-by: Michael Pratt <mpratt@google.com>
@gopherbot
Copy link
Contributor Author

Closed by merging c33adf4 to release-branch.go1.22.

bradfitz pushed a commit to tailscale/go that referenced this issue Mar 5, 2024
@nsrip-dd @bradfitz
[release-branch.go1.22] cmd/trace/v2,internal/trace: use correct fram…
24e8aed 
…e for identifying goroutines

To determine the identity of a goroutine for displaying in the trace UI,
we should use the root frame from a call stack. This will be the
starting function for the goroutine and is the same for each call stack
from a given goroutine. The new tracer no longer includes starting PCs
for goroutines which existed at the start of tracing, so we can't use a
PC for grouping together goroutines any more. Instead, we just use the
name of the entry function for grouping.

For golang#65574
Fixes golang#65577

Change-Id: I5324653316f1acf0ab90c30680f181060ea45dd7
Reviewed-on: https://go-review.googlesource.com/c/go/+/562455
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit a517131)
Reviewed-on: https://go-review.googlesource.com/c/go/+/562558
Reviewed-by: Michael Pratt <mpratt@google.com>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Mar 6, 2024
@nsrip-dd @romaindoumenc
[release-branch.go1.22] cmd/trace/v2,internal/trace: use correct fram…
9534805 
…e for identifying goroutines

To determine the identity of a goroutine for displaying in the trace UI,
we should use the root frame from a call stack. This will be the
starting function for the goroutine and is the same for each call stack
from a given goroutine. The new tracer no longer includes starting PCs
for goroutines which existed at the start of tracing, so we can't use a
PC for grouping together goroutines any more. Instead, we just use the
name of the entry function for grouping.

For golang#65574
Fixes golang#65577

Change-Id: I5324653316f1acf0ab90c30680f181060ea45dd7
Reviewed-on: https://go-review.googlesource.com/c/go/+/562455
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: David Chase <drchase@google.com>
(cherry picked from commit a517131)
Reviewed-on: https://go-review.googlesource.com/c/go/+/562558
Reviewed-by: Michael Pratt <mpratt@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases
Projects
None yet
Milestone
Go1.22.1
Development

No branches or pull requests
3 participants
@mknyszek @dmitshur @gopherbot