From 822a647018adbd994114cb0dc8932f2853b805aa Mon Sep 17 00:00:00 2001 From: Jamie Liu Date: Fri, 5 Nov 2021 00:49:55 -0700 Subject: Fix unfair comparison to unbuffered channels in sleep_test.go. Consider the following benchmark, which is equivalent to BenchmarkSleeperWaitOnSingleSelect with names changed to more closely reflect the behavior of BenchmarkGoWaitOnSingleSelect: var ( empty Sleeper emptyCond Waker full Sleeper fullCond Waker ) empty.AddWaker(&emptyCond) full.AddWaker(&fullCond) go func() { for i := 0; i < b.N; i++ { empty.Fetch(true) fullCond.Assert() } }() for i := 0; i < b.N; i++ { emptyCond.Assert() full.Fetch(true) } The unfairness arises because runtime.chansend and runtime.chanrecv don't actually work this way. If runtime.chansend blocks, it has already enqueued the element to be sent on runtime.hchan.sendq, which runtime.chanrecv dequeues before calling goready(); in sleep-like terms, by the time empty.Fetch() returns, fullCond.Assert() has already happened and been fetched by the other goroutine. The same property applies to runtime.chanrecv/runtime.hchan.recvq. This property has no correspondence to the actual usage of the sleep package, so change the channel benchmarks to explicitly exchange control using buffered channels instead. Also remove some stale comments and align the syncevent benchmarks with the sleep ones. BenchmarkSleeperWaitOnSingleSelect BenchmarkSleeperWaitOnSingleSelect-12 2118603 472.5 ns/op BenchmarkGoWaitOnSingleSelect BenchmarkGoWaitOnSingleSelect-12 2224262 517.7 ns/op BenchmarkSleeperWaitOnMultiSelect BenchmarkSleeperWaitOnMultiSelect-12 2630569 459.8 ns/op BenchmarkGoWaitOnMultiSelect BenchmarkGoWaitOnMultiSelect-12 807918 1312 ns/op BenchmarkWaiterPingPong BenchmarkWaiterPingPong-12 2955579 385.8 ns/op BenchmarkSleeperPingPong BenchmarkSleeperPingPong-12 2454367 474.3 ns/op BenchmarkChannelPingPong BenchmarkChannelPingPong-12 2302662 513.5 ns/op BenchmarkWaiterPingPongMulti BenchmarkWaiterPingPongMulti-12 3023676 388.8 ns/op BenchmarkSleeperPingPongMulti BenchmarkSleeperPingPongMulti-12 2574064 471.5 ns/op BenchmarkChannelPingPongMulti BenchmarkChannelPingPongMulti-12 1000000 1088 ns/op PiperOrigin-RevId: 407760956 --- pkg/sleep/sleep_test.go | 32 ++++--- pkg/syncevent/waiter_test.go | 205 +++++++++++++++++++++++++++++-------------- 2 files changed, 158 insertions(+), 79 deletions(-) diff --git a/pkg/sleep/sleep_test.go b/pkg/sleep/sleep_test.go index b27feb99b..f4af9d710 100644 --- a/pkg/sleep/sleep_test.go +++ b/pkg/sleep/sleep_test.go @@ -489,9 +489,7 @@ func BenchmarkGoAssertNonWaiting(b *testing.B) { } // BenchmarkSleeperWaitOnSingleSelect measures how long it takes to wait on one -// waker channel while another goroutine wakes up the sleeper. This assumes that -// a new goroutine doesn't run immediately (i.e., the creator of a new goroutine -// is allowed to go to sleep before the new goroutine has a chance to run). +// waker channel while another goroutine wakes up the sleeper. func BenchmarkSleeperWaitOnSingleSelect(b *testing.B) { var ( s Sleeper @@ -514,25 +512,24 @@ func BenchmarkSleeperWaitOnSingleSelect(b *testing.B) { } // BenchmarkGoWaitOnSingleSelect measures how long it takes to wait on one -// channel while another goroutine wakes up the sleeper. This assumes that a new -// goroutine doesn't run immediately (i.e., the creator of a new goroutine is -// allowed to go to sleep before the new goroutine has a chance to run). +// channel while another goroutine wakes up the sleeper. func BenchmarkGoWaitOnSingleSelect(b *testing.B) { - ch := make(chan struct{}) + ch := make(chan struct{}, 1) + nch := make(chan struct{}, 1) go func() { for i := 0; i < b.N; i++ { + <-nch ch <- struct{}{} } }() for i := 0; i < b.N; i++ { + nch <- struct{}{} <-ch } } // BenchmarkSleeperWaitOnMultiSelect measures how long it takes to wait on 4 -// wakers while another goroutine wakes up the sleeper. This assumes that a new -// goroutine doesn't run immediately (i.e., the creator of a new goroutine is -// allowed to go to sleep before the new goroutine has a chance to run). +// wakers while another goroutine wakes up the sleeper. func BenchmarkSleeperWaitOnMultiSelect(b *testing.B) { const count = 4 var ( @@ -560,23 +557,30 @@ func BenchmarkSleeperWaitOnMultiSelect(b *testing.B) { } // BenchmarkGoWaitOnMultiSelect measures how long it takes to wait on 4 channels -// while another goroutine wakes up the sleeper. This assumes that a new -// goroutine doesn't run immediately (i.e., the creator of a new goroutine is -// allowed to go to sleep before the new goroutine has a chance to run). +// while another goroutine wakes up the sleeper. func BenchmarkGoWaitOnMultiSelect(b *testing.B) { const count = 4 ch := make([]chan struct{}, count) + nch := make([]chan struct{}, count) for i := range ch { - ch[i] = make(chan struct{}) + ch[i] = make(chan struct{}, 1) + nch[i] = make(chan struct{}, 1) } b.ResetTimer() go func() { for i := 0; i < b.N; i++ { + select { + case <-nch[0]: + case <-nch[1]: + case <-nch[2]: + case <-nch[3]: + } ch[count-1] <- struct{}{} } }() for i := 0; i < b.N; i++ { + nch[count-1] <- struct{}{} select { case <-ch[0]: case <-ch[1]: diff --git a/pkg/syncevent/waiter_test.go b/pkg/syncevent/waiter_test.go index cfa0972c0..428b20d0d 100644 --- a/pkg/syncevent/waiter_test.go +++ b/pkg/syncevent/waiter_test.go @@ -15,6 +15,7 @@ package syncevent import ( + "fmt" "sync/atomic" "testing" "time" @@ -303,112 +304,186 @@ func BenchmarkChannelNotifyWaitMultiAck(b *testing.B) { } } -// BenchmarkXxxNotifyAsyncWaitAck measures how long it takes to wait for an -// event while another goroutine signals the event. This assumes that a new -// goroutine doesn't run immediately (i.e. the creator of a new goroutine is -// allowed to go to sleep before the new goroutine has a chance to run). +// BenchmarkXxxPingPong exchanges control between two goroutines. -func BenchmarkWaiterNotifyAsyncWaitAck(b *testing.B) { - var w Waiter - w.Init() +func BenchmarkWaiterPingPong(b *testing.B) { + var w1, w2 Waiter + w1.Init() + w2.Init() + var wg sync.WaitGroup + defer wg.Wait() + w1.Notify(evBench) b.ResetTimer() + go func() { + for i := 0; i < b.N; i++ { + w1.Wait() + w1.Ack(evBench) + w2.Notify(evBench) + } + }() for i := 0; i < b.N; i++ { - go func() { - w.Notify(1) - }() - w.Wait() - w.Ack(evBench) + w2.Wait() + w2.Ack(evBench) + w1.Notify(evBench) } } -func BenchmarkSleeperNotifyAsyncWaitAck(b *testing.B) { - var s sleep.Sleeper - var w sleep.Waker - s.AddWaker(&w) - +func BenchmarkSleeperPingPong(b *testing.B) { + var ( + s1 sleep.Sleeper + w1 sleep.Waker + s2 sleep.Sleeper + w2 sleep.Waker + ) + s1.AddWaker(&w1) + s2.AddWaker(&w2) + var wg sync.WaitGroup + defer wg.Wait() + + w1.Assert() + wg.Add(1) b.ResetTimer() + go func() { + defer wg.Done() + for i := 0; i < b.N; i++ { + s1.Fetch(true) + w2.Assert() + } + }() for i := 0; i < b.N; i++ { - go func() { - w.Assert() - }() - s.Fetch(true) + s2.Fetch(true) + w1.Assert() } } -func BenchmarkChannelNotifyAsyncWaitAck(b *testing.B) { - ch := make(chan struct{}, 1) +func BenchmarkChannelPingPong(b *testing.B) { + ch1 := make(chan struct{}, 1) + ch2 := make(chan struct{}, 1) + var wg sync.WaitGroup + defer wg.Wait() + ch1 <- struct{}{} + wg.Add(1) b.ResetTimer() + go func() { + defer wg.Done() + for i := 0; i < b.N; i++ { + <-ch1 + ch2 <- struct{}{} + } + }() for i := 0; i < b.N; i++ { - go func() { - select { - case ch <- struct{}{}: - default: - } - }() - <-ch + <-ch2 + ch1 <- struct{}{} } } -// BenchmarkXxxNotifyAsyncWaitMultiAck is equivalent to NotifyAsyncWaitAck, but -// allows for multiple event sources. +// BenchmarkXxxPingPongMulti is equivalent to PingPong, but allows each +// goroutine to receive from multiple event sources (although only one is ever +// signaled). -func BenchmarkWaiterNotifyAsyncWaitMultiAck(b *testing.B) { - var w Waiter - w.Init() +func BenchmarkWaiterPingPongMulti(b *testing.B) { + var w1, w2 Waiter + w1.Init() + w2.Init() + var wg sync.WaitGroup + defer wg.Wait() + w1.Notify(evBench) + wg.Add(1) b.ResetTimer() + go func() { + defer wg.Done() + for i := 0; i < b.N; i++ { + if e := w1.Wait(); e != evBench { + // b.Fatalf() can only be called from the main goroutine. + panic(fmt.Sprintf("Wait: got %#x, wanted %#x", e, evBench)) + } + w1.Ack(evBench) + w2.Notify(evBench) + } + }() for i := 0; i < b.N; i++ { - go func() { - w.Notify(evBench) - }() - if e := w.Wait(); e != evBench { + if e := w2.Wait(); e != evBench { b.Fatalf("Wait: got %#x, wanted %#x", e, evBench) } - w.Ack(evBench) + w2.Ack(evBench) + w1.Notify(evBench) } } -func BenchmarkSleeperNotifyAsyncWaitMultiAck(b *testing.B) { - var s sleep.Sleeper - var ws [3]sleep.Waker - for i := range ws { - s.AddWaker(&ws[i]) - } - +func BenchmarkSleeperPingPongMulti(b *testing.B) { + var ( + s1 sleep.Sleeper + w1, w1a, w1b sleep.Waker + s2 sleep.Sleeper + w2, w2a, w2b sleep.Waker + ) + s1.AddWaker(&w1) + s1.AddWaker(&w1a) + s1.AddWaker(&w1b) + s2.AddWaker(&w2) + s2.AddWaker(&w2a) + s2.AddWaker(&w2b) + var wg sync.WaitGroup + defer wg.Wait() + + w1.Assert() + wg.Add(1) b.ResetTimer() + go func() { + defer wg.Done() + for i := 0; i < b.N; i++ { + if w := s1.Fetch(true); w != &w1 { + // b.Fatalf() can only be called from the main goroutine. + panic(fmt.Sprintf("Fetch: got %p, wanted %p", w, &w1)) + } + w2.Assert() + } + }() for i := 0; i < b.N; i++ { - go func() { - ws[0].Assert() - }() - if v := s.Fetch(true); v != &ws[0] { - b.Fatalf("Fetch: got %v, expected %v", v, &ws[0]) + if w := s2.Fetch(true); w != &w2 { + b.Fatalf("Fetch: got %p, wanted %p", w, &w2) } + w1.Assert() } } -func BenchmarkChannelNotifyAsyncWaitMultiAck(b *testing.B) { - ch0 := make(chan struct{}, 1) +func BenchmarkChannelPingPongMulti(b *testing.B) { ch1 := make(chan struct{}, 1) + ch1a := make(chan struct{}, 1) + ch1b := make(chan struct{}, 1) ch2 := make(chan struct{}, 1) + ch2a := make(chan struct{}, 1) + ch2b := make(chan struct{}, 1) + var wg sync.WaitGroup + defer wg.Wait() + ch1 <- struct{}{} + wg.Add(1) b.ResetTimer() - for i := 0; i < b.N; i++ { - go func() { + go func() { + defer wg.Done() + for i := 0; i < b.N; i++ { select { - case ch0 <- struct{}{}: - default: + case <-ch1: + case <-ch1a: + panic("received from ch1a") + case <-ch1b: + panic("received from ch1a") } - }() - + ch2 <- struct{}{} + } + }() + for i := 0; i < b.N; i++ { select { - case <-ch0: - // ok - case <-ch1: - b.Fatalf("received from ch1") case <-ch2: - b.Fatalf("received from ch2") + case <-ch2a: + panic("received from ch2a") + case <-ch2b: + panic("received from ch2a") } + ch1 <- struct{}{} } } -- cgit v1.2.3