code review 46010043: sync: scalable Pool

sync: scalable Pool
Introduce fixed-size P-local caches.
When local caches overflow/underflow a batch of items
is transferred to/from global mutex-protected cache.

benchmark                    old ns/op    new ns/op    delta
BenchmarkPool                    50554        22423  -55.65%
BenchmarkPool-4                 400359         5904  -98.53%
BenchmarkPool-16                403311         1598  -99.60%
BenchmarkPool-32                367310         1526  -99.58%

BenchmarkPoolOverlflow            5214         3633  -30.32%
BenchmarkPoolOverlflow-4         42663         9539  -77.64%
BenchmarkPoolOverlflow-8         46919        11385  -75.73%
BenchmarkPoolOverlflow-16        39454        13048  -66.93%

BenchmarkSprintfEmpty                    84           63  -25.68%
BenchmarkSprintfEmpty-2                 371           32  -91.13%
BenchmarkSprintfEmpty-4                 465           22  -95.25%
BenchmarkSprintfEmpty-8                 565           12  -97.77%
BenchmarkSprintfEmpty-16                498            5  -98.87%
BenchmarkSprintfEmpty-32                492            4  -99.04%

BenchmarkSprintfString                  259          229  -11.58%
BenchmarkSprintfString-2                574          144  -74.91%
BenchmarkSprintfString-4                651           77  -88.05%
BenchmarkSprintfString-8                868           47  -94.48%
BenchmarkSprintfString-16               825           33  -95.96%
BenchmarkSprintfString-32               825           30  -96.28%

BenchmarkSprintfInt                     213          188  -11.74%
BenchmarkSprintfInt-2                   448          138  -69.20%
BenchmarkSprintfInt-4                   624           52  -91.63%
BenchmarkSprintfInt-8                   691           31  -95.43%
BenchmarkSprintfInt-16                  724           18  -97.46%
BenchmarkSprintfInt-32                  718           16  -97.70%

BenchmarkSprintfIntInt                  311          282   -9.32%
BenchmarkSprintfIntInt-2                333          145  -56.46%
BenchmarkSprintfIntInt-4                642          110  -82.87%
BenchmarkSprintfIntInt-8                832           42  -94.90%
BenchmarkSprintfIntInt-16               817           24  -97.00%
BenchmarkSprintfIntInt-32               805           22  -97.17%

BenchmarkSprintfPrefixedInt             309          269  -12.94%
BenchmarkSprintfPrefixedInt-2           245          168  -31.43%
BenchmarkSprintfPrefixedInt-4           598           99  -83.36%
BenchmarkSprintfPrefixedInt-8           770           67  -91.23%
BenchmarkSprintfPrefixedInt-16          829           54  -93.49%
BenchmarkSprintfPrefixedInt-32          824           50  -93.83%

BenchmarkSprintfFloat                   418          398   -4.78%
BenchmarkSprintfFloat-2                 295          203  -31.19%
BenchmarkSprintfFloat-4                 585          128  -78.12%
BenchmarkSprintfFloat-8                 873           60  -93.13%
BenchmarkSprintfFloat-16                884           33  -96.24%
BenchmarkSprintfFloat-32                881           29  -96.62%

BenchmarkManyArgs                      1097         1069   -2.55%
BenchmarkManyArgs-2                     705          567  -19.57%
BenchmarkManyArgs-4                     792          319  -59.72%
BenchmarkManyArgs-8                     963          172  -82.14%
BenchmarkManyArgs-16                   1115          103  -90.76%
BenchmarkManyArgs-32                   1133           90  -92.03%

[dvyukov, 2013-12-31 11:09:08]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://dvyukov%40google.com@code.google.com/p/go/

[dvyukov, 2013-12-31 11:10:24]

If it LGTM, I yet need to figure out necessary race detector support. It does
not understand that per-P thing.

[bradfitz, 2014-01-02 18:42:02]

Can you include a benchmark of fmt and/or json too?

[dvyukov, 2014-01-09 16:11:55]

On 2014/01/02 18:42:02, bradfitz wrote:
> Can you include a benchmark of fmt and/or json too?

Done

[bradfitz, 2014-01-09 16:21:56]

Any improvement on JSON? I can try later. I imagine so, if fmt improved
that much.




On Thu, Jan 9, 2014 at 8:11 AM, <dvyukov@google.com> wrote:

> On 2014/01/02 18:42:02, bradfitz wrote:
>
>> Can you include a benchmark of fmt and/or json too?
>>
>
> Done
>
> https://codereview.appspot.com/46010043/
>

[minux1, 2014-01-09 20:10:18]

the performance improvements are amazing! Thanks.

https://codereview.appspot.com/46010043/diff/60001/src/pkg/sync/pool.go
File src/pkg/sync/pool.go (right):

https://codereview.appspot.com/46010043/diff/60001/src/pkg/sync/pool.go#newco...
src/pkg/sync/pool.go:54: // Read-mostly date above this point, mutable data
follows.
s/date/data/

[gobot, 2014-01-16 19:31:09]

R=rsc@golang.org (assigned by bradfitz@golang.org)

[rsc, 2014-01-16 20:12:46]

On a 64-bit system, the old Pool took 48 bytes. The new pool takes 200+N*8
bytes, where N is the GOMAXPROCS setting.
If someone does runtime.GOMAXPROCS(1000), it gets set to 256 (the real
maximum), and then each pool takes 200+256*8 bytes, 45x bigger than they
used to be.
Even at GOMAXPROCS=8, you're still talking about a 5x memory hit.

Can we find a way to avoid wasting so much memory?

Russ

[dvyukov, 2014-01-17 08:01:09]

On 2014/01/16 20:12:46, rsc wrote:
> On a 64-bit system, the old Pool took 48 bytes. The new pool takes 200+N*8
> bytes, where N is the GOMAXPROCS setting.
> If someone does runtime.GOMAXPROCS(1000), it gets set to 256 (the real
> maximum), and then each pool takes 200+256*8 bytes, 45x bigger than they
> used to be.
> Even at GOMAXPROCS=8, you're still talking about a 5x memory hit.
> 
> Can we find a way to avoid wasting so much memory?


The memory overhead is much higher -- 272*N.

I probably could take it down to 128*N, but I am not sure it worth it. The Pool
is intended to be global (so some definition of global). So some packages may
have 1-5 Pools. If you are running on a 256-processor machine, these bytes are
really nothing *and* you want things to be really scalable as never.
Trading some bytes to 10x performance drop makes no sense (i.e. using
256-processor machine as 25-processor machine).
Comparison with the current version is not fully fair, because current
implementation does not address scalability at all.
And if a user abuses Pool, we can not fix it in Pool implementation. Just like
for encoding/gob.Encoder you've said that it's intended to be per-connection,
and we build implementation around that assumption.

[rsc, 2014-01-17 14:49:02]

Okay, I guess this just reemphasizes the fact that sync.Pool is ONLY for
global variables. I am glad we all agree about that.

[rsc, 2014-01-22 18:53:42]

LGTM

[dvyukov, 2014-01-24 18:29:58]

*** Submitted as https://code.google.com/p/go/source/detail?r=e4a4cb47c141 ***

sync: scalable Pool
Introduce fixed-size P-local caches.
When local caches overflow/underflow a batch of items
is transferred to/from global mutex-protected cache.

benchmark                    old ns/op    new ns/op    delta
BenchmarkPool                    50554        22423  -55.65%
BenchmarkPool-4                 400359         5904  -98.53%
BenchmarkPool-16                403311         1598  -99.60%
BenchmarkPool-32                367310         1526  -99.58%

BenchmarkPoolOverlflow            5214         3633  -30.32%
BenchmarkPoolOverlflow-4         42663         9539  -77.64%
BenchmarkPoolOverlflow-8         46919        11385  -75.73%
BenchmarkPoolOverlflow-16        39454        13048  -66.93%

BenchmarkSprintfEmpty                    84           63  -25.68%
BenchmarkSprintfEmpty-2                 371           32  -91.13%
BenchmarkSprintfEmpty-4                 465           22  -95.25%
BenchmarkSprintfEmpty-8                 565           12  -97.77%
BenchmarkSprintfEmpty-16                498            5  -98.87%
BenchmarkSprintfEmpty-32                492            4  -99.04%

BenchmarkSprintfString                  259          229  -11.58%
BenchmarkSprintfString-2                574          144  -74.91%
BenchmarkSprintfString-4                651           77  -88.05%
BenchmarkSprintfString-8                868           47  -94.48%
BenchmarkSprintfString-16               825           33  -95.96%
BenchmarkSprintfString-32               825           30  -96.28%

BenchmarkSprintfInt                     213          188  -11.74%
BenchmarkSprintfInt-2                   448          138  -69.20%
BenchmarkSprintfInt-4                   624           52  -91.63%
BenchmarkSprintfInt-8                   691           31  -95.43%
BenchmarkSprintfInt-16                  724           18  -97.46%
BenchmarkSprintfInt-32                  718           16  -97.70%

BenchmarkSprintfIntInt                  311          282   -9.32%
BenchmarkSprintfIntInt-2                333          145  -56.46%
BenchmarkSprintfIntInt-4                642          110  -82.87%
BenchmarkSprintfIntInt-8                832           42  -94.90%
BenchmarkSprintfIntInt-16               817           24  -97.00%
BenchmarkSprintfIntInt-32               805           22  -97.17%

BenchmarkSprintfPrefixedInt             309          269  -12.94%
BenchmarkSprintfPrefixedInt-2           245          168  -31.43%
BenchmarkSprintfPrefixedInt-4           598           99  -83.36%
BenchmarkSprintfPrefixedInt-8           770           67  -91.23%
BenchmarkSprintfPrefixedInt-16          829           54  -93.49%
BenchmarkSprintfPrefixedInt-32          824           50  -93.83%

BenchmarkSprintfFloat                   418          398   -4.78%
BenchmarkSprintfFloat-2                 295          203  -31.19%
BenchmarkSprintfFloat-4                 585          128  -78.12%
BenchmarkSprintfFloat-8                 873           60  -93.13%
BenchmarkSprintfFloat-16                884           33  -96.24%
BenchmarkSprintfFloat-32                881           29  -96.62%

BenchmarkManyArgs                      1097         1069   -2.55%
BenchmarkManyArgs-2                     705          567  -19.57%
BenchmarkManyArgs-4                     792          319  -59.72%
BenchmarkManyArgs-8                     963          172  -82.14%
BenchmarkManyArgs-16                   1115          103  -90.76%
BenchmarkManyArgs-32                   1133           90  -92.03%

LGTM=rsc
R=golang-codereviews, bradfitz, minux.ma, gobot, rsc
CC=golang-codereviews
https://codereview.appspot.com/46010043

[gobot, 2014-01-24 18:41:23]

This CL appears to have broken the freebsd-amd64 builder.