runtime/pprof: clarify recursive inline heuristic

Following CL 226818, the compiler will allow inlining a single cycle in
an inline chain. Immediately-recursive functions are still disallowed,
which is what this heuristic refers to.

Add a regression test for this case.

Note that in addition to this check, if the compiler were to inline
multiple cycles via a loop (i.e., rather than appending duplicate code),
much more work would be required here to handle a single address
appearing in multiple different inline frames.

Updates #29737

Change-Id: I88de15cfbeabb9c04381e1c12cc36778623132a5
Reviewed-on: https://go-review.googlesource.com/c/go/+/227346
Run-TryBot: Michael Pratt <mpratt@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Dan Scales <danscales@google.com>
Reviewed-by: Hyang-Ah Hana Kim <hyangah@gmail.com>

diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go
index 52b51ee60d1d2980c7f445ec8497090994020a77..c3e253eb41cf49ed5d464cd003fe8a31cb0fcf2b 100644 (file)
--- a/src/runtime/pprof/pprof_test.go
+++ b/src/runtime/pprof/pprof_test.go
@@ -227,6 +227,25 @@ func recursionCallee(n, x int) int {
        return y * recursionCallee(n-1, x)
 }
 
+func recursionChainTop(x int, pcs []uintptr) {
+       if x < 0 {
+               return
+       }
+       recursionChainMiddle(x, pcs)
+}
+
+func recursionChainMiddle(x int, pcs []uintptr) {
+       recursionChainBottom(x, pcs)
+}
+
+func recursionChainBottom(x int, pcs []uintptr) {
+       // This will be called each time, we only care about the last. We
+       // can't make this conditional or this function won't be inlined.
+       dumpCallers(pcs)
+
+       recursionChainTop(x - 1, pcs)
+}
+
 func parseProfile(t *testing.T, valBytes []byte, f func(uintptr, []*profile.Location, map[string][]string)) *profile.Profile {
        p, err := profile.Parse(bytes.NewReader(valBytes))
        if err != nil {
@@ -1158,11 +1177,12 @@ func TestTracebackAll(t *testing.T) {
        }
 }
 
-// TestTryAdd tests the cases that's hard to test with real program execution.
-// For example, the current go compilers may not inline functions involved in recursion
-// but that may not be true in the future compilers. This tests such cases by
-// using fake call sequences and forcing the profile build utilizing
-// translateCPUProfile defined in proto_test.go
+// TestTryAdd tests the cases that are hard to test with real program execution.
+//
+// For example, the current go compilers may not always inline functions
+// involved in recursion but that may not be true in the future compilers. This
+// tests such cases by using fake call sequences and forcing the profile build
+// utilizing translateCPUProfile defined in proto_test.go
 func TestTryAdd(t *testing.T) {
        if _, found := findInlinedCall(inlinedCallerDump, 4<<10); !found {
                t.Skip("Can't determine whether anything was inlined into inlinedCallerDump.")
@@ -1177,6 +1197,23 @@ func TestTryAdd(t *testing.T) {
                inlinedCallerStack[i] = uint64(pcs[i])
        }
 
+       if _, found := findInlinedCall(recursionChainBottom, 4<<10); !found {
+               t.Skip("Can't determine whether anything was inlined into recursionChainBottom.")
+       }
+
+       // recursionChainTop
+       //   recursionChainMiddle
+       //     recursionChainBottom
+       //       recursionChainTop
+       //         recursionChainMiddle
+       //           recursionChainBottom
+       pcs = make([]uintptr, 6)
+       recursionChainTop(1, pcs)
+       recursionStack := make([]uint64, len(pcs))
+       for i := range pcs {
+               recursionStack[i] = uint64(pcs[i])
+       }
+
        period := int64(2000 * 1000) // 1/500*1e9 nanosec.
 
        testCases := []struct {
@@ -1225,14 +1262,14 @@ func TestTryAdd(t *testing.T) {
                        {Value: []int64{4242, 4242 * period}, Location: []*profile.Location{{ID: 1}}},
                },
        }, {
-               // If a function is called recursively then it must not be
-               // inlined in the caller.
+               // If a function is directly called recursively then it must
+               // not be inlined in the caller.
                //
                // N.B. We're generating an impossible profile here, with a
                // recursive inlineCalleeDump call. This is simulating a non-Go
                // function that looks like an inlined Go function other than
                // its recursive property. See pcDeck.tryAdd.
-               name: "recursive_func_is_not_inlined",
+               name: "directly_recursive_func_is_not_inlined",
                input: []uint64{
                        3, 0, 500, // hz = 500. Must match the period.
                        5, 0, 30, inlinedCallerStack[0], inlinedCallerStack[0],
@@ -1245,6 +1282,28 @@ func TestTryAdd(t *testing.T) {
                        {Value: []int64{30, 30 * period}, Location: []*profile.Location{{ID: 1}, {ID: 1}, {ID: 2}}},
                        {Value: []int64{40, 40 * period}, Location: []*profile.Location{{ID: 1}, {ID: 2}}},
                },
+       }, {
+               name: "recursion_chain_inline",
+               input: []uint64{
+                       3, 0, 500, // hz = 500. Must match the period.
+                       9, 0, 10, recursionStack[0], recursionStack[1], recursionStack[2], recursionStack[3], recursionStack[4], recursionStack[5],
+               },
+               wantLocs: [][]string{
+                       {"runtime/pprof.recursionChainBottom"},
+                       {
+                               "runtime/pprof.recursionChainMiddle",
+                               "runtime/pprof.recursionChainTop",
+                               "runtime/pprof.recursionChainBottom",
+                       },
+                       {
+                               "runtime/pprof.recursionChainMiddle",
+                               "runtime/pprof.recursionChainTop",
+                               "runtime/pprof.TestTryAdd", // inlined into the test.
+                       },
+               },
+               wantSamples: []*profile.Sample{
+                       {Value: []int64{10, 10 * period}, Location: []*profile.Location{{ID: 1}, {ID: 2}, {ID: 3}}},
+               },
        }, {
                name: "truncated_stack_trace_later",
                input: []uint64{

diff --git a/src/runtime/pprof/proto.go b/src/runtime/pprof/proto.go
index bb63153a701b38f89abc87d1c8da38ecb469e872..f3d8ac38bf0384120582bc4f2642bf12ffbf5a72 100644 (file)
--- a/src/runtime/pprof/proto.go
+++ b/src/runtime/pprof/proto.go
@@ -461,7 +461,7 @@ func (b *profileBuilder) appendLocsForStack(locs []uint64, stk []uintptr) (newLo
 // have the following properties:
 //   Frame's Func is nil (note: also true for non-Go functions), and
 //   Frame's Entry matches its entry function frame's Entry (note: could also be true for recursive calls and non-Go functions), and
-//   Frame's Name does not match its entry function frame's name (note: inlined functions cannot be recursive).
+//   Frame's Name does not match its entry function frame's name (note: inlined functions cannot be directly recursive).
 //
 // As reading and processing the pcs in a stack trace one by one (from leaf to the root),
 // we use pcDeck to temporarily hold the observed pcs and their expanded frames