code review 12724044: runtime: remove unused m->racepc

runtime: remove unused m->racepc
The original plan was to collect allocation stacks
for all memory blocks. But it was never implemented
and it's not in near plans and it's unclear how to do it at all.

[dvyukov, 2013-08-12 08:12:26]

Hello golang-dev@googlegroups.com,

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

[dave_cheney.net, 2013-08-12 10:23:17]

SGTM. 



On 12/08/2013, at 17:12, dvyukov@google.com wrote:

> Reviewers: golang-dev1,
> 
> Message:
> Hello golang-dev@googlegroups.com,
> 
> I'd like you to review this change to
> https://dvyukov%40google.com@code.google.com/p/go/
> 
> 
> Description:
> runtime: remove unused m->racepc
> The original plan was to collect allocation stacks
> for all memory blocks. But it was never implemented
> and it's not in near plans and it's unclear how to do it at all.
> 
> Please review this at https://codereview.appspot.com/12724044/
> 
> Affected files:
>  M src/pkg/runtime/malloc.goc
>  M src/pkg/runtime/race.c
>  M src/pkg/runtime/race.h
>  M src/pkg/runtime/race0.c
>  M src/pkg/runtime/runtime.h
> 
> 
> Index: src/pkg/runtime/malloc.goc
> ===================================================================
> --- a/src/pkg/runtime/malloc.goc
> +++ b/src/pkg/runtime/malloc.goc
> @@ -141,10 +141,8 @@
>    if(!(flag & FlagNoInvokeGC) && mstats.heap_alloc >= mstats.next_gc)
>        runtime·gc(0);
> 
> -    if(raceenabled) {
> -        runtime·racemalloc(v, size, m->racepc);
> -        m->racepc = nil;
> -    }
> +    if(raceenabled)
> +        runtime·racemalloc(v, size);
>    return v;
> }
> 
> @@ -702,8 +700,6 @@
> void
> runtime·new(Type *typ, uint8 *ret)
> {
> -    if(raceenabled)
> -        m->racepc = runtime·getcallerpc(&typ);
>    ret = runtime·mallocgc(typ->size, (uintptr)typ | TypeInfo_SingleObject,
typ->kind&KindNoPointers ? FlagNoPointers : 0);
>    FLUSH(&ret);
> }
> Index: src/pkg/runtime/race.c
> ===================================================================
> --- a/src/pkg/runtime/race.c
> +++ b/src/pkg/runtime/race.c
> @@ -138,13 +138,13 @@
> }
> 
> void
> -runtime·racemalloc(void *p, uintptr sz, void *pc)
> +runtime·racemalloc(void *p, uintptr sz)
> {
>    // use m->curg because runtime·stackalloc() is called from g0
>    if(m->curg == nil)
>        return;
>    m->racecall = true;
> -    runtime/race·Malloc(m->curg->racectx, p, sz, pc);
> +    runtime/race·Malloc(m->curg->racectx, p, sz, /* unused pc */ 0);
>    m->racecall = false;
> }
> 
> Index: src/pkg/runtime/race.h
> ===================================================================
> --- a/src/pkg/runtime/race.h
> +++ b/src/pkg/runtime/race.h
> @@ -16,7 +16,7 @@
> void    runtime·racefini(void);
> 
> void    runtime·racemapshadow(void *addr, uintptr size);
> -void    runtime·racemalloc(void *p, uintptr sz, void *pc);
> +void    runtime·racemalloc(void *p, uintptr sz);
> void    runtime·racefree(void *p);
> uintptr    runtime·racegostart(void *pc);
> void    runtime·racegoend(void);
> Index: src/pkg/runtime/race0.c
> ===================================================================
> --- a/src/pkg/runtime/race0.c
> +++ b/src/pkg/runtime/race0.c
> @@ -105,11 +105,10 @@
> }
> 
> void
> -runtime·racemalloc(void *p, uintptr sz, void *pc)
> +runtime·racemalloc(void *p, uintptr sz)
> {
>    USED(p);
>    USED(sz);
> -    USED(pc);
> }
> 
> void
> Index: src/pkg/runtime/runtime.h
> ===================================================================
> --- a/src/pkg/runtime/runtime.h
> +++ b/src/pkg/runtime/runtime.h
> @@ -339,7 +339,6 @@
>    GCStats    gcstats;
>    bool    racecall;
>    bool    needextram;
> -    void*    racepc;
>    void    (*waitunlockf)(Lock*);
>    void*    waitlock;
> 
> 
> 
> -- 
> 
> ---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.
> 
> 

[bradfitz, 2013-08-12 15:44:56]

LGTM



On Mon, Aug 12, 2013 at 1:12 AM, <dvyukov@google.com> wrote:

> Reviewers: golang-dev1,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
>
https://dvyukov%40google.com@**code.google.com/p/go/<http://40google.com@code...
>
>
> Description:
> runtime: remove unused m->racepc
> The original plan was to collect allocation stacks
> for all memory blocks. But it was never implemented
> and it's not in near plans and it's unclear how to do it at all.
>
> Please review this at
https://codereview.appspot.**com/12724044/<https://codereview.appspot.com/127...
>
> Affected files:
>   M src/pkg/runtime/malloc.goc
>   M src/pkg/runtime/race.c
>   M src/pkg/runtime/race.h
>   M src/pkg/runtime/race0.c
>   M src/pkg/runtime/runtime.h
>
>
> Index: src/pkg/runtime/malloc.goc
> ==============================**==============================**=======
> --- a/src/pkg/runtime/malloc.goc
> +++ b/src/pkg/runtime/malloc.goc
> @@ -141,10 +141,8 @@
>         if(!(flag & FlagNoInvokeGC) && mstats.heap_alloc >= mstats.next_gc)
>                 runtime·gc(0);
>
> -       if(raceenabled) {
> -               runtime·racemalloc(v, size, m->racepc);
> -               m->racepc = nil;
> -       }
> +       if(raceenabled)
> +               runtime·racemalloc(v, size);
>         return v;
>  }
>
> @@ -702,8 +700,6 @@
>  void
>  runtime·new(Type *typ, uint8 *ret)
>  {
> -       if(raceenabled)
> -               m->racepc = runtime·getcallerpc(&typ);
>         ret = runtime·mallocgc(typ->size, (uintptr)typ |
> TypeInfo_SingleObject, typ->kind&KindNoPointers ? FlagNoPointers : 0);
>         FLUSH(&ret);
>  }
> Index: src/pkg/runtime/race.c
> ==============================**==============================**=======
> --- a/src/pkg/runtime/race.c
> +++ b/src/pkg/runtime/race.c
> @@ -138,13 +138,13 @@
>  }
>
>  void
> -runtime·racemalloc(void *p, uintptr sz, void *pc)
> +runtime·racemalloc(void *p, uintptr sz)
>  {
>         // use m->curg because runtime·stackalloc() is called from g0
>         if(m->curg == nil)
>                 return;
>         m->racecall = true;
> -       runtime/race·Malloc(m->curg->**racectx, p, sz, pc);
> +       runtime/race·Malloc(m->curg->**racectx, p, sz, /* unused pc */ 0);
>         m->racecall = false;
>  }
>
> Index: src/pkg/runtime/race.h
> ==============================**==============================**=======
> --- a/src/pkg/runtime/race.h
> +++ b/src/pkg/runtime/race.h
> @@ -16,7 +16,7 @@
>  void   runtime·racefini(void);
>
>  void   runtime·racemapshadow(void *addr, uintptr size);
> -void   runtime·racemalloc(void *p, uintptr sz, void *pc);
> +void   runtime·racemalloc(void *p, uintptr sz);
>  void   runtime·racefree(void *p);
>  uintptr        runtime·racegostart(void *pc);
>  void   runtime·racegoend(void);
> Index: src/pkg/runtime/race0.c
> ==============================**==============================**=======
> --- a/src/pkg/runtime/race0.c
> +++ b/src/pkg/runtime/race0.c
> @@ -105,11 +105,10 @@
>  }
>
>  void
> -runtime·racemalloc(void *p, uintptr sz, void *pc)
> +runtime·racemalloc(void *p, uintptr sz)
>  {
>         USED(p);
>         USED(sz);
> -       USED(pc);
>  }
>
>  void
> Index: src/pkg/runtime/runtime.h
> ==============================**==============================**=======
> --- a/src/pkg/runtime/runtime.h
> +++ b/src/pkg/runtime/runtime.h
> @@ -339,7 +339,6 @@
>         GCStats gcstats;
>         bool    racecall;
>         bool    needextram;
> -       void*   racepc;
>         void    (*waitunlockf)(Lock*);
>         void*   waitlock;
>
>
>
> --
>
> ---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...
> .
>
>
>

[dvyukov, 2013-08-12 17:48:23]

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

runtime: remove unused m->racepc
The original plan was to collect allocation stacks
for all memory blocks. But it was never implemented
and it's not in near plans and it's unclear how to do it at all.

R=golang-dev, dave, bradfitz
CC=golang-dev
https://codereview.appspot.com/12724044