From e32cfb196103ccfbebc88661d9d6b2b3469caa5f Mon Sep 17 00:00:00 2001 From: Jacky Wang Date: Wed, 1 Jul 2020 18:22:23 -0700 Subject: [PATCH 1/5] Fixed the toggle method, updated test cases --- bool.go | 49 +++++++++++++++++++++++++++-------------- bool_test.go | 61 +++++++++++++++++++++------------------------------- 2 files changed, 58 insertions(+), 52 deletions(-) diff --git a/bool.go b/bool.go index 3a3ca64..7e2330f 100644 --- a/bool.go +++ b/bool.go @@ -4,6 +4,13 @@ package abool import "sync/atomic" +// AtomicBool is an atomic Boolean +// Its methods are all atomic, thus safe to be called by +// multiple goroutines simultaneously +// Note: When embedding into a struct, one should always use +// *AtomicBool to avoid copy +type AtomicBool int32 + // New creates an AtomicBool with default to false func New() *AtomicBool { return new(AtomicBool) @@ -18,13 +25,6 @@ func NewBool(ok bool) *AtomicBool { return ab } -// AtomicBool is an atomic Boolean -// Its methods are all atomic, thus safe to be called by -// multiple goroutines simultaneously -// Note: When embedding into a struct, one should always use -// *AtomicBool to avoid copy -type AtomicBool int32 - // Set sets the Boolean to true func (ab *AtomicBool) Set() { atomic.StoreInt32((*int32)(ab), 1) @@ -37,21 +37,22 @@ func (ab *AtomicBool) UnSet() { // IsSet returns whether the Boolean is true func (ab *AtomicBool) IsSet() bool { - return atomic.LoadInt32((*int32)(ab)) == 1 + return intToBool(atomic.LoadInt32((*int32)(ab))) } // SetTo sets the boolean with given Boolean. func (ab *AtomicBool) SetTo(yes bool) { - if yes { - atomic.StoreInt32((*int32)(ab), 1) - } else { - atomic.StoreInt32((*int32)(ab), 0) - } + atomic.StoreInt32((*int32)(ab), boolToInt(yes)) } -// Toggle negates boolean atomically and returns a new AtomicBool object which holds previous boolean value. -func (ab *AtomicBool) Toggle() *AtomicBool { - return NewBool(atomic.AddInt32((*int32)(ab), 1)&1 == 0) +// Toggle negates boolean atomically and returns the previous boolean value. +func (ab *AtomicBool) Toggle() bool { + for { + old := ab.IsSet() + if ab.SetToIf(old, !old) { + return old + } + } } // SetToIf sets the Boolean to new only if the Boolean matches the old @@ -66,3 +67,19 @@ func (ab *AtomicBool) SetToIf(old, new bool) (set bool) { } return atomic.CompareAndSwapInt32((*int32)(ab), o, n) } + +// boolToInt convert a boolean to int32 +func boolToInt(b bool) int32 { + if b { + return 1 + } + return 0 +} + +// intToBool convert a int32 to boolean +func intToBool(i int32) bool { + if i == 1 { + return true + } + return false +} diff --git a/bool_test.go b/bool_test.go index 18760b0..cf698a0 100644 --- a/bool_test.go +++ b/bool_test.go @@ -55,54 +55,44 @@ func TestBool(t *testing.T) { t.Fatal("Empty value of AtomicBool should be false") } - _ = v.Toggle() + v.Toggle() if !v.IsSet() { t.Fatal("AtomicBool.Toggle() to true failed") } prev := v.Toggle() - if v.IsSet() == prev.IsSet() { + if v.IsSet() == prev { t.Fatal("AtomicBool.Toggle() to false failed") } } func TestRace(t *testing.T) { + v := New() + fs := []func() { + func() {v.IsSet()}, + v.Set, + v.UnSet, + func() {v.Toggle()}, + func() {v.SetToIf(true, false)}, + func() {v.SetToIf(false, true)}, + func() {v.SetTo(true)}, + func() {v.SetTo(false)}, + } + grPerFunc := 10 repeat := 10000 var wg sync.WaitGroup - wg.Add(repeat * 4) - v := New() - - // Writer - go func() { - for i := 0; i < repeat; i++ { - v.Set() - wg.Done() + wg.Add(grPerFunc * len(fs)) + + for _, f := range fs { + for grIndex := 0; grIndex != grPerFunc; grIndex++{ + go func(testFunc func()) { + for i := 0; i != repeat; i++ { + testFunc() + } + wg.Done() + }(f) } - }() - - // Reader - go func() { - for i := 0; i < repeat; i++ { - v.IsSet() - wg.Done() - } - }() - - // Writer - go func() { - for i := 0; i < repeat; i++ { - v.UnSet() - wg.Done() - } - }() - - // Reader And Writer - go func() { - for i := 0; i < repeat; i++ { - v.Toggle() - wg.Done() - } - }() + } wg.Wait() } @@ -177,7 +167,6 @@ func BenchmarkAtomicBoolWrite(b *testing.B) { } // Benchmark CAS - func BenchmarkMutexCAS(b *testing.B) { var m sync.RWMutex var v bool From 3b7370bd2d08b693333ac77f7d90331dc0b2960e Mon Sep 17 00:00:00 2001 From: Jacky Wang Date: Wed, 1 Jul 2020 18:38:00 -0700 Subject: [PATCH 2/5] optimized toggle performance --- bool.go | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/bool.go b/bool.go index 7e2330f..c3b735d 100644 --- a/bool.go +++ b/bool.go @@ -48,9 +48,9 @@ func (ab *AtomicBool) SetTo(yes bool) { // Toggle negates boolean atomically and returns the previous boolean value. func (ab *AtomicBool) Toggle() bool { for { - old := ab.IsSet() - if ab.SetToIf(old, !old) { - return old + old := atomic.LoadInt32((*int32)(ab)) + if atomic.CompareAndSwapInt32((*int32)(ab), old, toggleInt(old)) { + return old == 1 } } } @@ -78,8 +78,13 @@ func boolToInt(b bool) int32 { // intToBool convert a int32 to boolean func intToBool(i int32) bool { + return i == 1 +} + +// toggleInt toggles the int32 +func toggleInt(i int32) int32 { if i == 1 { - return true + return 0 } - return false + return 1 } From c3da033981cfa0da808738d1a58a3c2c22756c96 Mon Sep 17 00:00:00 2001 From: Jacky Wang Date: Wed, 1 Jul 2020 18:43:29 -0700 Subject: [PATCH 3/5] refactored some bool impl --- bool.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/bool.go b/bool.go index c3b735d..bdde321 100644 --- a/bool.go +++ b/bool.go @@ -58,13 +58,8 @@ func (ab *AtomicBool) Toggle() bool { // SetToIf sets the Boolean to new only if the Boolean matches the old // Returns whether the set was done func (ab *AtomicBool) SetToIf(old, new bool) (set bool) { - var o, n int32 - if old { - o = 1 - } - if new { - n = 1 - } + o := boolToInt(old) + n := boolToInt(new) return atomic.CompareAndSwapInt32((*int32)(ab), o, n) } From f6f76c439a011f38076398870932514e881ec23c Mon Sep 17 00:00:00 2001 From: Jacky Wang Date: Wed, 1 Jul 2020 18:47:10 -0700 Subject: [PATCH 4/5] updated readme --- README.md | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 2295c0a..1fe5e1e 100644 --- a/README.md +++ b/README.md @@ -31,25 +31,27 @@ type Foo struct { ## Benchmark: -- Go 1.11.5 -- OS X 10.14.5 +- Go 1.14.2 +- OS X 10.15.5 ```shell +1 ns/op # Read -BenchmarkMutexRead-4 100000000 14.7 ns/op -BenchmarkAtomicValueRead-4 2000000000 0.45 ns/op -BenchmarkAtomicBoolRead-4 2000000000 0.35 ns/op # <--- This package +BenchmarkMutexRead-12 100000000 11.0 ns/op +BenchmarkAtomicValueRead-12 1000000000 0.253 ns/op +BenchmarkAtomicBoolRead-12 1000000000 0.259 ns/op # <--- This package # Write -BenchmarkMutexWrite-4 100000000 14.5 ns/op -BenchmarkAtomicValueWrite-4 100000000 10.5 ns/op -BenchmarkAtomicBoolWrite-4 300000000 5.21 ns/op # <--- This package +BenchmarkMutexWrite-12 100000000 10.8 ns/op +BenchmarkAtomicValueWrite-12 132855918 9.12 ns/op +BenchmarkAtomicBoolWrite-12 263941647 4.52 ns/op # <--- This package # CAS -BenchmarkMutexCAS-4 50000000 31.3 ns/op -BenchmarkAtomicBoolCAS-4 200000000 7.18 ns/op # <--- This package +BenchmarkMutexCAS-12 54871387 21.6 ns/op +BenchmarkAtomicBoolCAS-12 267147930 4.50 ns/op # <--- This package # Toggle -BenchmarkMutexToggle-4 50000000 32.6 ns/op -BenchmarkAtomicBoolToggle-4 300000000 5.21 ns/op # <--- This package +BenchmarkMutexToggle-12 55389297 21.4 ns/op +BenchmarkAtomicBoolToggle-12 145680895 8.32 ns/op # <--- This package + ``` From 3b84aab116f0a1596ebef03bad909f8489d7f80c Mon Sep 17 00:00:00 2001 From: Jacky Wang Date: Wed, 1 Jul 2020 18:47:31 -0700 Subject: [PATCH 5/5] fix typo in readme --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 1fe5e1e..9f8e854 100644 --- a/README.md +++ b/README.md @@ -35,7 +35,6 @@ type Foo struct { - OS X 10.15.5 ```shell -1 ns/op # Read BenchmarkMutexRead-12 100000000 11.0 ns/op BenchmarkAtomicValueRead-12 1000000000 0.253 ns/op