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: spurious cycles in time.Now pprof output (vdsoPC and vdsoSP not WAI?) #47324

Closed
josharian opened this issue Jul 21, 2021 · 9 comments
Closed

Comments

@josharian
Copy link
Contributor

@josharian josharian commented Jul 21, 2021

On linux:

go test -bench=kNow$ -run=NONE -count=3 -cpuprofile=c.p time
go tool pprof -pdf -lines c.p > o.pdf

Result:

o.pdf

Observe the cycles in the graph. The actual call graph doesn't have any cycles in it.

@thanm thanm added this to the Backlog milestone Jul 23, 2021
@thanm
Copy link
Contributor

@thanm thanm commented Jul 23, 2021

@cherrymui per owners (I am assuming that this is a runtime/pprof problem).

Loading

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Jul 26, 2021

I assume this is Linux/ARM64? (from sys_linux_arm64.s in the profile)
What version of Go is this? I assume this is not tip, where walltime1 is gone.

It appears to me in sys_linux_arm64.s we probably store the wrong value into m.vdsoSP. I think we should store the caller's SP instead of its own SP.

I'll work on a fix. (But would still be good to know the information above. Thanks.)

Loading

@josharian
Copy link
Contributor Author

@josharian josharian commented Jul 26, 2021

Sorry, yes, linux/arm64. I’ve also seen it on linux/amd64, and it wouldn’t surprise me if it were also present on other platforms. The reproduction above was with 1.16, I believe. (I’m AFK.)

Loading

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Jul 26, 2021

Does CL https://go-review.googlesource.com/c/go/+/337590 help? (It's on tip.) Thanks.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 26, 2021

Change https://golang.org/cl/337590 mentions this issue: runtime: set m.vdsoSP correctly on Linux/ARM64

Loading

@josharian
Copy link
Contributor Author

@josharian josharian commented Jul 27, 2021

Does CL https://go-review.googlesource.com/c/go/+/337590 help? (It's on tip.) Thanks.

Sure does!

Loading

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Jul 27, 2021

Thanks. I'll make it a real CL then.

Loading

@mengzhuo
Copy link
Contributor

@mengzhuo mengzhuo commented Jul 28, 2021

@cherrymui should we fix other architectures in CL 337590 or submit separate CLs for this issue?

Loading

@cherrymui
Copy link
Contributor

@cherrymui cherrymui commented Jul 28, 2021

I'll probably do all of them in one CL.

Loading

@gopherbot gopherbot closed this in 217507e Sep 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants