]> Cypherpunks.ru repositories - gostls13.git/commit
internal/profile: fully decode proto even if there are no samples
authorMichael Pratt <mpratt@google.com>
Wed, 6 Dec 2023 18:29:03 +0000 (13:29 -0500)
committerMichael Pratt <mpratt@google.com>
Thu, 7 Dec 2023 19:52:28 +0000 (19:52 +0000)
commite1c0349a7c607cdfcfa8f7c0c6b70aceff9d3752
tree867a43ae4a6ca02387b5074d273e0be3b4714ca3
parentc71eedf90aff3fc73a645b88d2e5166b8a0179fd
internal/profile: fully decode proto even if there are no samples

This is a partial revert of CL 483137.

CL 483137 started checking errors in postDecode, which is good. Now we
can catch more malformed pprof protos. However this made
TestEmptyProfile fail, so an early return was added when the profile was
"empty" (no samples).

Unfortunately, this was problematic. Profiles with no samples can still
be valid, but skipping postDecode meant that the resulting Profile was
missing values from the string table. In particular, net/http/pprof
needs to parse empty profiles in order to pass through the sample and
period types to a final output proto. CL 483137 broke this behavior.

internal/profile.Parse is only used in two places: in cmd/compile to
parse PGO pprof profiles, and in net/http/pprof to parse before/after
pprof profiles for delta profiles. In both cases, the input is never
literally empty (0 bytes). Even a pprof proto with no samples still
contains some header fields, such as sample and period type. Upstream
github.com/google/pprof/profile even has an explicit error on 0 byte
input, so `go tool pprof` will not support such an input.

Thus TestEmptyProfile was misleading; this profile doesn't need to
support empty input at all.

Resolve this by removing TestEmptyProfile and replacing it with an
explicit error on empty input, as upstream
github.com/google/pprof/profile has. For non-empty input, always run
postDecode to ensure the string table is processed.

TestConvertCPUProfileEmpty is reverted back to assert the values from
before CL 483137. Note that in this case "Empty" means no samples, not a
0 byte input.

Continue to allow empty files for PGO in order to minimize the chance of
last minute breakage if some users have empty files.

Fixes #64566.

Change-Id: I83a1f0200ae225ac6da0009d4b2431fe215b283f
Reviewed-on: https://go-review.googlesource.com/c/go/+/547996
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
src/cmd/compile/internal/pgo/irgraph.go
src/internal/profile/encode.go
src/internal/profile/profile.go
src/internal/profile/profile_test.go
src/net/http/pprof/pprof_test.go
src/net/http/pprof/testdata/delta_mutex.go [new file with mode: 0644]
src/runtime/pprof/proto_test.go