]> Cypherpunks repositories - gostls13.git/commitdiff
runtime/pprof: fix inlined generics locations
authorTolya Korniltsev <korniltsev.anatoly@gmail.com>
Tue, 12 Dec 2023 15:24:34 +0000 (22:24 +0700)
committerMichael Pratt <mpratt@google.com>
Wed, 13 Dec 2023 20:40:52 +0000 (20:40 +0000)
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>

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

index db9384eb214e6352f2c2844c6acaa48f1fb572e5..5214374bd9b087c4b6c272d3908de23d4eb5a9f6 100644 (file)
@@ -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
                }
        }
index 505c323d68672cffdd305cab7ae807fc95befe4f..5fb67c53f6753186675b7cd3276a0b261cb5ece3 100644 (file)
@@ -8,6 +8,7 @@ import (
        "bytes"
        "fmt"
        "internal/profile"
+       "internal/testenv"
        "runtime"
        "slices"
        "strings"
@@ -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
@@ -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]",
@@ -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)
+       }
+}