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

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

Closed
aclements opened this issue Sep 11, 2017 · 5 comments

Comments

Projects
None yet
4 participants
@aclements
Copy link
Member

commented Sep 11, 2017

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

This comment has been minimized.

Copy link

commented Sep 12, 2017

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

@gopherbot

This comment has been minimized.

Copy link

commented Sep 12, 2017

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

@gopherbot gopherbot closed this in 96b1eff Sep 12, 2017

gopherbot pushed a commit that referenced this issue Sep 12, 2017

[release-branch.go1.9] runtime: in cpuProfile.addExtra, set p.lostExt…
…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

This comment has been minimized.

Copy link
Contributor

commented Oct 13, 2017

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.)

@gopherbot

This comment has been minimized.

Copy link

commented Oct 15, 2017

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

[release-branch.go1.9] runtime: in cpuProfile.addExtra, set p.lostExt…
…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

This comment has been minimized.

Copy link
Contributor

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 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.
You can’t perform that action at this time.