Skip to content

Commit ee833ed

Browse files
committed
sync: use atomic.Uint64 for WaitGroup state
So it's guaranteed to have 64-bit alignment, simplify the code without losing any performance: name old time/op new time/op delta WaitGroupUncontended-8 3.84ns ± 2% 3.82ns ± 1% ~ (p=0.159 n=10+10) WaitGroupAddDone-8 33.2ns ± 3% 33.0ns ± 3% ~ (p=0.564 n=9+10) WaitGroupAddDoneWork-8 39.3ns ± 1% 39.3ns ± 1% ~ (p=1.000 n=8+9) WaitGroupWait-8 0.70ns ± 3% 0.70ns ± 2% ~ (p=0.720 n=9+10) WaitGroupWaitWork-8 7.93ns ± 1% 7.99ns ± 3% ~ (p=0.271 n=10+10) WaitGroupActuallyWait-8 135ns ± 2% 135ns ± 1% ~ (p=0.897 n=10+10) Change-Id: I446b53fa92873419aadd592f45e51398f8ad8652 Reviewed-on: https://go-review.googlesource.com/c/go/+/424835 TryBot-Result: Gopher Robot <[email protected]> Reviewed-by: Keith Randall <[email protected]> Reviewed-by: Cherry Mui <[email protected]> Run-TryBot: Cuong Manh Le <[email protected]>
1 parent e51b3ae commit ee833ed

File tree

1 file changed

+12
-35
lines changed

1 file changed

+12
-35
lines changed

src/sync/waitgroup.go

Lines changed: 12 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -23,27 +23,8 @@ import (
2323
type WaitGroup struct {
2424
noCopy noCopy
2525

26-
// 64-bit value: high 32 bits are counter, low 32 bits are waiter count.
27-
// 64-bit atomic operations require 64-bit alignment, but 32-bit
28-
// compilers only guarantee that 64-bit fields are 32-bit aligned.
29-
// For this reason on 32 bit architectures we need to check in state()
30-
// if state1 is aligned or not, and dynamically "swap" the field order if
31-
// needed.
32-
state1 uint64
33-
state2 uint32
34-
}
35-
36-
// state returns pointers to the state and sema fields stored within wg.state*.
37-
func (wg *WaitGroup) state() (statep *uint64, semap *uint32) {
38-
if unsafe.Alignof(wg.state1) == 8 || uintptr(unsafe.Pointer(&wg.state1))%8 == 0 {
39-
// state1 is 64-bit aligned: nothing to do.
40-
return &wg.state1, &wg.state2
41-
} else {
42-
// state1 is 32-bit aligned but not 64-bit aligned: this means that
43-
// (&state1)+4 is 64-bit aligned.
44-
state := (*[3]uint32)(unsafe.Pointer(&wg.state1))
45-
return (*uint64)(unsafe.Pointer(&state[1])), &state[0]
46-
}
26+
state atomic.Uint64 // high 32 bits are counter, low 32 bits are waiter count.
27+
sema uint32
4728
}
4829

4930
// Add adds delta, which may be negative, to the WaitGroup counter.
@@ -60,24 +41,22 @@ func (wg *WaitGroup) state() (statep *uint64, semap *uint32) {
6041
// new Add calls must happen after all previous Wait calls have returned.
6142
// See the WaitGroup example.
6243
func (wg *WaitGroup) Add(delta int) {
63-
statep, semap := wg.state()
6444
if race.Enabled {
65-
_ = *statep // trigger nil deref early
6645
if delta < 0 {
6746
// Synchronize decrements with Wait.
6847
race.ReleaseMerge(unsafe.Pointer(wg))
6948
}
7049
race.Disable()
7150
defer race.Enable()
7251
}
73-
state := atomic.AddUint64(statep, uint64(delta)<<32)
52+
state := wg.state.Add(uint64(delta) << 32)
7453
v := int32(state >> 32)
7554
w := uint32(state)
7655
if race.Enabled && delta > 0 && v == int32(delta) {
7756
// The first increment must be synchronized with Wait.
7857
// Need to model this as a read, because there can be
7958
// several concurrent wg.counter transitions from 0.
80-
race.Read(unsafe.Pointer(semap))
59+
race.Read(unsafe.Pointer(&wg.sema))
8160
}
8261
if v < 0 {
8362
panic("sync: negative WaitGroup counter")
@@ -93,13 +72,13 @@ func (wg *WaitGroup) Add(delta int) {
9372
// - Adds must not happen concurrently with Wait,
9473
// - Wait does not increment waiters if it sees counter == 0.
9574
// Still do a cheap sanity check to detect WaitGroup misuse.
96-
if *statep != state {
75+
if wg.state.Load() != state {
9776
panic("sync: WaitGroup misuse: Add called concurrently with Wait")
9877
}
9978
// Reset waiters count to 0.
100-
*statep = 0
79+
wg.state.Store(0)
10180
for ; w != 0; w-- {
102-
runtime_Semrelease(semap, false, 0)
81+
runtime_Semrelease(&wg.sema, false, 0)
10382
}
10483
}
10584

@@ -110,13 +89,11 @@ func (wg *WaitGroup) Done() {
11089

11190
// Wait blocks until the WaitGroup counter is zero.
11291
func (wg *WaitGroup) Wait() {
113-
statep, semap := wg.state()
11492
if race.Enabled {
115-
_ = *statep // trigger nil deref early
11693
race.Disable()
11794
}
11895
for {
119-
state := atomic.LoadUint64(statep)
96+
state := wg.state.Load()
12097
v := int32(state >> 32)
12198
w := uint32(state)
12299
if v == 0 {
@@ -128,16 +105,16 @@ func (wg *WaitGroup) Wait() {
128105
return
129106
}
130107
// Increment waiters count.
131-
if atomic.CompareAndSwapUint64(statep, state, state+1) {
108+
if wg.state.CompareAndSwap(state, state+1) {
132109
if race.Enabled && w == 0 {
133110
// Wait must be synchronized with the first Add.
134111
// Need to model this is as a write to race with the read in Add.
135112
// As a consequence, can do the write only for the first waiter,
136113
// otherwise concurrent Waits will race with each other.
137-
race.Write(unsafe.Pointer(semap))
114+
race.Write(unsafe.Pointer(&wg.sema))
138115
}
139-
runtime_Semacquire(semap)
140-
if *statep != 0 {
116+
runtime_Semacquire(&wg.sema)
117+
if wg.state.Load() != 0 {
141118
panic("sync: WaitGroup is reused before previous Wait has returned")
142119
}
143120
if race.Enabled {

0 commit comments

Comments
 (0)