From 0ee5479199394b4be8c856094e18ba7d14ccfaf5 Mon Sep 17 00:00:00 2001 From: irfan sharif Date: Mon, 15 Aug 2022 18:34:35 -0400 Subject: [PATCH] grunning: improve some tests Release note: None Release justification: test-only changes --- pkg/util/grunning/BUILD.bazel | 39 ---------------------- pkg/util/grunning/enabled_test.go | 54 +++++++++++++++---------------- 2 files changed, 26 insertions(+), 67 deletions(-) diff --git a/pkg/util/grunning/BUILD.bazel b/pkg/util/grunning/BUILD.bazel index 9fe68e765990..ccbd400fd10c 100644 --- a/pkg/util/grunning/BUILD.bazel +++ b/pkg/util/grunning/BUILD.bazel @@ -22,49 +22,41 @@ go_test( "@io_bazel_rules_go//go/platform:aix_ppc64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:android_386": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:android_amd64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:android_arm": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:android_arm64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:darwin_386": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:darwin_amd64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:darwin_arm": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:darwin_arm64": [ @@ -74,7 +66,6 @@ go_test( "@io_bazel_rules_go//go/platform:dragonfly_amd64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:freebsd_386": [ @@ -96,13 +87,11 @@ go_test( "@io_bazel_rules_go//go/platform:illumos_amd64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:ios_amd64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:ios_arm64": [ @@ -112,169 +101,141 @@ go_test( "@io_bazel_rules_go//go/platform:js_wasm": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux_386": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux_amd64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux_arm": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux_arm64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux_mips": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux_mips64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux_mips64le": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux_mipsle": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux_ppc64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux_ppc64le": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux_riscv64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:linux_s390x": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:netbsd_386": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:netbsd_amd64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:netbsd_arm": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:netbsd_arm64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:openbsd_386": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:openbsd_amd64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:openbsd_arm": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:openbsd_arm64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:plan9_386": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:plan9_amd64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:plan9_arm": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:solaris_amd64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:windows_386": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:windows_amd64": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "@io_bazel_rules_go//go/platform:windows_arm": [ ":grunning", "//pkg/testutils/skip", - "//pkg/util/syncutil", "@com_github_stretchr_testify//require", ], "//conditions:default": [], diff --git a/pkg/util/grunning/enabled_test.go b/pkg/util/grunning/enabled_test.go index 068a86c71e95..649a6f202236 100644 --- a/pkg/util/grunning/enabled_test.go +++ b/pkg/util/grunning/enabled_test.go @@ -20,12 +20,10 @@ package grunning_test import ( "runtime" "sync" - "sync/atomic" "testing" "github.com/cockroachdb/cockroach/pkg/testutils/skip" "github.com/cockroachdb/cockroach/pkg/util/grunning" - "github.com/cockroachdb/cockroach/pkg/util/syncutil" "github.com/stretchr/testify/require" ) @@ -46,13 +44,7 @@ func TestEnabled(t *testing.T) { func TestEquivalentGoroutines(t *testing.T) { skip.UnderStress(t, "not applicable") - mu := struct { - syncutil.Mutex - nanos map[int]int64 - }{} - mu.nanos = make(map[int]int64) - - f := func(wg *sync.WaitGroup, id int) { + f := func(wg *sync.WaitGroup, result *int64) { defer wg.Done() var sum int @@ -64,31 +56,27 @@ func TestEquivalentGoroutines(t *testing.T) { } nanos := grunning.Time().Nanoseconds() - mu.Lock() - mu.nanos[id] = nanos - mu.Unlock() + *result = nanos } const threads = 10 var wg sync.WaitGroup + results := make([]int64, threads) for i := 0; i < threads; i++ { i := i // copy loop variable wg.Add(1) - go f(&wg, i) + go f(&wg, &results[i]) } wg.Wait() - mu.Lock() - defer mu.Unlock() - total := int64(0) - for _, nanos := range mu.nanos { - total += nanos + for _, result := range results { + total += result } exp := 1.0 / threads - for i, nanos := range mu.nanos { - got := float64(nanos) / float64(total) + for i, result := range results { + got := float64(result) / float64(total) t.Logf("thread=%02d expected≈%5.2f%% got=%5.2f%% of on-cpu time", i+1, exp*100, got*100) @@ -112,7 +100,7 @@ func TestProportionalGoroutines(t *testing.T) { } nanos := grunning.Time().Nanoseconds() - atomic.AddInt64(result, nanos) + *result = nanos } results := make([]int64, 10) @@ -144,13 +132,17 @@ func TestProportionalGoroutines(t *testing.T) { } // TestPingPongHog is adapted from a benchmark in the Go runtime, forcing the -// scheduler to continually schedule goroutines. +// scheduler to continually schedule goroutines. It demonstrates that if two +// goroutines alternately cycle between running and waiting, they will get +// similar running times. func TestPingPongHog(t *testing.T) { skip.UnderStress(t, "not applicable") defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(1)) - // Create a CPU hog. + // Create a CPU hog. It makes the two goroutines that want to cycle between + // running and waiting also have to wait in runnable state, until the CPU + // hog is finished with its time slice. stop, done := make(chan bool), make(chan bool) go func() { for { @@ -172,7 +164,7 @@ func TestPingPongHog(t *testing.T) { pong <- <-ping } pingern = grunning.Time().Nanoseconds() - close(stop) + close(stop) // stop the CPU hog done <- true }() go func() { @@ -183,9 +175,9 @@ func TestPingPongHog(t *testing.T) { done <- true }() ping <- true // start ping-pong - <-stop - <-ping // let last ponger exit - <-done // make sure goroutines exit + <-stop // wait until the pinger tells the CPU hog to stop + <-ping // wait for the ponger to finish + <-done // make sure goroutines exit <-done <-done @@ -194,7 +186,13 @@ func TestPingPongHog(t *testing.T) { } // BenchmarkGRunningTime measures how costly it is to read the current -// goroutine's running time. +// goroutine's running time. Results: +// +// goos: linux +// goarch: amd64 +// cpu: Intel(R) Xeon(R) CPU @ 2.20GHz +// BenchmarkGRunningTime +// BenchmarkGRunningTime-24 38336452 31.59 ns/op func BenchmarkGRunningTime(b *testing.B) { for n := 0; n < b.N; n++ { _ = grunning.Time()