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

Issue 14317043: code review 14317043: runtime: change default stack segment size to 8 kB (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
10 years, 5 months ago by rsc
Modified:
10 years, 5 months ago
Reviewers:
ugorji, dfc, mtj1, khr1
CC:
golang-dev, khr1, dfc, bradfitz, dvyukov
Visibility:
Public.

Description

runtime: change default stack segment size to 8 kB Changing from 4 kB to 8 kB brings significant improvement on a variety of the Go 1 benchmarks, on both amd64 and 386 systems. Significant runtime reductions: amd64 386 GoParse -14% -1% GobDecode -12% -20% GobEncode -64% -1% JSONDecode -9% -4% JSONEncode -15% -5% Template -17% -14% In the longer term, khr's new stacks will avoid needing to make this decision at all, but for Go 1.2 this is a reasonable stopgap that makes performance significantly better. Demand paging should mean that if the second 4 kB is not used, it will not be brought into memory, so the change should not adversely affect resident set size. The same argument could justify bumping as high as 64 kB on 64-bit machines, but there are diminishing returns after 8 kB, and using 8 kB limits the possible unintended memory overheads we are not aware of. Benchmark graphs at http://swtch.com/~rsc/gostackamd64.html http://swtch.com/~rsc/gostack386.html Full data at http://swtch.com/~rsc/gostack.zip

Patch Set 1 #

Patch Set 2 : diff -r 6703ffd112ea https://code.google.com/p/go/ #

Patch Set 3 : diff -r c4d996668981 https://code.google.com/p/go/ #

Patch Set 4 : diff -r 7aaac307aa0e https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2 lines, -2 lines) Patch
M src/pkg/runtime/stack.h View 1 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 17
rsc
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
10 years, 5 months ago (2013-10-03 01:53:05 UTC) #1
khr1
LGTM. On Wed, Oct 2, 2013 at 6:53 PM, <rsc@golang.org> wrote: > Reviewers: golang-dev1, > ...
10 years, 5 months ago (2013-10-03 02:49:44 UTC) #2
dfc
LGTM. Thank you. On Thu, Oct 3, 2013 at 12:49 PM, Keith Randall <khr@google.com> wrote: ...
10 years, 5 months ago (2013-10-03 03:02:20 UTC) #3
bradfitz
Where did this come from? Where was the discussion / bug? CL description has no ...
10 years, 5 months ago (2013-10-03 04:54:34 UTC) #4
dvyukov
Agree. I can significantly increase performance w/o increasing memory consumption, as well as leave performance ...
10 years, 5 months ago (2013-10-03 05:49:55 UTC) #5
rsc
*** Submitted as https://code.google.com/p/go/source/detail?r=b4a7054c3243 *** runtime: change default stack segment size to 8 kB Changing ...
10 years, 5 months ago (2013-10-03 13:19:19 UTC) #6
ugorji
Thank you. I'm pleasantly surprised that this was done. I'm very happy it was done. ...
10 years, 5 months ago (2013-10-03 13:52:51 UTC) #7
rsc
On Thu, Oct 3, 2013 at 12:54 AM, Brad Fitzpatrick <bradfitz@golang.org>wrote: > Where did this ...
10 years, 5 months ago (2013-10-03 14:00:56 UTC) #8
rsc
On Thu, Oct 3, 2013 at 1:49 AM, Dmitry Vyukov <dvyukov@google.com> wrote: > Agree. > ...
10 years, 5 months ago (2013-10-03 14:02:28 UTC) #9
dvyukov
On Thu, Oct 3, 2013 at 6:02 PM, Russ Cox <rsc@golang.org> wrote: > On Thu, ...
10 years, 5 months ago (2013-10-03 14:14:51 UTC) #10
rsc
> > I looked at them. > Any X is too small for a variety ...
10 years, 5 months ago (2013-10-03 14:53:44 UTC) #11
mtj1
My finely-optimized exact cover solver is an example code unlike many of the Go benchmarks ...
10 years, 5 months ago (2013-10-03 15:18:19 UTC) #12
bradfitz
On Oct 3, 2013 4:00 AM, "Russ Cox" <rsc@golang.org> wrote: > > On Thu, Oct ...
10 years, 5 months ago (2013-10-03 17:04:15 UTC) #13
rsc
On Thu, Oct 3, 2013 at 1:04 PM, Brad Fitzpatrick <bradfitz@golang.org>wrote: > > On Oct ...
10 years, 5 months ago (2013-10-03 17:28:38 UTC) #14
mtj1
Additional 3.45% faster today...8.2% faster over the last two weeks! # Gap(12, 2): 55696 solutions ...
10 years, 5 months ago (2013-10-09 00:21:17 UTC) #15
dfc
Michael, What do you attribute this 3.45% improvement to ? I can't see any changes ...
10 years, 5 months ago (2013-10-09 00:31:19 UTC) #16
mtj1
10 years, 5 months ago (2013-10-09 00:37:58 UTC) #17
sorry. I don't see anything special either.


On Tue, Oct 8, 2013 at 5:31 PM, Dave Cheney <dave@cheney.net> wrote:

> Michael,
>
> What do you attribute this 3.45% improvement to ? I can't see any
> changes the last few days that actually change code, just docs and
> examples.
>
> On Wed, Oct 9, 2013 at 11:20 AM, Michael Jones <mtj@google.com> wrote:
> > Additional 3.45% faster today...8.2% faster over the last two weeks!
> >
> > # Gap(12, 2):            55696 solutions           818550 nodes
> > 11935776 updates         0.108 s 110.173 Mu/s
> > # Gap(12, 1):           108144 solutions          1351543 nodes
> > 19494378 updates         0.168 s 115.913 Mu/s
> > # Gap(12, 0):           227968 solutions          2137237 nodes
> > 31496482 updates         0.261 s 120.868 Mu/s
> > # Gap(13, 6):            17800 solutions           276931 nodes
> > 4032013 updates         0.037 s
> > # Gap(13, 4):           129220 solutions          2406091 nodes
> > 34510000 updates         0.302 s 114.125 Mu/s
> > # Gap(13, 2):           360876 solutions          5586798 nodes
> > 81469260 updates         0.710 s 114.785 Mu/s
> > # Gap(13, 0):          1520280 solutions         14940920 nodes
> > 219953588 updates         1.933 s 113.760 Mu/s
> > # Gap(15, 7):           329816 solutions          6433598 nodes
> > 94719176 updates         0.867 s 109.256 Mu/s
> > # Gap(15, 5):          3912272 solutions         82944237 nodes
> > 1180126632 updates        10.502 s 112.377 Mu/s
> > # Gap(15, 3):         10346528 solutions        197704780 nodes
> > 2866894951 updates        25.618 s 111.909 Mu/s
> > # Gap(15, 1):         39809640 solutions        550148348 nodes
> > 7965822480 updates        71.657 s 111.165 Mu/s
> > # Gap(16, 7):          5097864 solutions        101351300 nodes
> > 1410017217 updates        12.974 s 108.677 Mu/s
> > # Gap(16, 6):         17367424 solutions        375304505 nodes
> > 5250867254 updates        47.744 s 109.981 Mu/s
> > # Gap(16, 5):         30346968 solutions        685284939 nodes
> > 9843205022 updates        88.756 s 110.902 Mu/s
> >
> >
> > On Thu, Oct 3, 2013 at 10:28 AM, Russ Cox <rsc@golang.org> wrote:
> >>
> >> On Thu, Oct 3, 2013 at 1:04 PM, Brad Fitzpatrick <bradfitz@golang.org>
> >> wrote:
> >>>
> >>>
> >>> On Oct 3, 2013 4:00 AM, "Russ Cox" <rsc@golang.org> wrote:
> >>> >
> >>> > On Thu, Oct 3, 2013 at 12:54 AM, Brad Fitzpatrick <
> bradfitz@golang.org>
> >>> > wrote:
> >>> >>
> >>> >> Where did this come from?
> >>> >>
> >>> >> Where was the discussion / bug? CL description has no links.
> >>> >>
> >>> >> Seems kinda late in the cycle for something so big.
> >>> >
> >>> > This is the reason JSONDecode was 10% slower than in Go 1.1.
> >>>
> >>> Did stack frames get bigger? At which CLs? Just curious.
> >>
> >> The biggest performance dip was due to CL 12829043, which made stack
> >> frames smaller.
> >>
> >> Russ
> >>
> >> --
> >>
> >> ---
> >> You received this message because you are subscribed to the Google
> Groups
> >> "golang-dev" group.
> >> To unsubscribe from this group and stop receiving emails from it, send
> an
> >> email to golang-dev+unsubscribe@googlegroups.com.
> >> For more options, visit https://groups.google.com/groups/opt_out.
> >
> >
> >
> >
> > --
> > Michael T. Jones | Chief Technology Advocate  | mtj@google.com |  +1
> > 650-335-5765
> >
> > --
> >
> > ---
> > You received this message because you are subscribed to the Google Groups
> > "golang-dev" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to golang-dev+unsubscribe@googlegroups.com.
> > For more options, visit https://groups.google.com/groups/opt_out.
>



-- 
Michael T. Jones | Chief Technology Advocate  | mtj@google.com |  +1
650-335-5765
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b