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/pprof: no API to access deep stacktraces with MemProfileRecord and BlockProfileRecord #67941

Open
korniltsev opened this issue Jun 12, 2024 · 10 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@korniltsev
Copy link
Contributor

Go version

go version devel go1.23-b788e91bad Tue Jun 11 18:08:44 2024 +0000 linux/amd64

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/korniltsev/.cache/go-build'
GOENV='/home/korniltsev/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/korniltsev/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/korniltsev/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/home/korniltsev/github/go'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/home/korniltsev/github/go/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.23-b788e91bad Tue Jun 11 18:08:44 2024 +0000'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/home/korniltsev/.config/go/telemetry'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/home/korniltsev/github/go/src/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build1768278824=/tmp/go-build -gno-record-gcc-switches'

What did you do?

I compare stacks from runtime.MemProfile and runtime/pprof.WriteHeapProfile

https://go.dev/play/p/7iUI7DdMI3r?v=gotip

What did you see happen?

I see the MemprofileRecord stacks are truncated at 32 depth and pprof stacks has proper deep stacks.

What did you expect to see?

I expected MemprofileRecord.Stack() to return a slice with correct deep stack.

@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jun 12, 2024
@korniltsev
Copy link
Contributor Author

runtime.MemProfile and MemprofileRecord are used in [godeltaprof] (https://github.com/grafana/pyroscope-go/tree/main/godeltaprof)

It is probably possible to hack around the limitations by linking into runtime/pprof.memProfileInternal but in the light of #67401 this now looks like less attractive idea.

@felixge
Copy link
Contributor

felixge commented Jun 12, 2024

I worked on the patch for adding deep stack traces. FWIW I initially tried to make deep stack traces available via the public API, see v35 of my CL.

The problem is that there are various go1compat challenges in doing this, so we'll need a proposal that either manages to solve the problem without compat issues, or build consensus for accepting a minor amount of compat issues. Specifically:

  1. Are we okay with breaking apps that don't expect Stack() to return more than Stack0 frames?
  2. Are we okay if the StackRecord types are no longer properly comparable? (in either the spec sense, or by containing a pointer that makes comparison meaningless)
  3. Are we okay with changing the semantics of whether or not the slice returned by Stack() is backed by the Stack0 array in all cases?

@korniltsev
Copy link
Contributor Author

One of the possible solutions (for godeltaprof) could be to have the same functionality in golang standard library and to deprecate the godeltaprof project alltogether.

#57765
#67942

@znkr
Copy link
Contributor

znkr commented Jun 12, 2024

We have a similar issue in with Google's internal peakheap profile implementation. As the name suggest, the peakheap profile keeps track of high-water-mark heap profiles. I was considering filing a proposal to add peakheap profiles to the runtime, but I didn't have enough time to think it through. Given that I am not the only one with such a problem, I am favoring the idea of exporting the larger stacks via a runtime API now.

Regarding the go1compat challenges: Do we have to change the existing API or could there be a v2 API (e.g. in a new package, say runtime/profile).

@prattmic prattmic added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jun 12, 2024
@prattmic
Copy link
Member

cc @golang/runtime

@rsc
Copy link
Contributor

rsc commented Jun 12, 2024

I believe this is a duplicate of #43669.

@mknyszek mknyszek added this to the Backlog milestone Jun 12, 2024
@mknyszek
Copy link
Contributor

mknyszek commented Jun 12, 2024

@rsc I think there's some overlap, but this is really about a specific unresolved part of #43669. Meanwhile, #43669 is now mostly resolved, since as of https://go.dev/cl/572396, deep stacks are available everywhere except the runtime APIs mentioned in this issue. I think maybe we should close #43669 in favor of this issue?

@rsc
Copy link
Contributor

rsc commented Jun 28, 2024

Sure, if you think #43669 is done, then feel free to close it.

Personally, I would say that the API to access the deep stack traces is runtime/pprof, and we just deprecate the old MemProfileRecord, BlockProfileRecord structs.

@korniltsev
Copy link
Contributor Author

I assume we will have to link into new pprof_memPrifileInternal and co. if we want deep stacktraces? I am not missing anything, right?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
Development

No branches or pull requests

9 participants