]> Cypherpunks repositories - gostls13.git/commitdiff
[release-branch.go1.21] runtime/pprof: fix generics function names
authorTolya Korniltsev <korniltsev.anatoly@gmail.com>
Mon, 4 Dec 2023 10:53:29 +0000 (17:53 +0700)
committerGopher Robot <gobot@golang.org>
Thu, 4 Jan 2024 17:22:01 +0000 (17:22 +0000)
profileBuilder is using Frame->Function as key for checking if we already
emitted a function. However for generics functions it has dots there [...],
so sometimes for different functions with different generics types,
the profileBuilder emits wrong functions.

For #64528
For #64609

Change-Id: I8b39245e0b18f4288ce758c912c6748f87cba39a
Reviewed-on: https://go-review.googlesource.com/c/go/+/546815
Reviewed-by: Cherry Mui <cherryyz@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
(cherry picked from commit 20a03fc7130d8d99b513071c7e413b436ea649a2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/549535
Auto-Submit: Matthew Dempsky <mdempsky@google.com>

src/runtime/pprof/proto.go
src/runtime/pprof/protomem_test.go

index cdc4bd7c80d873e78ca04e995cdaae1557a1a0ce..db9384eb214e6352f2c2844c6acaa48f1fb572e5 100644 (file)
@@ -611,13 +611,14 @@ func (b *profileBuilder) emitLocation() uint64 {
        b.pb.uint64Opt(tagLocation_Address, uint64(firstFrame.PC))
        for _, frame := range b.deck.frames {
                // Write out each line in frame expansion.
-               funcID := uint64(b.funcs[frame.Function])
+               funcName := runtime_FrameSymbolName(&frame)
+               funcID := uint64(b.funcs[funcName])
                if funcID == 0 {
                        funcID = uint64(len(b.funcs)) + 1
-                       b.funcs[frame.Function] = int(funcID)
+                       b.funcs[funcName] = int(funcID)
                        newFuncs = append(newFuncs, newFunc{
                                id:        funcID,
-                               name:      runtime_FrameSymbolName(&frame),
+                               name:      funcName,
                                file:      frame.File,
                                startLine: int64(runtime_FrameStartLine(&frame)),
                        })
index 156f6286a92fffab514c159e04bf37dc712bf380..505c323d68672cffdd305cab7ae807fc95befe4f 100644 (file)
@@ -6,8 +6,11 @@ package pprof
 
 import (
        "bytes"
+       "fmt"
        "internal/profile"
        "runtime"
+       "slices"
+       "strings"
        "testing"
 )
 
@@ -82,3 +85,62 @@ func TestConvertMemProfile(t *testing.T) {
                })
        }
 }
+
+func genericAllocFunc[T interface{ uint32 | uint64 }](n int) []T {
+       return make([]T, n)
+}
+
+func profileToString(p *profile.Profile) []string {
+       var res []string
+       for _, s := range p.Sample {
+               var funcs []string
+               for i := len(s.Location) - 1; i >= 0; i-- {
+                       loc := s.Location[i]
+                       for j := len(loc.Line) - 1; j >= 0; j-- {
+                               line := loc.Line[j]
+                               funcs = append(funcs, line.Function.Name)
+                       }
+               }
+               res = append(res, fmt.Sprintf("%s %v", strings.Join(funcs, ";"), s.Value))
+       }
+       return res
+}
+
+// This is a regression test for https://go.dev/issue/64528 .
+func TestGenericsHashKeyInPprofBuilder(t *testing.T) {
+       previousRate := runtime.MemProfileRate
+       runtime.MemProfileRate = 1
+       defer func() {
+               runtime.MemProfileRate = previousRate
+       }()
+       for _, sz := range []int{128, 256} {
+               genericAllocFunc[uint32](sz / 4)
+       }
+       for _, sz := range []int{32, 64} {
+               genericAllocFunc[uint64](sz / 8)
+       }
+
+       runtime.GC()
+       buf := bytes.NewBuffer(nil)
+       if err := WriteHeapProfile(buf); err != nil {
+               t.Fatalf("writing profile: %v", err)
+       }
+       p, err := profile.Parse(buf)
+       if err != nil {
+               t.Fatalf("profile.Parse: %v", err)
+       }
+
+       actual := profileToString(p)
+       expected := []string{
+               "testing.tRunner;runtime/pprof.TestGenericsHashKeyInPprofBuilder;runtime/pprof.genericAllocFunc[go.shape.uint32] [1 128 0 0]",
+               "testing.tRunner;runtime/pprof.TestGenericsHashKeyInPprofBuilder;runtime/pprof.genericAllocFunc[go.shape.uint32] [1 256 0 0]",
+               "testing.tRunner;runtime/pprof.TestGenericsHashKeyInPprofBuilder;runtime/pprof.genericAllocFunc[go.shape.uint64] [1 32 0 0]",
+               "testing.tRunner;runtime/pprof.TestGenericsHashKeyInPprofBuilder;runtime/pprof.genericAllocFunc[go.shape.uint64] [1 64 0 0]",
+       }
+
+       for _, l := range expected {
+               if !slices.Contains(actual, l) {
+                       t.Errorf("profile = %v\nwant = %v", strings.Join(actual, "\n"), l)
+               }
+       }
+}