code review 92540043: runtime: switch default stack size back to 8kB

runtime: switch default stack size back to 8kB

The move from 4kB to 8kB in Go 1.2 was to eliminate many stack split hot spots.

The move back to 4kB was predicated on copying stacks eliminating
the potential for hot spots.

Unfortunately, the fact that stacks do not copy 100% of the time means
that hot spots can still happen under the right conditions, and the slowdown
is worse now than it was in Go 1.2. There is a real program in issue 8030 that
sees about a 30x slowdown: it has a reflect call near the top of the stack
which inhibits any stack copying on that segment.

Go back to 8kB until stack copying can be used 100% of the time.

Fixes issue 8030.

[rsc, 2014-05-20 03:46:34]

Hello iant, khr, r (cc: golang-codereviews@googlegroups.com),

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

[khr1, 2014-05-20 03:48:05]

LGTM.



On Mon, May 19, 2014 at 8:46 PM, <rsc@golang.org> wrote:

> Reviewers: iant, khr, r,
>
> Message:
> Hello iant, khr, r (cc: golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> runtime: switch default stack size back to 8kB
>
> The move from 4kB to 8kB in Go 1.2 was to eliminate many stack split hot
> spots.
>
> The move back to 4kB was predicated on copying stacks eliminating
> the potential for hot spots.
>
> Unfortunately, the fact that stacks do not copy 100% of the time means
> that hot spots can still happen under the right conditions, and the
> slowdown
> is worse now than it was in Go 1.2. There is a real program in issue
> 8030 that
> sees about a 30x slowdown: it has a reflect call near the top of the
> stack
> which inhibits any stack copying on that segment.
>
> Go back to 8kB until stack copying can be used 100% of the time.
>
> Fixes issue 8030.
>
> Please review this at https://codereview.appspot.com/92540043/
>
> Affected files (+1, -8 lines):
>   M doc/go1.3.html
>   M src/pkg/runtime/stack.h
>
>
> Index: doc/go1.3.html
> ===================================================================
> --- a/doc/go1.3.html
> +++ b/doc/go1.3.html
> @@ -118,13 +118,6 @@
>  <a href="http://golang.org/s/contigstacks">design document</a>.
>  </p>
>
> -<h3 id="stack_size">Stack size</h3>
> -
> -<p>
> -Go 1.2 increased the minimum stack size to 8 kilobytes; with the new
> stack model, it has been
> -put back to 4 kilobytes.
> -</p>
> -
>  <h3 id="garbage_collector">Changes to the garbage collector</h3>
>
>  <p>
> Index: src/pkg/runtime/stack.h
> ===================================================================
> --- a/src/pkg/runtime/stack.h
> +++ b/src/pkg/runtime/stack.h
> @@ -76,7 +76,7 @@
>         // The minimum stack segment size to allocate.
>         // If the amount needed for the splitting frame + StackExtra
>         // is less than this number, the stack will have this size instead.
> -       StackMin = 4096,
> +       StackMin = 8192,
>         StackSystemRounded = StackSystem + (-StackSystem & (StackMin-1)),
>         FixedStack = StackMin + StackSystemRounded,
>
>
>
>

[bradfitz, 2014-05-20 03:55:04]

Bummer.
 On May 19, 2014 8:46 PM, <rsc@golang.org> wrote:

> Reviewers: iant, khr, r,
>
> Message:
> Hello iant, khr, r (cc: golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> runtime: switch default stack size back to 8kB
>
> The move from 4kB to 8kB in Go 1.2 was to eliminate many stack split hot
> spots.
>
> The move back to 4kB was predicated on copying stacks eliminating
> the potential for hot spots.
>
> Unfortunately, the fact that stacks do not copy 100% of the time means
> that hot spots can still happen under the right conditions, and the
> slowdown
> is worse now than it was in Go 1.2. There is a real program in issue
> 8030 that
> sees about a 30x slowdown: it has a reflect call near the top of the
> stack
> which inhibits any stack copying on that segment.
>
> Go back to 8kB until stack copying can be used 100% of the time.
>
> Fixes issue 8030.
>
> Please review this at https://codereview.appspot.com/92540043/
>
> Affected files (+1, -8 lines):
>   M doc/go1.3.html
>   M src/pkg/runtime/stack.h
>
>
> Index: doc/go1.3.html
> ===================================================================
> --- a/doc/go1.3.html
> +++ b/doc/go1.3.html
> @@ -118,13 +118,6 @@
>  <a href="http://golang.org/s/contigstacks">design document</a>.
>  </p>
>
> -<h3 id="stack_size">Stack size</h3>
> -
> -<p>
> -Go 1.2 increased the minimum stack size to 8 kilobytes; with the new
> stack model, it has been
> -put back to 4 kilobytes.
> -</p>
> -
>  <h3 id="garbage_collector">Changes to the garbage collector</h3>
>
>  <p>
> Index: src/pkg/runtime/stack.h
> ===================================================================
> --- a/src/pkg/runtime/stack.h
> +++ b/src/pkg/runtime/stack.h
> @@ -76,7 +76,7 @@
>         // The minimum stack segment size to allocate.
>         // If the amount needed for the splitting frame + StackExtra
>         // is less than this number, the stack will have this size instead.
> -       StackMin = 4096,
> +       StackMin = 8192,
>         StackSystemRounded = StackSystem + (-StackSystem & (StackMin-1)),
>         FixedStack = StackMin + StackSystemRounded,
>
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[dave_cheney.net, 2014-05-20 03:57:24]

LGTM.

On Tue, May 20, 2014 at 1:48 PM, 'Keith Randall' via
golang-codereviews <golang-codereviews@googlegroups.com> wrote:
> LGTM.
>
>
>
> On Mon, May 19, 2014 at 8:46 PM, <rsc@golang.org> wrote:
>>
>> Reviewers: iant, khr, r,
>>
>> Message:
>> Hello iant, khr, r (cc: golang-codereviews@googlegroups.com),
>>
>> I'd like you to review this change to
>> https://code.google.com/p/go/
>>
>>
>> Description:
>> runtime: switch default stack size back to 8kB
>>
>> The move from 4kB to 8kB in Go 1.2 was to eliminate many stack split hot
>> spots.
>>
>> The move back to 4kB was predicated on copying stacks eliminating
>> the potential for hot spots.
>>
>> Unfortunately, the fact that stacks do not copy 100% of the time means
>> that hot spots can still happen under the right conditions, and the
>> slowdown
>> is worse now than it was in Go 1.2. There is a real program in issue
>> 8030 that
>> sees about a 30x slowdown: it has a reflect call near the top of the
>> stack
>> which inhibits any stack copying on that segment.
>>
>> Go back to 8kB until stack copying can be used 100% of the time.
>>
>> Fixes issue 8030.
>>
>> Please review this at https://codereview.appspot.com/92540043/
>>
>> Affected files (+1, -8 lines):
>>   M doc/go1.3.html
>>   M src/pkg/runtime/stack.h
>>
>>
>> Index: doc/go1.3.html
>> ===================================================================
>> --- a/doc/go1.3.html
>> +++ b/doc/go1.3.html
>> @@ -118,13 +118,6 @@
>>  <a href="http://golang.org/s/contigstacks">design document</a>.
>>  </p>
>>
>> -<h3 id="stack_size">Stack size</h3>
>> -
>> -<p>
>> -Go 1.2 increased the minimum stack size to 8 kilobytes; with the new
>> stack model, it has been
>> -put back to 4 kilobytes.
>> -</p>
>> -
>>  <h3 id="garbage_collector">Changes to the garbage collector</h3>
>>
>>  <p>
>> Index: src/pkg/runtime/stack.h
>> ===================================================================
>> --- a/src/pkg/runtime/stack.h
>> +++ b/src/pkg/runtime/stack.h
>> @@ -76,7 +76,7 @@
>>         // The minimum stack segment size to allocate.
>>         // If the amount needed for the splitting frame + StackExtra
>>         // is less than this number, the stack will have this size
>> instead.
>> -       StackMin = 4096,
>> +       StackMin = 8192,
>>         StackSystemRounded = StackSystem + (-StackSystem & (StackMin-1)),
>>         FixedStack = StackMin + StackSystemRounded,
>>
>>
>>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[iant, 2014-05-20 04:29:05]

LGTM

[rsc, 2014-05-20 04:30:46]

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

runtime: switch default stack size back to 8kB

The move from 4kB to 8kB in Go 1.2 was to eliminate many stack split hot spots.

The move back to 4kB was predicated on copying stacks eliminating
the potential for hot spots.

Unfortunately, the fact that stacks do not copy 100% of the time means
that hot spots can still happen under the right conditions, and the slowdown
is worse now than it was in Go 1.2. There is a real program in issue 8030 that
sees about a 30x slowdown: it has a reflect call near the top of the stack
which inhibits any stack copying on that segment.

Go back to 8kB until stack copying can be used 100% of the time.

Fixes issue 8030.

LGTM=khr, dave, iant
R=iant, khr, r, bradfitz, dave
CC=golang-codereviews
https://codereview.appspot.com/92540043