code review 112240044: undo CL 101570044 / 2c57aaea79c4

undo CL 101570044 / 2c57aaea79c4

redo stack allocation.  This is mostly the same as
the original CL with a few bug fixes.

1. add racemalloc() for stack allocations
2. fix poolalloc/poolfree to terminate free lists correctly.
3. adjust span ref count correctly.
4. don't use cache for sizes >= StackCacheSize.

Should fix bugs and memory leaks in original changelist.

««« original CL description
undo CL 104200047 / 318b04f28372

Breaks windows and race detector.
TBR=rsc

««« original CL description
runtime: stack allocator, separate from mallocgc

In order to move malloc to Go, we need to have a
separate stack allocator.  If we run out of stack
during malloc, malloc will not be available
to allocate a new stack.

Stacks are the last remaining FlagNoGC objects in the
GC heap.  Once they are out, we can get rid of the
distinction between the allocated/blockboundary bits.
(This will be in a separate change.)

Fixes issue 7468
Fixes issue 7424

LGTM=rsc, dvyukov
R=golang-codereviews, dvyukov, khr, dave, rsc
CC=golang-codereviews
https://codereview.appspot.com/104200047
»»»

TBR=rsc
CC=golang-codereviews
https://codereview.appspot.com/101570044
»»»

[khr, 2014-07-17 03:57:19]

Hello dvyukov (cc: golang-codereviews@googlegroups.com),

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

[dave_cheney.net, 2014-07-17 03:59:14]

Is this an undo of an undo ?

On Thu, Jul 17, 2014 at 1:57 PM,  <khr@golang.org> wrote:
> Reviewers: dvyukov,
>
> Message:
> Hello dvyukov (cc: golang-codereviews@googlegroups.com),
>
> I'd like you to review this change to
> https://khr%40golang.org@code.google.com/p/go/
>
>
> Description:
> undo CL 101570044 / 2c57aaea79c4
>
> redo stack allocation.  This is mostly the same as
> the original CL with a few bug fixes.
>
> 1. add racemalloc() for stack allocations
> 2. fix poolalloc/poolfree to terminate free lists correctly.
> 3. adjust span ref count correctly.
> 4. don't use cache for sizes >= StackCacheSize.
>
> Should fix bugs and memory leaks in original changelist.
>
> ««« original CL description
> undo CL 104200047 / 318b04f28372
>
> Breaks windows and race detector.
> TBR=rsc
>
> ««« original CL description
> runtime: stack allocator, separate from mallocgc
>
> In order to move malloc to Go, we need to have a
> separate stack allocator.  If we run out of stack
> during malloc, malloc will not be available
> to allocate a new stack.
>
> Stacks are the last remaining FlagNoGC objects in the
> GC heap.  Once they are out, we can get rid of the
> distinction between the allocated/blockboundary bits.
> (This will be in a separate change.)
>
> Fixes issue 7468
> Fixes issue 7424
>
> LGTM=rsc, dvyukov
> R=golang-codereviews, dvyukov, khr, dave, rsc
> CC=golang-codereviews
> https://codereview.appspot.com/104200047
> »»»
>
> TBR=rsc
> CC=golang-codereviews
> https://codereview.appspot.com/101570044
> »»»
>
> Please review this at https://codereview.appspot.com/112240044/
>
> Affected files (+521, -298 lines):
>   M src/pkg/runtime/malloc.h
>   M src/pkg/runtime/mcache.c
>   M src/pkg/runtime/mcentral.c
>   M src/pkg/runtime/mem.go
>   M src/pkg/runtime/mgc0.c
>   M src/pkg/runtime/mheap.c
>   M src/pkg/runtime/proc.c
>   M src/pkg/runtime/runtime.h
>   M src/pkg/runtime/stack.c
>   M src/pkg/runtime/stack_test.go
>
>
> --
> 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.

[khr1, 2014-07-17 04:09:39]

Yes, it is.  Plus bug fixes.



On Wed, Jul 16, 2014 at 8:59 PM, Dave Cheney <dave@cheney.net> wrote:

> Is this an undo of an undo ?
>
> On Thu, Jul 17, 2014 at 1:57 PM,  <khr@golang.org> wrote:
> > Reviewers: dvyukov,
> >
> > Message:
> > Hello dvyukov (cc: golang-codereviews@googlegroups.com),
> >
> > I'd like you to review this change to
> > https://khr%40golang.org@code.google.com/p/go/
> >
> >
> > Description:
> > undo CL 101570044 / 2c57aaea79c4
> >
> > redo stack allocation.  This is mostly the same as
> > the original CL with a few bug fixes.
> >
> > 1. add racemalloc() for stack allocations
> > 2. fix poolalloc/poolfree to terminate free lists correctly.
> > 3. adjust span ref count correctly.
> > 4. don't use cache for sizes >= StackCacheSize.
> >
> > Should fix bugs and memory leaks in original changelist.
> >
> > ««« original CL description
> > undo CL 104200047 / 318b04f28372
> >
> > Breaks windows and race detector.
> > TBR=rsc
> >
> > ««« original CL description
> > runtime: stack allocator, separate from mallocgc
> >
> > In order to move malloc to Go, we need to have a
> > separate stack allocator.  If we run out of stack
> > during malloc, malloc will not be available
> > to allocate a new stack.
> >
> > Stacks are the last remaining FlagNoGC objects in the
> > GC heap.  Once they are out, we can get rid of the
> > distinction between the allocated/blockboundary bits.
> > (This will be in a separate change.)
> >
> > Fixes issue 7468
> > Fixes issue 7424
> >
> > LGTM=rsc, dvyukov
> > R=golang-codereviews, dvyukov, khr, dave, rsc
> > CC=golang-codereviews
> > https://codereview.appspot.com/104200047
> > »»»
> >
> > TBR=rsc
> > CC=golang-codereviews
> > https://codereview.appspot.com/101570044
> > »»»
> >
> > Please review this at https://codereview.appspot.com/112240044/
> >
> > Affected files (+521, -298 lines):
> >   M src/pkg/runtime/malloc.h
> >   M src/pkg/runtime/mcache.c
> >   M src/pkg/runtime/mcentral.c
> >   M src/pkg/runtime/mem.go
> >   M src/pkg/runtime/mgc0.c
> >   M src/pkg/runtime/mheap.c
> >   M src/pkg/runtime/proc.c
> >   M src/pkg/runtime/runtime.h
> >   M src/pkg/runtime/stack.c
> >   M src/pkg/runtime/stack_test.go
> >
> >
> > --
> > 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.
>

[dvyukov, 2014-07-17 04:41:37]

On 2014/07/17 03:59:14, dfc wrote:
> Is this an undo of an undo ?

I am OOO today, I will review it tomorrow.
If I see that it breaks something, I will not tell you, because I want to see
'undo of undo of undo of undo' when you roll it back and then fix again.

[brainman, 2014-07-17 06:43:58]

All tests pass on both windows/386 and windows/amd64.

Alex

[dave_cheney.net, 2014-07-17 08:21:58]

On 2014/07/17 06:43:58, brainman wrote:
> All tests pass on both windows/386 and windows/amd64.
> 
> Alex

Race build passes on darinw/amd64

[dave_cheney.net, 2014-07-17 09:51:32]

linux/arm is fine as well.


On Thu, Jul 17, 2014 at 6:21 PM, <dave@cheney.net> wrote:

> On 2014/07/17 06:43:58, brainman wrote:
>
>> All tests pass on both windows/386 and windows/amd64.
>>
>
>  Alex
>>
>
> Race build passes on darinw/amd64
>
> https://codereview.appspot.com/112240044/
>

[dvyukov, 2014-07-17 16:43:49]

LGTM with nits

https://codereview.appspot.com/112240044/diff/60001/src/pkg/runtime/stack.c
File src/pkg/runtime/stack.c (right):

https://codereview.appspot.com/112240044/diff/60001/src/pkg/runtime/stack.c#n...
src/pkg/runtime/stack.c:242: s = runtime·MHeap_AllocStack(&runtime·mheap,
(n+PageSize-1) >> PageShift);
ROUND(n, PageSize)

https://codereview.appspot.com/112240044/diff/60001/src/pkg/runtime/stack.c#n...
src/pkg/runtime/stack.c:464: if(gp->stackbase == 0)
when can it happen?

https://codereview.appspot.com/112240044/diff/60001/src/pkg/runtime/stack.c#n...
src/pkg/runtime/stack.c:1027: if(gp->stackbase == 0)
in mgc0.c we call shrinkstack for all existing G's, even Gdead
try to check status for Gdead, and then assert gp->stackbase != 0

[khr, 2014-07-17 21:41:59]

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

undo CL 101570044 / 2c57aaea79c4

redo stack allocation.  This is mostly the same as
the original CL with a few bug fixes.

1. add racemalloc() for stack allocations
2. fix poolalloc/poolfree to terminate free lists correctly.
3. adjust span ref count correctly.
4. don't use cache for sizes >= StackCacheSize.

Should fix bugs and memory leaks in original changelist.

««« original CL description
undo CL 104200047 / 318b04f28372

Breaks windows and race detector.
TBR=rsc

««« original CL description
runtime: stack allocator, separate from mallocgc

In order to move malloc to Go, we need to have a
separate stack allocator.  If we run out of stack
during malloc, malloc will not be available
to allocate a new stack.

Stacks are the last remaining FlagNoGC objects in the
GC heap.  Once they are out, we can get rid of the
distinction between the allocated/blockboundary bits.
(This will be in a separate change.)

Fixes issue 7468
Fixes issue 7424

LGTM=rsc, dvyukov
R=golang-codereviews, dvyukov, khr, dave, rsc
CC=golang-codereviews
https://codereview.appspot.com/104200047
»»»

TBR=rsc
CC=golang-codereviews
https://codereview.appspot.com/101570044
»»»

LGTM=dvyukov
R=dvyukov, dave, khr, alex.brainman
CC=golang-codereviews
https://codereview.appspot.com/112240044

[dvyukov, 2014-07-18 10:22:19]

Perf dashboard reports 7036874417757228.00% increase in stack usage:
http://goperfd.appspot.com/perfdetail?commit=a622a4ff09da1ca11880d1b58fc122ac...




On Fri, Jul 18, 2014 at 1:41 AM,  <khr@golang.org> wrote:
> *** Submitted as
> https://code.google.com/p/go/source/detail?r=bac0a67b0b70 ***
>
>
> undo CL 101570044 / 2c57aaea79c4
>
> redo stack allocation.  This is mostly the same as
> the original CL with a few bug fixes.
>
> 1. add racemalloc() for stack allocations
> 2. fix poolalloc/poolfree to terminate free lists correctly.
> 3. adjust span ref count correctly.
> 4. don't use cache for sizes >= StackCacheSize.
>
> Should fix bugs and memory leaks in original changelist.
>
> ««« original CL description
> undo CL 104200047 / 318b04f28372
>
> Breaks windows and race detector.
> TBR=rsc
>
> ««« original CL description
> runtime: stack allocator, separate from mallocgc
>
> In order to move malloc to Go, we need to have a
> separate stack allocator.  If we run out of stack
> during malloc, malloc will not be available
> to allocate a new stack.
>
> Stacks are the last remaining FlagNoGC objects in the
> GC heap.  Once they are out, we can get rid of the
> distinction between the allocated/blockboundary bits.
> (This will be in a separate change.)
>
> Fixes issue 7468
> Fixes issue 7424
>
> LGTM=rsc, dvyukov
> R=golang-codereviews, dvyukov, khr, dave, rsc
> CC=golang-codereviews
> https://codereview.appspot.com/104200047
> »»»
>
> TBR=rsc
> CC=golang-codereviews
> https://codereview.appspot.com/101570044
> »»»
>
> LGTM=dvyukov
> R=dvyukov, dave, khr, alex.brainman
> CC=golang-codereviews
> https://codereview.appspot.com/112240044
>
>
> https://codereview.appspot.com/112240044/

[khr1, 2014-07-18 22:59:31]

Fix at https://codereview.appspot.com/112450045/


On Fri, Jul 18, 2014 at 3:21 AM, Dmitry Vyukov <dvyukov@google.com> wrote:

> Perf dashboard reports 7036874417757228.00% increase in stack usage:
>
>
http://goperfd.appspot.com/perfdetail?commit=a622a4ff09da1ca11880d1b58fc122ac...
>
>
>
>
> On Fri, Jul 18, 2014 at 1:41 AM,  <khr@golang.org> wrote:
> > *** Submitted as
> > https://code.google.com/p/go/source/detail?r=bac0a67b0b70 ***
> >
> >
> > undo CL 101570044 / 2c57aaea79c4
> >
> > redo stack allocation.  This is mostly the same as
> > the original CL with a few bug fixes.
> >
> > 1. add racemalloc() for stack allocations
> > 2. fix poolalloc/poolfree to terminate free lists correctly.
> > 3. adjust span ref count correctly.
> > 4. don't use cache for sizes >= StackCacheSize.
> >
> > Should fix bugs and memory leaks in original changelist.
> >
> > ««« original CL description
> > undo CL 104200047 / 318b04f28372
> >
> > Breaks windows and race detector.
> > TBR=rsc
> >
> > ««« original CL description
> > runtime: stack allocator, separate from mallocgc
> >
> > In order to move malloc to Go, we need to have a
> > separate stack allocator.  If we run out of stack
> > during malloc, malloc will not be available
> > to allocate a new stack.
> >
> > Stacks are the last remaining FlagNoGC objects in the
> > GC heap.  Once they are out, we can get rid of the
> > distinction between the allocated/blockboundary bits.
> > (This will be in a separate change.)
> >
> > Fixes issue 7468
> > Fixes issue 7424
> >
> > LGTM=rsc, dvyukov
> > R=golang-codereviews, dvyukov, khr, dave, rsc
> > CC=golang-codereviews
> > https://codereview.appspot.com/104200047
> > »»»
> >
> > TBR=rsc
> > CC=golang-codereviews
> > https://codereview.appspot.com/101570044
> > »»»
> >
> > LGTM=dvyukov
> > R=dvyukov, dave, khr, alex.brainman
> > CC=golang-codereviews
> > https://codereview.appspot.com/112240044
> >
> >
> > https://codereview.appspot.com/112240044/
>