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

cmd/compile/internal/pgo: Detect sample value position instead of hard-coding [1.20 backport] #58309

Closed
gopherbot opened this issue Feb 3, 2023 · 3 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Milestone

Comments

@gopherbot
Copy link

@prattmic requested issue #58292 to be considered for backport to the next 1.20 minor release.

@gopherbot please backport to 1.20. Profiles with only a single sample type can an ICE.

@gopherbot gopherbot added the CherryPickCandidate Used during the release process for point releases label Feb 3, 2023
@gopherbot gopherbot added this to the Go1.20.1 milestone Feb 3, 2023
@dr2chase dr2chase added the CherryPickApproved Used during the release process for point releases label Feb 8, 2023
@gopherbot gopherbot removed the CherryPickCandidate Used during the release process for point releases label Feb 8, 2023
@gopherbot
Copy link
Author

Change https://go.dev/cl/466438 mentions this issue: [release-branch.go1.20] cmd/compile/internal/pgo: fix hard-coded PGO sample data position

@gopherbot
Copy link
Author

Change https://go.dev/cl/467375 mentions this issue: [release-branch.go1.20] cmd/compile/internal/pgo: fix hard-coded PGO sample data position

@gopherbot
Copy link
Author

Closed by merging 3a04b6e to release-branch.go1.20.

gopherbot pushed a commit that referenced this issue Feb 10, 2023
…sample data position

This patch detects at which index position profiling samples that have
the value-type samples count are, instead of the previously hard-coded
position of index 1. Runtime generated profiles always generate CPU
profiling data with the 0 index being CPU nanoseconds, and samples count
at index 1, which is why this previously hasn't come up.

This is a redo of CL 465135, now allowing empty profiles. Note that
preprocessProfileGraph will already cause pgo.New to return nil for
empty profiles.

For #58292
For #58309

Change-Id: Ia6c94f0793f6ca9b0882b5e2c4d34f38e600c1e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/467375
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
romaindoumenc pushed a commit to TroutSoftware/go that referenced this issue Feb 14, 2023
…sample data position

This patch detects at which index position profiling samples that have
the value-type samples count are, instead of the previously hard-coded
position of index 1. Runtime generated profiles always generate CPU
profiling data with the 0 index being CPU nanoseconds, and samples count
at index 1, which is why this previously hasn't come up.

This is a redo of CL 465135, now allowing empty profiles. Note that
preprocessProfileGraph will already cause pgo.New to return nil for
empty profiles.

For golang#58292
For golang#58309

Change-Id: Ia6c94f0793f6ca9b0882b5e2c4d34f38e600c1e3
Reviewed-on: https://go-review.googlesource.com/c/go/+/467375
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
@golang golang locked and limited conversation to collaborators Feb 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge
Projects
None yet
Development

No branches or pull requests

2 participants