Merge pull request #1 from JackyWYX/fix_toggle

Fix toggle method

README.md

@@ -31,25 +31,26 @@ type Foo struct {
 
 ## Benchmark:
 
-- Go 1.11.5
-- OS X 10.14.5
+- Go 1.14.2
+- OS X 10.15.5
 
 ```shell
 # 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
+
 ```

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,32 +37,49 @@ 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 := atomic.LoadInt32((*int32)(ab))
+		if atomic.CompareAndSwapInt32((*int32)(ab), old, toggleInt(old)) {
+			return old == 1
+		}
+	}
 }
 
 // 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
+	o := boolToInt(old)
+	n := boolToInt(new)
+	return atomic.CompareAndSwapInt32((*int32)(ab), o, n)
+}
+
+// boolToInt convert a boolean to int32
+func boolToInt(b bool) int32 {
+	if b {
+		return 1
 	}
-	if new {
-		n = 1
+	return 0
+}
+
+// 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 0
 	}
-	return atomic.CompareAndSwapInt32((*int32)(ab), o, n)
+	return 1
 }

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