This repository was archived by the owner on Apr 1, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 495
This repository was archived by the owner on Apr 1, 2025. It is now read-only.
Do we expect StandardEWMA.Tick
to be called concurrently? #286
Copy link
Copy link
Open
Description
Do we expect StandardEWMA.Tick
to be called concurrently?
If not, then there's no need to use a mutex and atomic operations on a.init
, just below code is fine:
if a.init == 0 {
atomic.StoreUint64(&a.rate, math.Float64bits(a.fetchInstantRate()))
a.init = 1
} else {
a.updateRate(a.fetchInstantRate())
}
if yes, then the current implementation is incorrect, because, after the current goroutine executed L115, other goroutines could execute L103, which creates a race condition with L116 of the current goroutine, and L103 itself could create race conditions between concurrent goroutines.
Lines 100 to 120 in cf1acfc
func (a *StandardEWMA) Tick() { | |
// Optimization to avoid mutex locking in the hot-path. | |
if atomic.LoadUint32(&a.init) == 1 { | |
a.updateRate(a.fetchInstantRate()) | |
} else { | |
// Slow-path: this is only needed on the first Tick() and preserves transactional updating | |
// of init and rate in the else block. The first conditional is needed below because | |
// a different thread could have set a.init = 1 between the time of the first atomic load and when | |
// the lock was acquired. | |
a.mutex.Lock() | |
if atomic.LoadUint32(&a.init) == 1 { | |
// The fetchInstantRate() uses atomic loading, which is unecessary in this critical section | |
// but again, this section is only invoked on the first successful Tick() operation. | |
a.updateRate(a.fetchInstantRate()) | |
} else { | |
atomic.StoreUint32(&a.init, 1) | |
atomic.StoreUint64(&a.rate, math.Float64bits(a.fetchInstantRate())) | |
} | |
a.mutex.Unlock() | |
} | |
} |
samutamm
Metadata
Metadata
Assignees
Labels
No labels