Skip to content

runtime: lost non-Go profiling samples are overcounted #21836

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
Closed
aclements opened this issue Sep 11, 2017 · 5 comments
Closed

runtime: lost non-Go profiling samples are overcounted #21836

aclements opened this issue Sep 11, 2017 · 5 comments
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Milestone
Go1.9.2

Comments

@aclements
Copy link
Member

cpuProfile.addExtra is called from the Go profiling signal handler to flush any accumulated non-Go profiling samples. If we lost any non-Go profiling samples, it flushes this as a special _LostExternalCode sample. However, it fails to reset the lost sample count, so every subsequent addExtra call will record another _LostExternalCode sample.

This fix should be as simple as setting p.lostExtra to 0, but this should get back-ported to 1.9.1.

/cc @ianlancetaylor @matloob
@aclements aclements added this to the Go1.9.1 milestone Sep 11, 2017
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/63270 mentions this issue: runtime: in cpuProfile.addExtra, set p.lostExtra to 0 after flush

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/63310 mentions this issue: runtime: in cpuProfile.addExtra, set p.lostExtra to 0 after flush

gopherbot pushed a commit that referenced this issue Sep 12, 2017
@matloob
[release-branch.go1.9] runtime: in cpuProfile.addExtra, set p.lostExt…
78a0a31 
…ra to 0 after flush

After the number of lost extra events are written to the the cpuprof log,
the number of lost extra events should be set to zero, or else, the next
time time addExtra is logged, lostExtra will be overcounted. This change
resets lostExtra after its value is written to the log.

Fixes #21836

Change-Id: I8a6ac9c61e579e7a5ca7bdb0f3463f8ae8b9f863
Reviewed-on: https://go-review.googlesource.com/63270
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
(cherry picked from commit 96b1eff)
Reviewed-on: https://go-review.googlesource.com/63310
Run-TryBot: Ian Lance Taylor <iant@golang.org>
@rsc rsc modified the milestones: Go1.9.1, Go1.9.2 Oct 4, 2017
@rsc rsc reopened this Oct 13, 2017
@rsc
Copy link
Contributor

rsc commented Oct 13, 2017 •
edited
Loading

CL 63270 OK for Go 1.9.2. (Gerrit confused by hard rollback.)
CL 70974 OK for Go 1.9.2. (Same CL but with Change-Id incremented.)

@rsc rsc added the CherryPickApproved Used during the release process for point releases label Oct 14, 2017
@rsc rsc mentioned this issue Oct 15, 2017
Closed
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/70974 mentions this issue: [release-branch.go1.9] runtime: in cpuProfile.addExtra, set p.lostExtra to 0 after flush

gopherbot pushed a commit that referenced this issue Oct 25, 2017
@matloob @rsc
[release-branch.go1.9] runtime: in cpuProfile.addExtra, set p.lostExt…
f36b126 
…ra to 0 after flush

After the number of lost extra events are written to the the cpuprof log,
the number of lost extra events should be set to zero, or else, the next
time time addExtra is logged, lostExtra will be overcounted. This change
resets lostExtra after its value is written to the log.

Fixes #21836

Change-Id: I8a6ac9c61e579e7a5ca7bdb0f3463f8ae8b9f864
Reviewed-on: https://go-review.googlesource.com/63270
Reviewed-by: Austin Clements <austin@google.com>
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-on: https://go-review.googlesource.com/70974
Run-TryBot: Russ Cox <rsc@golang.org>
@rsc
Copy link
Contributor

rsc commented Oct 26, 2017

go1.9.2 has been packaged and includes:

The release is posted at golang.org/dl.

— golang.org/x/build/cmd/releasebot, Oct 26 21:09:14 UTC

@rsc rsc closed this as completed Oct 26, 2017
@golang golang locked and limited conversation to collaborators Oct 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
CherryPickApproved Used during the release process for point releases FrozenDueToAge release-blocker
Projects
None yet
Milestone
Go1.9.2
Development

No branches or pull requests
4 participants
@rsc @aclements @ianlancetaylor @gopherbot