Please suggest correct / best approaches, solutions to this code :
func main() {
var counter int
var mu sync.Mutex
var wg sync.WaitGroup
wg.Add(1)
go func() {
for i := 1; i <= 3000; i++ {
mu.Lock()
counter++
mu.Unlock()
}
defer wg.Done()
}()
wg.Add(1)
go func() {
for i := 1; i <= 3000; i++ {
mu.Lock()
counter++
mu.Unlock()
}
defer wg.Done()
}()
wg.Add(1)
go func() {
for i := 1; i <= 3000; i++ {
mu.Lock()
counter++
mu.Unlock()
}
defer wg.Done()
}()
wg.Wait()
fmt.Println(counter)
}
I mean - what are you trying to do? In this specific instance, I would say take a look at the sync/atomic package. And specifically atomic.Int32. From the Add method:
Add atomically adds delta to x and returns the new value.
Atomics compile to low-level CPU instructions that are very fast. I believe on x86 this is a single instruction. So, you could refactor your code to this:
package main
import (
"fmt"
"sync"
"sync/atomic"
)
func main() {
var counter atomic.Int32
var wg sync.WaitGroup
const numWorkers, perWorker = 10, 3000
wg.Add(numWorkers)
for w := 0; w < numWorkers; w++ {
go func() {
defer wg.Done()
for i := 1; i <= perWorker; i++ {
counter.Add(1)
}
}()
}
wg.Wait()
fmt.Println(counter.Load())
}
Nary a mutex in sight. OR - if you wanted to further refactor you could have each worker accumulate then send on a channel when complete with whatever unit of work they are doing:
package main
import "fmt"
func main() {
const numWorkers, perWorker = 10, 3000
results := make(chan int, numWorkers)
for w := 0; w < numWorkers; w++ {
go func() {
// Accumulate locally
sum := 0
for i := 1; i <= perWorker; i++ {
sum++
}
// Send our result
results <- sum
}()
}
total := 0
// Grab all results and total them
for i := 0; i < numWorkers; i++ {
total += <-results
}
fmt.Println(total)
}
Your code is race-free: the mutex protects `counter`, and `WaitGroup.Wait()` makes sure both goroutines finish before you read it, so the result will be correct.
Two practical notes:
1. Locking and unlocking on every increment is correct but slow — the mutex becomes the bottleneck. As Dean suggested, `atomic.Int32` is a good fit here, or give each goroutine its own local counter and add the two totals together at the end, so the workers never contend.
2. You can prove it to yourself by running it with the race detector: `go run -race main.go`. If there were a race on `counter`, it would report it, and with the mutex in place it should stay clean.