code review 7744047: runtime: scan the type of an interface value

runtime: scan the type of an interface value

[atom, 2013-03-15 18:44:41]

Hello golang-dev@googlegroups.com (cc: rsc),

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

[rsc, 2013-03-15 18:50:15]

LGTM

[bradfitz, 2013-03-15 19:16:57]

Just curious: are changes like this not testable with a minimal go program
and runtime.GC and runtime.SetFinalizer?

On Fri, Mar 15, 2013 at 11:44 AM, <0xE2.0x9A.0x9B@gmail.com> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com (cc: rsc),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> runtime: scan the type of an interface value
>
> Please review this at
https://codereview.appspot.**com/7744047/<https://codereview.appspot.com/7744...
>
> Affected files:
>   M src/pkg/runtime/mgc0.c
>
>
> Index: src/pkg/runtime/mgc0.c
> ==============================**==============================**=======
> --- a/src/pkg/runtime/mgc0.c
> +++ b/src/pkg/runtime/mgc0.c
> @@ -754,11 +754,22 @@
>                 case GC_EFACE:
>                         eface = (Eface*)(stack_top.b + pc[1]);
>                         pc += 2;
> -                       if(eface->type != nil && (eface->data >=
> arena_start && eface->data < arena_used)) {
> -                               t = eface->type;
> +                       if(eface->type == nil)
> +                               continue;
> +
> +                       // eface->type
> +                       t = eface->type;
> +                       if((void*)t >= arena_start && (void*)t <
> arena_used) {
> +                               *ptrbufpos++ = (PtrTarget){t, 0};
> +                               if(ptrbufpos == ptrbuf_end)
> +                                       flushptrbuf(ptrbuf, &ptrbufpos,
> &wp, &wbuf, &nobj);
> +                       }
> +
> +                       // eface->data
> +                       if(eface->data >= arena_start && eface->data <
> arena_used) {
>                                 if(t->size <= sizeof(void*)) {
>                                         if((t->kind & KindNoPointers))
> -                                               break;
> +                                               continue;
>
>                                         obj = eface->data;
>                                         if((t->kind & ~KindNoPointers) ==
> KindPtr)
> @@ -774,7 +785,7 @@
>                         iface = (Iface*)(stack_top.b + pc[1]);
>                         pc += 2;
>                         if(iface->tab == nil)
> -                               break;
> +                               continue;
>
>                         // iface->tab
>                         if((void*)iface->tab >= arena_start &&
> (void*)iface->tab < arena_used) {
> @@ -788,7 +799,7 @@
>                                 t = iface->tab->type;
>                                 if(t->size <= sizeof(void*)) {
>                                         if((t->kind & KindNoPointers))
> -                                               break;
> +                                               continue;
>
>                                         obj = iface->data;
>                                         if((t->kind & ~KindNoPointers) ==
> KindPtr)
> @@ -815,9 +826,8 @@
>                 case GC_END:
>                         if(--stack_top.count != 0) {
>                                 // Next iteration of a loop if possible.
> -                               elemsize = stack_top.elemsize;
> -                               stack_top.b += elemsize;
> -                               if(stack_top.b + elemsize <=
> end_b+PtrSize) {
> +                               stack_top.b += stack_top.elemsize;
> +                               if(stack_top.b + stack_top.elemsize <=
> end_b+PtrSize) {
>                                         pc = stack_top.loop_or_ret;
>                                         continue;
>                                 }
> @@ -945,7 +955,7 @@
>                         *objbufpos++ = (Obj){obj, size, objti};
>                         if(objbufpos == objbuf_end)
>                                 flushobjbuf(objbuf, &objbufpos, &wp,
> &wbuf, &nobj);
> -                       break;
> +                       continue;
>
>                 case GC_CHAN:
>                         // There are no heap pointers in struct Hchan,
>
>
> --
>
> ---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<golang-dev%2Bunsubscribe@googlegrou...
> .
> For more options, visit
https://groups.google.com/**groups/opt_out<https://groups.google.com/groups/o...
> .
>
>
>

[rsc, 2013-03-15 19:20:16]

I'll add a good test in the reflect package as part of the work I'm doing.

[atom, 2013-03-15 19:35:58]

On 2013/03/15 18:50:15, rsc wrote:
> LGTM

"hg submit 7744047" doesn't seem to be working on my machine.

[rsc, 2013-03-15 19:43:25]

On Fri, Mar 15, 2013 at 3:35 PM, <0xE2.0x9A.0x9B@gmail.com> wrote:

> On 2013/03/15 18:50:15, rsc wrote:
>
>> LGTM
>>
>
> "hg submit 7744047" doesn't seem to be working on my machine.
>

hg commands seem incredibly slow today. i don't know which server is to
blame.

[rsc, 2013-03-15 20:07:56]

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

runtime: scan the type of an interface value

R=golang-dev, rsc, bradfitz
CC=golang-dev
https://codereview.appspot.com/7744047

Committer: Russ Cox <rsc@golang.org>