Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(godeltaprof): incorrect function names for generics functions #106

Merged
merged 3 commits into from
May 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/go.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
strategy:
fail-fast: false
matrix:
go: ['1.16', '1.17', '1.18', '1.19', '1.20', '1.21', 'tip']
go: ['1.16', '1.17', '1.18', '1.19', '1.20', '1.21', '1.22', 'tip']
steps:
- name: Checkout
uses: actions/checkout@v3
Expand Down
155 changes: 155 additions & 0 deletions godeltaprof/compat/generics_go21_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,13 @@ package compat

import (
"bytes"
"fmt"
"github.com/google/pprof/profile"
"io"
"runtime"
"runtime/pprof"
"slices"
"strings"
"sync"
"testing"
"time"
Expand Down Expand Up @@ -194,3 +199,153 @@ func TestMutex(t *testing.T) {
expectStackFrames(t, buffer, expectedRealShape, 19)
})
}

func profileToStrings(p *profile.Profile) []string {
var res []string
for _, s := range p.Sample {
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
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 := profileToStrings(p)
expected := []string{
"testing.tRunner;github.com/grafana/pyroscope-go/godeltaprof/compat.TestGenericsHashKeyInPprofBuilder;github.com/grafana/pyroscope-go/godeltaprof/compat.genericAllocFunc[go.shape.uint32] [1 128 0 0]",
"testing.tRunner;github.com/grafana/pyroscope-go/godeltaprof/compat.TestGenericsHashKeyInPprofBuilder;github.com/grafana/pyroscope-go/godeltaprof/compat.genericAllocFunc[go.shape.uint32] [1 256 0 0]",
"testing.tRunner;github.com/grafana/pyroscope-go/godeltaprof/compat.TestGenericsHashKeyInPprofBuilder;github.com/grafana/pyroscope-go/godeltaprof/compat.genericAllocFunc[go.shape.uint64] [1 32 0 0]",
"testing.tRunner;github.com/grafana/pyroscope-go/godeltaprof/compat.TestGenericsHashKeyInPprofBuilder;github.com/grafana/pyroscope-go/godeltaprof/compat.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)
}
}
}

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 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;github.com/grafana/pyroscope-go/godeltaprof/compat.TestGenericsInlineLocations;github.com/grafana/pyroscope-go/godeltaprof/compat.nonRecursiveGenericAllocFunction[go.shape.struct {},go.shape.struct { github.com/grafana/pyroscope-go/godeltaprof/compat.buf [128]uint8 }];github.com/grafana/pyroscope-go/godeltaprof/compat.nonRecursiveGenericAllocFunction[go.shape.struct { github.com/grafana/pyroscope-go/godeltaprof/compat.buf [128]uint8 },go.shape.struct {}];github.com/grafana/pyroscope-go/godeltaprof/compat.storeAlloc [1 16 1 16]"
const expectedLocation = "github.com/grafana/pyroscope-go/godeltaprof/compat.nonRecursiveGenericAllocFunction[go.shape.struct {},go.shape.struct { github.com/grafana/pyroscope-go/godeltaprof/compat.buf [128]uint8 }];github.com/grafana/pyroscope-go/godeltaprof/compat.nonRecursiveGenericAllocFunction[go.shape.struct { github.com/grafana/pyroscope-go/godeltaprof/compat.buf [128]uint8 },go.shape.struct {}];github.com/grafana/pyroscope-go/godeltaprof/compat.storeAlloc"
const expectedLocationNewInliner = "github.com/grafana/pyroscope-go/godeltaprof/compat.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)
}
}

func OptimizationOff() bool {
optimizationMarker := func() uintptr {
pc, _, _, _ := runtime.Caller(0)
return pc
}
pc := optimizationMarker()
f := runtime.FuncForPC(runtime.FuncForPC(pc).Entry())
return f.Name() == "github.com/grafana/pyroscope-go/godeltaprof/compat.OptimizationOff.func1"
}

func WriteHeapProfile(w io.Writer) error {
runtime.GC()
dh := godeltaprof.NewHeapProfilerWithOptions(godeltaprof.ProfileOptions{
GenericsFrames: true,
LazyMappings: true,
})
return dh.Profile(w)
}
9 changes: 5 additions & 4 deletions godeltaprof/internal/pprof/proto.go
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,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 Expand Up @@ -524,13 +524,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)
var name string
if b.opt.GenericsFrames {
name = runtime_FrameSymbolName(&frame)
name = funcName
} else {
name = frame.Function
}
Expand Down
Loading