runtime/pprof: force use of 4-column profiles in pprof memprofile output

Pprof's converter from legacy text format to protobuf format
assumes that if the alloc and inuse stats are equal, then what's
really going on is that the program makes no distinction, and it
reads them as a two-column profile: objects and bytes.

Most of the time, some sampled object has been freed, and alloc != inuse.
In that case, pprof reads the profile as a four-column profile, with
alloc_objects, alloc_bytes, inuse_objects, inuse_bytes.

The 2-column form causes problems in a few ways. One is that if
you are reading the proto form and expect samples with the 4-column
names, they're not there. Another is that pprof's profile merger insists
on having the same number of columns and same names. This means
that

	pprof *.memprofile

works most of the time but fails if one of the memory profiles hit
the unlikely condition that alloc == inuse, since now its converted
form differs from the others.

Most programs should simply not be using this output form at all,
but cmd/compile and cmd/link still do, because x/tools/cmd/compilebench
reads some extra values from the text form that we have not yet added
to the proto form.

For the programs still writing this form, the easiest way to avoid the
column collapse issues is to ensure that the header never reports
alloc == inuse. The actual values in the header are ignored by pprof now,
except for the equality check (they should sum to the other values in the
file, so they are technically redundant). Because the actual values are not
used except for the equality check, we could hard-code different values
like 0 and 1, but just in case, to break as little as possible, this CL only
adjusts the values when they would otherwise be equal. In that case it
adds 1 to allocBytes. For most profiles, where alloc != inuse already, there
is no effect at all.

Change-Id: Ia563e402573d0f6eb81ae496645db27c08f9fe31
Reviewed-on: https://go-review.googlesource.com/c/go/+/432758
Reviewed-by: David Chase <drchase@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Run-TryBot: Russ Cox <rsc@golang.org>

diff --git a/src/runtime/pprof/pprof.go b/src/runtime/pprof/pprof.go
index 3a7191e092e7b95eb807e6d24f7f553ad842462d..78445b6264b4f11ae832390dec4afaff6d66a541 100644 (file)
--- a/src/runtime/pprof/pprof.go
+++ b/src/runtime/pprof/pprof.go
@@ -592,10 +592,24 @@ func writeHeapInternal(w io.Writer, debug int, defaultSampleType string) error {
        // Technically the rate is MemProfileRate not 2*MemProfileRate,
        // but early versions of the C++ heap profiler reported 2*MemProfileRate,
        // so that's what pprof has come to expect.
+       rate := 2 * runtime.MemProfileRate
+
+       // pprof reads a profile with alloc == inuse as being a "2-column" profile
+       // (objects and bytes, not distinguishing alloc from inuse),
+       // but then such a profile can't be merged using pprof *.prof with
+       // other 4-column profiles where alloc != inuse.
+       // The easiest way to avoid this bug is to adjust allocBytes so it's never == inuseBytes.
+       // pprof doesn't use these header values anymore except for checking equality.
+       inUseBytes := total.InUseBytes()
+       allocBytes := total.AllocBytes
+       if inUseBytes == allocBytes {
+               allocBytes++
+       }
+
        fmt.Fprintf(w, "heap profile: %d: %d [%d: %d] @ heap/%d\n",
-               total.InUseObjects(), total.InUseBytes(),
-               total.AllocObjects, total.AllocBytes,
-               2*runtime.MemProfileRate)
+               total.InUseObjects(), inUseBytes,
+               total.AllocObjects, allocBytes,
+               rate)
 
        for i := range p {
                r := &p[i]