From 0a15e7851a0ea1ebe1523bb70a6cfe56488ea2ef Mon Sep 17 00:00:00 2001 From: Michael Pratt Date: Mon, 6 Dec 2021 17:26:32 -0500 Subject: [PATCH] runtime/pprof: assert that labelHog samples are always labeled With https://golang.org/issue/50007 resolved, there are no known issues with pprof labels remaining. Thus, the 10% allowed error in TestLabelSystemstack should not be required. Drop it in favor of an explicit assertion that all samples containing labelHog are properly labeled. This is no flaky in my local testing. It is possible that other bugs will appear at larger testing scale, in which case this CL will be reverted, but then at least we will be aware of additional failure modes. For #50007. Change-Id: I1ef530c303bd9a01af649b8b08d4b35505e8aada Reviewed-on: https://go-review.googlesource.com/c/go/+/369744 Reviewed-by: Bryan Mills Trust: Michael Pratt Run-TryBot: Michael Pratt TryBot-Result: Gopher Robot --- src/runtime/pprof/pprof_test.go | 68 +++++++++++---------------------- 1 file changed, 22 insertions(+), 46 deletions(-) diff --git a/src/runtime/pprof/pprof_test.go b/src/runtime/pprof/pprof_test.go index 913f899593..2e6165ff88 100644 --- a/src/runtime/pprof/pprof_test.go +++ b/src/runtime/pprof/pprof_test.go @@ -1425,52 +1425,8 @@ func TestLabelRace(t *testing.T) { // TestLabelSystemstack makes sure CPU profiler samples of goroutines running // on systemstack include the correct pprof labels. See issue #48577 func TestLabelSystemstack(t *testing.T) { - matchBasics := matchAndAvoidStacks(stackContainsLabeled, []string{"runtime.systemstack;key=value"}, avoidFunctions()) - matches := func(t *testing.T, prof *profile.Profile) bool { - if !matchBasics(t, prof) { - return false - } - - var withLabel, withoutLabel int64 - for _, s := range prof.Sample { - var systemstack, labelHog bool - for _, loc := range s.Location { - for _, l := range loc.Line { - switch l.Function.Name { - case "runtime.systemstack": - systemstack = true - case "runtime/pprof.labelHog": - labelHog = true - } - } - } - - if systemstack && labelHog { - if s.Label != nil && contains(s.Label["key"], "value") { - withLabel += s.Value[0] - } else { - withoutLabel += s.Value[0] - } - } - } - - // ratio on 2019 Intel MBP before/after CL 351751 for n=30 runs: - // before: mean=0.013 stddev=0.013 min=0.000 max=0.039 - // after : mean=0.996 stddev=0.007 min=0.967 max=1.000 - // - // TODO: Figure out why some samples (containing gcWriteBarrier, gcStart) - // still have labelHog without labels. Once fixed this test case can be - // simplified to just check that all samples containing labelHog() have the - // label, and no other samples do. - ratio := float64(withLabel) / float64((withLabel + withoutLabel)) - if ratio < 0.9 { - t.Logf("only %.1f%% of labelHog(systemstack()) samples have label", ratio*100) - return false - } - return true - } - - testCPUProfile(t, matches, func(dur time.Duration) { + matches := matchAndAvoidStacks(stackContainsLabeled, []string{"runtime.systemstack;key=value"}, avoidFunctions()) + p := testCPUProfile(t, matches, func(dur time.Duration) { Do(context.Background(), Labels("key", "value"), func(context.Context) { var wg sync.WaitGroup stop := make(chan struct{}) @@ -1487,6 +1443,26 @@ func TestLabelSystemstack(t *testing.T) { wg.Wait() }) }) + + // labelHog should always be labeled. + for _, s := range p.Sample { + for _, loc := range s.Location { + for _, l := range loc.Line { + if l.Function.Name != "runtime/pprof.labelHog" { + continue + } + + if s.Label == nil { + t.Errorf("labelHog sample labels got nil want key=value") + continue + } + if !contains(s.Label["key"], "value") { + t.Errorf("labelHog sample labels got %+v want contains key=value", s.Label) + continue + } + } + } + } } // labelHog is designed to burn CPU time in a way that a high number of CPU -- 2.48.1