]> Cypherpunks repositories - gostls13.git/commitdiff
sync: move lock linearity test and treat it like a performance test
authorMichael Anthony Knyszek <mknyszek@google.com>
Fri, 10 Jun 2022 16:21:46 +0000 (16:21 +0000)
committerMichael Knyszek <mknyszek@google.com>
Mon, 13 Jun 2022 20:15:55 +0000 (20:15 +0000)
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 <mpratt@google.com>
src/internal/testenv/testenv.go
src/internal/testmath/bench.go [new file with mode: 0644]
src/sync/mutex_test.go
test/locklinear.go [deleted file]

index 1feb630cf5fa38e137c0e1a9c1267d1a79b37dbd..b7cb95063b21dd3f5046e16db2d67a4e69ec8c7d 100644 (file)
@@ -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 (file)
index 0000000..6f034b4
--- /dev/null
@@ -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()
+}
index cca0986a30975ebd9c51cf37d97037b43b666dae..9a4187c672651962f77402771a8f35b782941107 100644 (file)
@@ -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 (file)
index 54e40a5..0000000
+++ /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()
-               }
-       })
-}