DataDog/datadog-go

Use of rand.Float64() reduces parallelism

eliaslevy opened this issue · 6 comments

The global default math/rand generator is protected by a mutex. In a highly concurrent application making lots of calls to the DD library and making use of the rate parameter to the API sample metrics so as to not drop packets and overload the statsd server, the use of rand.Float64 in statsd.Client.send can become a bottleneck.

Ideally, there would be an alternative API that allows the caller to pass in goroutine local math.rand.Rand, which is not protected by a mutex.

arbll commented

Hi @eliaslevy,

This looks like it would be a nice improvement. Having an alternative API would work but I am wondering if using a separate math.rand.Rand for each client instance and creating one client per goroutine wouldn't be cleaner.
WDYT ?

That would work. Although it would require folks using the buffered client to reconfigure the timeout or buffer size value so the system to output roughly the same number of packets as when using a single global client.

I went a somewhat different route. I used sync.Pool to get a per-CPU RNG. The only issue is that per my understanding of sync.Pool, the RNG is destroyed during GC and recreated again and again. But it was a quick hack that did not require any API changes.

arbll commented

Once #108 is merged I should be able to simply add a rand per "worker".

@arbll I opened a PR that uses a *rand.Rand per worker to avoid the math/rand package-global RNG lock. Please check it out here: #178

arbll commented

Hey - I no longer work on this repo
cc @hush-hush

#178 was merged and release (see v4.3.1).