Just FYI, there is currently the following activity regarding sync.Pool optimizations that may land to go1.11:
- Increasing per-P pool capacity, which should reduce lock contention on shared items. https://go-review.googlesource.com/c/go/+/49110 . - Avoiding pool resets during GC - https://github.com/golang/go/issues/22950 . On Thursday, February 15, 2018 at 1:56:34 AM UTC+2, Carlo Alberto Ferraris wrote: > > In an attempt to reduce the time pprof says is spent in sync.Pool > (internal workloads, sorry) I modified Get and getSlow to skip locking the > per-P shared pools if the pools are likely to be empty. This yields > promising results, but I'm not sure the approach is sound since the check I > do is inherently racy. > > As a (artificial and contrived) benchmark, I'm using this: > > func BenchmarkPoolUnderflow(b *testing.B) { > var p Pool > b.RunParallel(func(pb *testing.PB) { > for pb.Next() { > p.Put(1) > p.Get() > p.Get() > } > }) > } > > This is meant to simulate a pool in which more or objects are Get() than > are Put() (it wouldn't make much sense to simulate a pool in which we only > get, so to keep things simple I opted for a 2:1 ratio) > > The change I applied to Get and getSlow is the following. Starting from > the current pattern of: > > l := ... # per-P poolLocal > l.Lock() > last := len(l.shared) - 1 > if last >= 0 { > x = l.shared[last] > l.shared = l.shared[:last] > } > l.Unlock() > > I add a check (not protected by the mutex, that is the expensive op we're > trying to skip if it's not necessary) to see if the pool is likely to be > non-empty: > > l := ... # per-P poolLocal > if len(l.shared) > 0 { # the racy check for non-emptiness > l.Lock() > last := len(l.shared) - 1 > if last >= 0 { > x = l.shared[last] > l.shared = l.shared[:last] > } > l.Unlock() > } > > I know I should not call this a benign race, but in this case I don't see > how this can lead to problems. If the racy check gets it right, then it's > almost a net win. If if it gets it wrong, either we do what we do now (i.e. > we lock, just to find an empty pool), or we skip an otherwise non-empty > pool - thereby failing to immediately return an otherwise reusable object > (note that 1. there is a per-P shared pool for each P, so I'd estimate the > chances of this happening on all of them to be pretty low and 2. the Pool > documentation explicitly say that Get is allowed to treat the pool as > empty). Also note that the race detector is already explicitly disabled in > all sync.Pool methods. > > The reason I'm asking is to understand whether my reasoning is sound and, > regardless, if anybody has suggestions about how to do this in a better way. > > The current results (of the approach above, plus some refactoring to > recover some lost performance on the other benchmarks) on my laptop are the > following: > > name old time/op new time/op delta > Pool-4 14.5ns ± 3% 14.2ns ± 2% -1.64% (p=0.023 n=9+10) > PoolOverflow-4 1.99µs ±12% 1.78µs ± 1% -10.62% (p=0.000 n=10+8) > PoolUnderflow-4 152ns ± 6% 30ns ± 1% -80.00% (p=0.000 n=10+8) > > (the first two benchmarks are already part of sync.Pool, the last one is > the one I described above) > > Any feedback is welcome. If this is deemed safe I'm going to submit a CL. > > Carlo > > > -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
