Open JetSetIlly opened 1 year ago
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
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.
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.
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
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.
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.
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.
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?
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.
https://github.com/golang/go/issues/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.
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.
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.
You can have multiple binaries with the same vcs.revision
and vcs.modified=true
. It does not record what the modification is.
Change https://go.dev/cl/481619 mentions this issue: runtime/pprof: avoid including filepath on Windows
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?go env
OutputWhat 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:
What did you see instead?