Skip to content

Commit

Permalink
runtime/pprof: fix generics function names
Browse files Browse the repository at this point in the history
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.

Fixes #64528

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>
  • Loading branch information
korniltsev authored and cherrymui committed Dec 11, 2023
1 parent 052d402 commit 20a03fc
Show file tree
Hide file tree
Showing 2 changed files with 66 additions and 3 deletions.
7 changes: 4 additions & 3 deletions src/runtime/pprof/proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
})
Expand Down
62 changes: 62 additions & 0 deletions src/runtime/pprof/protomem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ package pprof

import (
"bytes"
"fmt"
"internal/profile"
"runtime"
"slices"
"strings"
"testing"
)

Expand Down Expand Up @@ -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)
}
}
}

0 comments on commit 20a03fc

Please sign in to comment.