code review 76380043: runtime: redo stack map entries to avoid false retention

runtime: redo stack map entries to avoid false retention

Change two-bit stack map entries to encode:
0 = dead
1 = scalar
2 = pointer
3 = multiword

If multiword, the two-bit entry for the following word encodes:
0 = string
1 = slice
2 = iface
3 = eface

That way, during stack scanning we can check if a string
is zero length or a slice has zero capacity.  We can avoid
following the contained pointer in those cases.  It is safe
to do so because it can never be dereferenced, and it is
desirable to do so because it may cause false retention
of the following block in memory.

Slice feature turned off until issue 7564 is fixed.

Update issue 7549

[khr, 2014-03-19 07:55:55]

Hello golang-codereviews@googlegroups.com,

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

[bradfitz, 2014-03-19 16:00:47]

s/bug NNNN/issue NNNN/ for the auto-linking/updating magic.



On Wed, Mar 19, 2014 at 12:55 AM, <khr@golang.org> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> runtime: redo stack map entries to avoid false retention
>
> Change two-bit stack map entries to encode:
> 0 = dead
> 1 = scalar
> 2 = pointer
> 3 = multiword
>
> If multiword, the two-bit entry for the following word encodes:
> 0 = string
> 1 = slice
> 2 = iface
> 3 = eface
>
> That way, during stack scanning we can check if a string
> is zero length or a slice has zero capacity.  We can avoid
> following the contained pointer in those cases.  It is safe
> to do so because it can never be dereferenced, and it is
> desirable to do so because it may cause false retention
> of the following block in memory.
>
> Slice feature turned off until bug 7564 is fixed.
>
> Update bug 7549
>
> Please review this at https://codereview.appspot.com/76380043/
>
> Affected files (+213, -37 lines):
>   M src/cmd/cc/pgen.c
>   M src/cmd/gc/plive.c
>   M src/pkg/runtime/malloc.h
>   M src/pkg/runtime/mfinal_test.go
>   M src/pkg/runtime/mgc0.c
>   M src/pkg/runtime/stack.c
>
>
> --
> 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.
>

[khr, 2014-03-25 16:28:35]

On 2014/03/19 16:00:47, bradfitz wrote:
> s/bug NNNN/issue NNNN/ for the auto-linking/updating magic.
> 
> 
> 
> On Wed, Mar 19, 2014 at 12:55 AM, <mailto:khr@golang.org> wrote:
> 
> > Reviewers: golang-codereviews,
> >
> > Message:
> > Hello mailto:golang-codereviews@googlegroups.com,
> >
> > I'd like you to review this change to
> > https://code.google.com/p/go/
> >
> >
> > Description:
> > runtime: redo stack map entries to avoid false retention
> >
> > Change two-bit stack map entries to encode:
> > 0 = dead
> > 1 = scalar
> > 2 = pointer
> > 3 = multiword
> >
> > If multiword, the two-bit entry for the following word encodes:
> > 0 = string
> > 1 = slice
> > 2 = iface
> > 3 = eface
> >
> > That way, during stack scanning we can check if a string
> > is zero length or a slice has zero capacity.  We can avoid
> > following the contained pointer in those cases.  It is safe
> > to do so because it can never be dereferenced, and it is
> > desirable to do so because it may cause false retention
> > of the following block in memory.
> >
> > Slice feature turned off until bug 7564 is fixed.
> >
> > Update bug 7549
> >
> > Please review this at https://codereview.appspot.com/76380043/
> >
> > Affected files (+213, -37 lines):
> >   M src/cmd/cc/pgen.c
> >   M src/cmd/gc/plive.c
> >   M src/pkg/runtime/malloc.h
> >   M src/pkg/runtime/mfinal_test.go
> >   M src/pkg/runtime/mgc0.c
> >   M src/pkg/runtime/stack.c
> >
> >
> > --
> > 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 mailto:golang-codereviews+unsubscribe@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.
> >

Ping.

[rsc, 2014-03-25 19:41:49]

LGTM

i love it

https://codereview.appspot.com/76380043/diff/100001/src/pkg/runtime/mgc0.c
File src/pkg/runtime/mgc0.c (right):

https://codereview.appspot.com/76380043/diff/100001/src/pkg/runtime/mgc0.c#ne...
src/pkg/runtime/mgc0.c:70: ClobberDead = 0,
please make this a field in runtime.debug and then list in dbgvars so we can
turn it on using GODEBUG. let's call it "gcdead". and document in extern.go's
discussion of GODEBUG.

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

https://codereview.appspot.com/76380043/diff/100001/src/pkg/runtime/stack.c#n...
src/pkg/runtime/stack.c:358: // TODO: clobber in debug mode?
sgtm

[khr, 2014-03-25 21:11:41]

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

runtime: redo stack map entries to avoid false retention

Change two-bit stack map entries to encode:
0 = dead
1 = scalar
2 = pointer
3 = multiword

If multiword, the two-bit entry for the following word encodes:
0 = string
1 = slice
2 = iface
3 = eface

That way, during stack scanning we can check if a string
is zero length or a slice has zero capacity.  We can avoid
following the contained pointer in those cases.  It is safe
to do so because it can never be dereferenced, and it is
desirable to do so because it may cause false retention
of the following block in memory.

Slice feature turned off until issue 7564 is fixed.

Update issue 7549

LGTM=rsc
R=golang-codereviews, bradfitz, rsc
CC=golang-codereviews
https://codereview.appspot.com/76380043