From 1fe2810f9ca0dcd34e473f852102e2a49d45d7d8 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Fri, 10 Jun 2022 16:21:46 +0000 Subject: [PATCH] sync: move lock linearity test and treat it like a performance test This change moves test/locklinear.go into the sync package tests, and adds a bit of infrastructure since there are other linearity-checking tests that could benefit from it too. This infrastructure is also different than what test/locklinear.go does: instead of trying really hard to get at least one success, we instead treat this like a performance test and look for a significant difference via a t-test. This makes the methodology behind the tests more rigorous, and should reduce flakiness as transient noise should produce an insignificant result. A follow-up CL does more to make these tests even more robust. For #32986. Change-Id: I408c5f643962b70ea708930edb4ac9df1c6123ce Reviewed-on: https://go-review.googlesource.com/c/go/+/411396 Reviewed-by: Michael Pratt --- src/internal/testenv/testenv.go | 65 ++++++++++++ src/internal/testmath/bench.go | 38 +++++++ src/sync/mutex_test.go | 90 +++++++++++++++++ test/locklinear.go | 171 -------------------------------- 4 files changed, 193 insertions(+), 171 deletions(-) create mode 100644 src/internal/testmath/bench.go delete mode 100644 test/locklinear.go diff --git a/src/internal/testenv/testenv.go b/src/internal/testenv/testenv.go index 1feb630cf5f..b7cb95063b2 100644 --- a/src/internal/testenv/testenv.go +++ b/src/internal/testenv/testenv.go @@ -16,6 +16,7 @@ import ( "flag" "fmt" "internal/cfg" + "internal/testmath" "os" "os/exec" "path/filepath" @@ -463,3 +464,67 @@ func RunWithTimeout(t testing.TB, cmd *exec.Cmd) ([]byte, error) { return b.Bytes(), err } + +// CheckLinear checks if the function produced by f scales linearly. +// +// f must accept a scale factor which causes the input to the function it +// produces to scale by that factor. +func CheckLinear(t *testing.T, f func(scale float64) func(*testing.B)) { + MustHaveExec(t) + + if os.Getenv("GO_PERF_UNIT_TEST") == "" { + // Invoke the same test as a subprocess with the GO_PERF_UNIT_TEST environment variable set. + // We create a subprocess for two reasons: + // + // 1. There's no other way to set the benchmarking parameters of testing.Benchmark. + // 2. Since we're effectively running a performance test, running in a subprocess grants + // us a little bit more isolation than using the same process. + // + // As an alternative, we could fairly easily reimplement the timing code in testing.Benchmark, + // but a subprocess is just as easy to create. + + selfCmd := CleanCmdEnv(exec.Command(os.Args[0], "-test.v", fmt.Sprintf("-test.run=^%s$", t.Name()), "-test.benchtime=1x")) + selfCmd.Env = append(selfCmd.Env, "GO_PERF_UNIT_TEST=1") + output, err := RunWithTimeout(t, selfCmd) + if err != nil { + t.Error(err) + t.Logf("--- subprocess output ---\n%s", string(output)) + } + if bytes.Contains(output, []byte("insignificant result")) { + t.Skip("insignificant result") + } + return + } + + // Pick a reasonable sample count. + const count = 10 + + // Collect samples for scale factor 1. + x1 := make([]testing.BenchmarkResult, 0, count) + for i := 0; i < count; i++ { + x1 = append(x1, testing.Benchmark(f(1.0))) + } + + // Collect samples for scale factor 2. + x2 := make([]testing.BenchmarkResult, 0, count) + for i := 0; i < count; i++ { + x2 = append(x2, testing.Benchmark(f(2.0))) + } + + // Run a t-test on the results. + r1 := testmath.BenchmarkResults(x1) + r2 := testmath.BenchmarkResults(x2) + result, err := testmath.TwoSampleWelchTTest(r1, r2, testmath.LocationDiffers) + if err != nil { + t.Fatalf("failed to run t-test: %v", err) + } + if result.P > 0.005 { + // Insignificant result. + t.Skip("insignificant result") + } + + // Let ourselves be within 3x; 2x is too strict. + if m1, m2 := r1.Mean(), r2.Mean(); 3.0*m1 < m2 { + t.Fatalf("failure to scale linearly: µ_1=%s µ_2=%s p=%f", time.Duration(m1), time.Duration(m2), result.P) + } +} diff --git a/src/internal/testmath/bench.go b/src/internal/testmath/bench.go new file mode 100644 index 00000000000..6f034b46855 --- /dev/null +++ b/src/internal/testmath/bench.go @@ -0,0 +1,38 @@ +// Copyright 2022 The Go Authors. All rights reserved. +// Use of this source code is governed by a BSD-style +// license that can be found in the LICENSE file. + +package testmath + +import ( + "math" + "testing" + "time" +) + +type BenchmarkResults []testing.BenchmarkResult + +func (b BenchmarkResults) Weight() float64 { + var weight int + for _, r := range b { + weight += r.N + } + return float64(weight) +} + +func (b BenchmarkResults) Mean() float64 { + var dur time.Duration + for _, r := range b { + dur += r.T * time.Duration(r.N) + } + return float64(dur) / b.Weight() +} + +func (b BenchmarkResults) Variance() float64 { + var num float64 + mean := b.Mean() + for _, r := range b { + num += math.Pow(float64(r.T)-mean, 2) * float64(r.N) + } + return float64(num) / b.Weight() +} diff --git a/src/sync/mutex_test.go b/src/sync/mutex_test.go index cca0986a309..9a4187c6726 100644 --- a/src/sync/mutex_test.go +++ b/src/sync/mutex_test.go @@ -333,3 +333,93 @@ func BenchmarkMutexSpin(b *testing.B) { } }) } + +const runtimeSemaHashTableSize = 251 // known size of runtime hash table + +func TestMutexLinearOne(t *testing.T) { + testenv.CheckLinear(t, func(scale float64) func(*testing.B) { + n := int(1000 * scale) + return func(b *testing.B) { + ch := make(chan int) + locks := make([]RWMutex, runtimeSemaHashTableSize+1) + for i := 0; i < n; i++ { + go func() { + locks[0].Lock() + ch <- 1 + }() + } + time.Sleep(1 * time.Millisecond) + + go func() { + for j := 0; j < n; j++ { + locks[1].Lock() + locks[runtimeSemaHashTableSize].Lock() + locks[1].Unlock() + runtime.Gosched() + locks[runtimeSemaHashTableSize].Unlock() + } + }() + + for j := 0; j < n; j++ { + locks[1].Lock() + locks[runtimeSemaHashTableSize].Lock() + locks[1].Unlock() + runtime.Gosched() + locks[runtimeSemaHashTableSize].Unlock() + } + + for i := 0; i < n; i++ { + <-ch + locks[0].Unlock() + } + } + }) +} + +func TestMutexLinearMany(t *testing.T) { + if runtime.GOARCH == "arm" && os.Getenv("GOARM") == "5" { + // stressLockMany reliably fails on the linux-arm-arm5spacemonkey + // builder. See https://golang.org/issue/24221. + return + } + testenv.CheckLinear(t, func(scale float64) func(*testing.B) { + n := int(1000 * scale) + return func(b *testing.B) { + locks := make([]RWMutex, n*runtimeSemaHashTableSize+1) + + var wg WaitGroup + for i := 0; i < n; i++ { + wg.Add(1) + go func(i int) { + locks[(i+1)*runtimeSemaHashTableSize].Lock() + wg.Done() + locks[(i+1)*runtimeSemaHashTableSize].Lock() + locks[(i+1)*runtimeSemaHashTableSize].Unlock() + }(i) + } + wg.Wait() + + go func() { + for j := 0; j < n; j++ { + locks[1].Lock() + locks[0].Lock() + locks[1].Unlock() + runtime.Gosched() + locks[0].Unlock() + } + }() + + for j := 0; j < n; j++ { + locks[1].Lock() + locks[0].Lock() + locks[1].Unlock() + runtime.Gosched() + locks[0].Unlock() + } + + for i := 0; i < n; i++ { + locks[(i+1)*runtimeSemaHashTableSize].Unlock() + } + } + }) +} diff --git a/test/locklinear.go b/test/locklinear.go deleted file mode 100644 index 54e40a543b1..00000000000 --- a/test/locklinear.go +++ /dev/null @@ -1,171 +0,0 @@ -// run - -// Copyright 2017 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// Test that locks don't go quadratic due to runtime hash table collisions. - -package main - -import ( - "bytes" - "fmt" - "log" - "os" - "runtime" - "runtime/pprof" - "sync" - "time" -) - -const debug = false - -// checkLinear asserts that the running time of f(n) is at least linear but sub-quadratic. -// tries is the initial number of iterations. -func checkLinear(typ string, tries int, f func(n int)) { - // Depending on the machine and OS, this test might be too fast - // to measure with accurate enough granularity. On failure, - // make it run longer, hoping that the timing granularity - // is eventually sufficient. - - timeF := func(n int) time.Duration { - t1 := time.Now() - f(n) - return time.Since(t1) - } - - n := tries - fails := 0 - var buf bytes.Buffer - inversions := 0 - for { - t1 := timeF(n) - t2 := timeF(2 * n) - if debug { - println(n, t1.String(), 2*n, t2.String()) - } - fmt.Fprintf(&buf, "%d %v %d %v (%.1fX)\n", n, t1, 2*n, t2, float64(t2)/float64(t1)) - // should be 2x (linear); allow up to 3x - if t1*3/2 < t2 && t2 < t1*3 { - return - } - if t2 < t1 { - if inversions++; inversions >= 5 { - // The system must be overloaded (some builders). Give up. - return - } - continue // try again; don't increment fails - } - // Once the test runs long enough for n ops, - // try to get the right ratio at least once. - // If many in a row all fail, give up. - if fails++; fails >= 5 { - // If 2n ops run in under a second and the ratio - // doesn't work out, make n bigger, trying to reduce - // the effect that a constant amount of overhead has - // on the computed ratio. - if t2 < time.Second*4/10 { - fails = 0 - n *= 2 - continue - } - panic(fmt.Sprintf("%s: too slow: %d ops: %v; %d ops: %v\n\n%s", - typ, n, t1, 2*n, t2, buf.String())) - } - } -} - -const offset = 251 // known size of runtime hash table - -const profile = false - -func main() { - if profile { - f, err := os.Create("lock.prof") - if err != nil { - log.Fatal(err) - } - pprof.StartCPUProfile(f) - defer pprof.StopCPUProfile() - } - - checkLinear("lockone", 1000, func(n int) { - ch := make(chan int) - locks := make([]sync.RWMutex, offset+1) - for i := 0; i < n; i++ { - go func() { - locks[0].Lock() - ch <- 1 - }() - } - time.Sleep(1 * time.Millisecond) - - go func() { - for j := 0; j < n; j++ { - locks[1].Lock() - locks[offset].Lock() - locks[1].Unlock() - runtime.Gosched() - locks[offset].Unlock() - } - }() - - for j := 0; j < n; j++ { - locks[1].Lock() - locks[offset].Lock() - locks[1].Unlock() - runtime.Gosched() - locks[offset].Unlock() - } - - for i := 0; i < n; i++ { - <-ch - locks[0].Unlock() - } - }) - - if runtime.GOARCH == "arm" && os.Getenv("GOARM") == "5" { - // lockmany reliably fails on the linux-arm-arm5spacemonkey - // builder. See https://golang.org/issue/24221. - return - } - - checkLinear("lockmany", 1000, func(n int) { - locks := make([]sync.RWMutex, n*offset+1) - - var wg sync.WaitGroup - for i := 0; i < n; i++ { - wg.Add(1) - go func(i int) { - locks[(i+1)*offset].Lock() - wg.Done() - locks[(i+1)*offset].Lock() - locks[(i+1)*offset].Unlock() - }(i) - } - wg.Wait() - - go func() { - for j := 0; j < n; j++ { - locks[1].Lock() - locks[0].Lock() - locks[1].Unlock() - runtime.Gosched() - locks[0].Unlock() - } - }() - - for j := 0; j < n; j++ { - locks[1].Lock() - locks[0].Lock() - locks[1].Unlock() - runtime.Gosched() - locks[0].Unlock() - } - - for i := 0; i < n; i++ { - locks[(i+1)*offset].Unlock() - } - }) -}