Skip to content

Commit

Permalink
runtime/pprof: fix inlined generics locations
Browse files Browse the repository at this point in the history
When generic function[a,b] is inlined to the same generic function[b,a]
with different types (not recursion) it is expected to get a pprof with
a single Location with two functions. However due to incorrect check
for generics names using runtime.Frame.Function, the profileBuilder
assumes it is a recursion and emits separate Location.

This change fixes the recursion check for generics functions by using
runtime_expandFinalInlineFrame

Fixes #64641

Change-Id: I3f58818f08ee322b281daa377fa421555ad328c9
Reviewed-on: https://go-review.googlesource.com/c/go/+/549135
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
korniltsev authored and prattmic committed Dec 13, 2023
1 parent cc5ccd7 commit d95e25e
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 12 deletions.
2 changes: 1 addition & 1 deletion src/runtime/pprof/proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,7 @@ func (d *pcDeck) tryAdd(pc uintptr, frames []runtime.Frame, symbolizeResult symb
if last.Entry != newFrame.Entry { // newFrame is for a different function.
return false
}
if last.Function == newFrame.Function { // maybe recursion.
if runtime_FrameSymbolName(&last) == runtime_FrameSymbolName(&newFrame) { // maybe recursion.
return false
}
}
Expand Down
99 changes: 88 additions & 11 deletions src/runtime/pprof/protomem_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"bytes"
"fmt"
"internal/profile"
"internal/testenv"
"runtime"
"slices"
"strings"
Expand Down Expand Up @@ -90,22 +91,31 @@ func genericAllocFunc[T interface{ uint32 | uint64 }](n int) []T {
return make([]T, n)
}

func profileToString(p *profile.Profile) []string {
func profileToStrings(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))
res = append(res, sampleToString(s))
}
return res
}

func sampleToString(s *profile.Sample) string {
var funcs []string
for i := len(s.Location) - 1; i >= 0; i-- {
loc := s.Location[i]
funcs = locationToStrings(loc, funcs)
}
return fmt.Sprintf("%s %v", strings.Join(funcs, ";"), s.Value)
}

func locationToStrings(loc *profile.Location, funcs []string) []string {
for j := range loc.Line {
line := loc.Line[len(loc.Line)-1-j]
funcs = append(funcs, line.Function.Name)
}
return funcs
}

// This is a regression test for https://go.dev/issue/64528 .
func TestGenericsHashKeyInPprofBuilder(t *testing.T) {
previousRate := runtime.MemProfileRate
Expand All @@ -130,7 +140,7 @@ func TestGenericsHashKeyInPprofBuilder(t *testing.T) {
t.Fatalf("profile.Parse: %v", err)
}

actual := profileToString(p)
actual := profileToStrings(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]",
Expand All @@ -144,3 +154,70 @@ func TestGenericsHashKeyInPprofBuilder(t *testing.T) {
}
}
}

type opAlloc struct {
buf [128]byte
}

type opCall struct {
}

var sink []byte

func storeAlloc() {
sink = make([]byte, 16)
}

func nonRecursiveGenericAllocFunction[CurrentOp any, OtherOp any](alloc bool) {
if alloc {
storeAlloc()
} else {
nonRecursiveGenericAllocFunction[OtherOp, CurrentOp](true)
}
}

func TestGenericsInlineLocations(t *testing.T) {
if testenv.OptimizationOff() {
t.Skip("skipping test with optimizations disabled")
}

previousRate := runtime.MemProfileRate
runtime.MemProfileRate = 1
defer func() {
runtime.MemProfileRate = previousRate
sink = nil
}()

nonRecursiveGenericAllocFunction[opAlloc, opCall](true)
nonRecursiveGenericAllocFunction[opCall, opAlloc](false)

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)
}

const expectedSample = "testing.tRunner;runtime/pprof.TestGenericsInlineLocations;runtime/pprof.nonRecursiveGenericAllocFunction[go.shape.struct {},go.shape.struct { runtime/pprof.buf [128]uint8 }];runtime/pprof.nonRecursiveGenericAllocFunction[go.shape.struct { runtime/pprof.buf [128]uint8 },go.shape.struct {}];runtime/pprof.storeAlloc [1 16 1 16]"
const expectedLocation = "runtime/pprof.nonRecursiveGenericAllocFunction[go.shape.struct {},go.shape.struct { runtime/pprof.buf [128]uint8 }];runtime/pprof.nonRecursiveGenericAllocFunction[go.shape.struct { runtime/pprof.buf [128]uint8 },go.shape.struct {}];runtime/pprof.storeAlloc"
const expectedLocationNewInliner = "runtime/pprof.TestGenericsInlineLocations;" + expectedLocation
var s *profile.Sample
for _, sample := range p.Sample {
if sampleToString(sample) == expectedSample {
s = sample
break
}
}
if s == nil {
t.Fatalf("expected \n%s\ngot\n%s", expectedSample, strings.Join(profileToStrings(p), "\n"))
}
loc := s.Location[0]
actual := strings.Join(locationToStrings(loc, nil), ";")
if expectedLocation != actual && expectedLocationNewInliner != actual {
t.Errorf("expected a location with at least 3 functions\n%s\ngot\n%s\n", expectedLocation, actual)
}
}

0 comments on commit d95e25e

Please sign in to comment.