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: system information in output #59307

Open
JetSetIlly opened this issue Mar 29, 2023 · 13 comments
Open

runtime/pprof: system information in output #59307

JetSetIlly opened this issue Mar 29, 2023 · 13 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@JetSetIlly
Copy link

What version of Go are you using (go version)?

go version go1.20.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN="/home/Go/go-current/go/bin"
GOCACHE="/home/cache/go-build"
GOENV="/home/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/Go/go-current/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/Go/go-current/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/home/Go/go-current"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/home/Go/go-current/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.20.2"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/dev/null"
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 -fdebug-prefix-map=/tmp/go-build1401588216=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Cross compiled a binary from a Linux system to Windows.

What did you expect to see?

CPU profiles produced on the target system (Windows) to contain no system information and a valid build ID. For example:

$ go tool pprof myprofile
File: myprogram
Build ID: 291bb04836262539078614ce0e65ab3dcac05f04
Type: cpu
Time: Mar 29, 2023 at 11:23am (BST)
Duration: 2.63s, Total samples = 1.51s (57.43%)
Entering interactive mode (type "help" for commands, "o" for options)

What did you see instead?

$ go tool pprof myprofile
File: D:\Programs\myprogram\myprogram.exe
Build ID: D:\Programs\myprogram\myprogram.exe2023-03-27 23:33:12.3221123 > +0200 CEST
Type: cpu
Time: Mar 27, 2023 at 10:37pm (BST)
Duration: 24.23s, Total samples = 38.10s (157.22%)
Entering interactive mode (type "help" for commands, "o" for options)
@JetSetIlly JetSetIlly changed the title affected/package: pprof: system information in output Mar 29, 2023
@seankhliao
Copy link
Member

I believe this is from pprof locating the binary locally in its search?

  Environment Variables:
   PPROF_TMPDIR       Location for saved profiles (default $HOME/pprof)
   PPROF_TOOLS        Search path for object-level tools
   PPROF_BINARY_PATH  Search path for local binary files
                      default: $HOME/pprof/binaries
                      searches $buildid/$name, $buildid/*, $path/$buildid,
                      ${buildid:0:2}/${buildid:2}.debug, $name, $path
   * On Windows, %USERPROFILE% is used instead of $HOME

@seankhliao seankhliao added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Mar 29, 2023
@JetSetIlly
Copy link
Author

JetSetIlly commented Mar 29, 2023

I don't have any of those environment variables set and nor would the end users who are sending me the profile file.

The profile files are being sent to me by end users who are not Go programmers. I have a command line argument to my program that users can specify when running the program. I ask them to run with this on the occasions that they see performance issues.

@JetSetIlly
Copy link
Author

To clarify the issue: I'm wondering why local path information is in the profile file. It's of no use to the person examining the profile. Moreover, the BuildID field should be the Build ID of the binary that created the profile I think.

@cherrymui
Copy link
Member

The Build ID is from https://cs.opensource.google/go/go/+/master:src/runtime/pprof/pe.go;l=13 . Apparently it just needs to be a string that is unique, so it simply picked the file name and the timestamp. I guess we could read the build ID from the binary. Not sure if it matters much.

cc @egonelbre @golang/windows

@JetSetIlly
Copy link
Author

JetSetIlly commented Mar 29, 2023

The Build ID is from https://cs.opensource.google/go/go/+/master:src/runtime/pprof/pe.go;l=13 . Apparently it just needs to be a string that is unique, so it simply picked the file name and the timestamp. I guess we could read the build ID from the binary. Not sure if it matters much.

cc @egonelbre @golang/windows

I think it does matter. If someone sends me a CPU profile I want to know which revision of the code generated it.

edit: Yes, I could arrange for that information to be conveyed some other way but it seems to me that the BuildID field is the best way of conveying that information; particularly as executables compiled with recent Go versions are embedded with that same information.

@seankhliao seankhliao added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Mar 29, 2023
@seankhliao seankhliao changed the title pprof: system information in output runtime/pprof: system information in output Mar 29, 2023
@egonelbre
Copy link
Contributor

egonelbre commented Mar 30, 2023

Yes, I added filename as build id in https://go-review.googlesource.com/c/go/+/416975.

I agree that it's not ideal... unfortunately, I wasn't able to find a better alternative in https://go-review.googlesource.com/c/go/+/416975/comment/dd563dbb_63dff0da/ and no one else had a better suggestion.

@JetSetIlly
Copy link
Author

Thanks for the link egonelbre.

I personally think the filename should be stripped of path information, so "filepath.Base(file)" rather than just "file".

I suppose a filename-and-timestamp string acting as the build ID is fine but it's not clear to me why a hash of the file would be "wasteful". It would only be performed once wouldn't it?

How about a hash of the filename-and-timestamp string? That would be unique.

It's a shame that there doesn't seem to be a way to easily get the build ID of the go binary (as it would be returned by "go tool buildid"). That would be an ideal solution I think.

@egonelbre
Copy link
Contributor

I agree that we should avoid including the full path and using a hash of filename+modtime would be an improvement.

Calculating the hash of executable is wasteful because they can be several hundred megabytes in some cases.

But, yes, some unique identifier in the binary would be ideal. Should we insert one during compilation?

@JetSetIlly
Copy link
Author

JetSetIlly commented Mar 30, 2023

I was under the impression that a unique identifier is already included in the binary. Isn't the value returned by "go tool buildid" a unique identifier inserted during compilation? I don't think it's accessible through the standard library though.

There is also the the vcs.revision entry in the runtime/debug BuildInfo struct. But there is an issue with that currently by the looks of things.

#51279

If vcs.revision is not available then the hash of filename+modtime would be a good alternative.

edit: For me, the vcs.revision is perfect for the use case of a user submitting a profile because it removes any doubt about which version of the source the profile refers to.

@egonelbre
Copy link
Contributor

Yes, buildid would work. I don't remember anymore why I didn't use it. You can try https://github.com/golang/go/blob/master/src/cmd/internal/buildid/buildid.go#L265 logic.

vcs.revision would only work when the code hasn't been modified.

@JetSetIlly
Copy link
Author

vcs.revision would only work when the code hasn't been modified.

There's a vcs.modified field that could be used somehow in those cases.

@egonelbre
Copy link
Contributor

You can have multiple binaries with the same vcs.revision and vcs.modified=true. It does not record what the modification is.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/481619 mentions this issue: runtime/pprof: avoid including filepath on Windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

5 participants