Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(4)

Issue 46010043: code review 46010043: sync: scalable Pool (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
3 years, 2 months ago by dvyukov
Modified:
3 years, 1 month ago
Reviewers:
gobot, rsc
CC:
golang-codereviews, bradfitz, minux1, rsc
Visibility:
Public.

Description

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%

Patch Set 1 #

Patch Set 2 : diff -r 9963a911bc14 https://dvyukov%40google.com@code.google.com/p/go/ #

Patch Set 3 : diff -r 9963a911bc14 https://dvyukov%40google.com@code.google.com/p/go/ #

Patch Set 4 : diff -r ea67ddaa02ef https://dvyukov%40google.com@code.google.com/p/go/ #

Total comments: 1

Patch Set 5 : diff -r 7d1281494058 https://dvyukov%40google.com@code.google.com/p/go/ #

Patch Set 6 : diff -r 7d1281494058 https://dvyukov%40google.com@code.google.com/p/go/ #

Patch Set 7 : diff -r 7d1281494058 https://dvyukov%40google.com@code.google.com/p/go/ #

Patch Set 8 : diff -r 7d1281494058 https://dvyukov%40google.com@code.google.com/p/go/ #

Patch Set 9 : diff -r 50f52d5c2bb7 https://dvyukov%40google.com@code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+208 lines, -37 lines) Patch
M src/pkg/go/build/deps_test.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/runtime/mgc0.c View 1 2 3 4 5 1 chunk +7 lines, -3 lines 0 comments Download
M src/pkg/runtime/proc.c View 1 2 3 4 1 chunk +20 lines, -0 lines 0 comments Download
M src/pkg/sync/pool.go View 1 3 chunks +150 lines, -15 lines 0 comments Download
M src/pkg/sync/pool_test.go View 1 2 2 chunks +30 lines, -18 lines 0 comments Download

Messages

Total messages: 13
dvyukov
Hello golang-codereviews@googlegroups.com, I'd like you to review this change to https://dvyukov%40google.com@code.google.com/p/go/
3 years, 2 months ago (2013-12-31 11:09:08 UTC) #1
dvyukov
If it LGTM, I yet need to figure out necessary race detector support. It does ...
3 years, 2 months ago (2013-12-31 11:10:24 UTC) #2
bradfitz
Can you include a benchmark of fmt and/or json too?
3 years, 2 months ago (2014-01-02 18:42:02 UTC) #3
dvyukov
On 2014/01/02 18:42:02, bradfitz wrote: > Can you include a benchmark of fmt and/or json ...
3 years, 2 months ago (2014-01-09 16:11:55 UTC) #4
bradfitz
Any improvement on JSON? I can try later. I imagine so, if fmt improved that ...
3 years, 2 months ago (2014-01-09 16:21:56 UTC) #5
minux1
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#newcode54 src/pkg/sync/pool.go:54: // Read-mostly date ...
3 years, 2 months ago (2014-01-09 20:10:18 UTC) #6
gobot
R=rsc@golang.org (assigned by bradfitz@golang.org)
3 years, 2 months ago (2014-01-16 19:31:09 UTC) #7
rsc
On a 64-bit system, the old Pool took 48 bytes. The new pool takes 200+N*8 ...
3 years, 2 months ago (2014-01-16 20:12:46 UTC) #8
dvyukov
On 2014/01/16 20:12:46, rsc wrote: > On a 64-bit system, the old Pool took 48 ...
3 years, 2 months ago (2014-01-17 08:01:09 UTC) #9
rsc
Okay, I guess this just reemphasizes the fact that sync.Pool is ONLY for global variables. ...
3 years, 2 months ago (2014-01-17 14:49:02 UTC) #10
rsc
LGTM
3 years, 2 months ago (2014-01-22 18:53:42 UTC) #11
dvyukov
*** Submitted as https://code.google.com/p/go/source/detail?r=e4a4cb47c141 *** sync: scalable Pool Introduce fixed-size P-local caches. When local caches ...
3 years, 1 month ago (2014-01-24 18:29:58 UTC) #12
gobot
3 years, 1 month ago (2014-01-24 18:41:23 UTC) #13
Message was sent while issue was closed.
This CL appears to have broken the freebsd-amd64 builder.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 80a51fa-tainted