code review 5265044: runtime: run goroutines during init

runtime: run goroutines during init

Fixes issue 583.
Fixes issue 1776.
Fixes issue 2001.
Fixes issue 2112.

[rsc, 2011-10-13 16:45:12]

Hello golang-dev@googlegroups.com,

I'd like you to review this change to
https://go.googlecode.com/hg/

[bradfitz, 2011-10-13 16:49:37]

LGTM

I liked the predawn variable name, though.

On Thu, Oct 13, 2011 at 9:45 AM, <rsc@golang.org> wrote:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com,
>
> I'd like you to review this change to
> https://go.googlecode.com/hg/
>
>
> Description:
> runtime: run goroutines during init
>
> Fixes issue 583.
> Fixes issue 1776.
> Fixes issue 2001.
> Fixes issue 2112.
>
> Please review this at
http://codereview.appspot.com/**5265044/<http://codereview.appspot.com/5265044/>
>
> Affected files:
>  M doc/go_spec.html
>  M src/pkg/runtime/386/asm.s
>  M src/pkg/runtime/amd64/asm.s
>  M src/pkg/runtime/arm/asm.s
>  M src/pkg/runtime/proc.c
>  A test/init1.go
>
>
> Index: doc/go_spec.html
> ==============================**==============================**=======
> --- a/doc/go_spec.html
> +++ b/doc/go_spec.html
> @@ -5091,12 +5091,6 @@
>  in another package that refers to <code>B</code>.
>  </p>
>  <p>
> -Initialization code may contain "go" statements, but the functions
> -they invoke do not begin execution until initialization of the entire
> -program is complete. Therefore, all initialization code is run in a single
> -goroutine.
> -</p>
> -<p>
>  An <code>init</code> function cannot be referred to from anywhere
>  in a program. In particular, <code>init</code> cannot be called
> explicitly,
>  nor can a pointer to <code>init</code> be assigned to a function variable.
> Index: src/pkg/runtime/386/asm.s
> ==============================**==============================**=======
> --- a/src/pkg/runtime/386/asm.s
> +++ b/src/pkg/runtime/386/asm.s
> @@ -92,7 +92,6 @@
>
>  TEXT runtime·mainstart(SB),7,$0
>        CALL    main·init(SB)
> -       CALL    runtime·initdone(SB)
>        CALL    main·main(SB)
>        PUSHL   $0
>        CALL    runtime·exit(SB)
> Index: src/pkg/runtime/amd64/asm.s
> ==============================**==============================**=======
> --- a/src/pkg/runtime/amd64/asm.s
> +++ b/src/pkg/runtime/amd64/asm.s
> @@ -74,7 +74,6 @@
>
>  TEXT runtime·mainstart(SB),7,$0
>        CALL    main·init(SB)
> -       CALL    runtime·initdone(SB)
>        CALL    main·main(SB)
>        PUSHQ   $0
>        CALL    runtime·exit(SB)
> Index: src/pkg/runtime/arm/asm.s
> ==============================**==============================**=======
> --- a/src/pkg/runtime/arm/asm.s
> +++ b/src/pkg/runtime/arm/asm.s
> @@ -63,7 +63,6 @@
>
>  TEXT runtime·mainstart(SB),7,$4
>        BL      main·init(SB)
> -       BL      runtime·initdone(SB)
>        EOR     R0, R0
>        MOVW    R0, 0(R13)
>        BL      main·main(SB)
> Index: src/pkg/runtime/proc.c
> ==============================**==============================**=======
> --- a/src/pkg/runtime/proc.c
> +++ b/src/pkg/runtime/proc.c
> @@ -71,7 +71,6 @@
>
>        volatile uint32 atomic; // atomic scheduling word (see below)
>
> -       int32 predawn;          // running initialization, don't run new
> g's.
>        int32 profilehz;        // cpu profiling rate
>
>        Note    stopped;        // one g can set waitstop and wait here for
> m's to stop
> @@ -202,8 +201,8 @@
>        }
>        setmcpumax(runtime·gomaxprocs)**;
>        runtime·singleproc = runtime·gomaxprocs == 1;
> -       runtime·sched.predawn = 1;
>
> +       mstats.enablegc = 1;
>        m->nomemprof--;
>  }
>
> @@ -227,22 +226,6 @@
>                runtime·notewakeup(&m->**havenextg);
>  }
>
> -// Called after main·init_function; main·main will be called on return.
> -void
> -runtime·initdone(void)
> -{
> -       // Let's go.
> -       runtime·sched.predawn = 0;
> -       mstats.enablegc = 1;
> -
> -       // If main·init_function started other goroutines,
> -       // kick off new m's to handle them, like ready
> -       // would have, had it not been pre-dawn.
> -       schedlock();
> -       matchmg();
> -       schedunlock();
> -}
> -
>  void
>  runtime·goexit(void)
>  {
> @@ -467,8 +450,7 @@
>        g->status = Grunnable;
>
>        gput(g);
> -       if(!runtime·sched.predawn)
> -               matchmg();
> +       matchmg();
>  }
>
>  static void
> @@ -793,9 +775,6 @@
>
>        schedlock();
>        if(gp != nil) {
> -               if(runtime·sched.predawn)
> -                       runtime·throw("init rescheduling");
> -
>                // Just finished running gp.
>                gp->m = nil;
>                runtime·sched.grunning--;
> @@ -893,9 +872,6 @@
>  {
>        uint32 v;
>
> -       if(runtime·sched.predawn)
> -               return;
> -
>        // Leave SP around for gc and traceback.
>        runtime·gosave(&g->sched);
>        g->gcsp = g->sched.sp;
> @@ -947,9 +923,6 @@
>  {
>        uint32 v;
>
> -       if(runtime·sched.predawn)
> -               return;
> -
>        // Fast path.
>        // If we can do the mcpu++ bookkeeping and
>        // find that we still have mcpu <= mcpumax, then we can
> @@ -1513,8 +1486,6 @@
>  void
>  runtime·LockOSThread(void)
>  {
> -       if(runtime·sched.predawn)
> -               runtime·throw("cannot wire during init");
>        m->lockedg = g;
>        g->lockedm = m;
>  }
> Index: test/init1.go
> ==============================**==============================**=======
> new file mode 100644
> --- /dev/null
> +++ b/test/init1.go
> @@ -0,0 +1,44 @@
> +// $G $D/$F.go && $L $F.$A && ./$A.out
> +
> +// Copyright 2011 The Go Authors.  All rights reserved.
> +// Use of this source code is governed by a BSD-style
> +// license that can be found in the LICENSE file.
> +
> +// Test that goroutines and garbage collection run during init.
> +
> +package main
> +
> +import "runtime"
> +
> +var x []byte
> +
> +func init() {
> +       c := make(chan int)
> +       go send(c)
> +       <-c
> +
> +       const chunk = 1<<20
> +       runtime.UpdateMemStats()
> +       sys := runtime.MemStats.Sys
> +       b := make([]byte, chunk)
> +       for i := range b {
> +               b[i] = byte(i%10 + '0')
> +       }
> +       s := string(b)
> +       for i := 0; i < 1000; i++ {
> +               x = []byte(s)
> +       }
> +       runtime.UpdateMemStats()
> +       sys1 := runtime.MemStats.Sys
> +       if sys1-sys > chunk*50 {
> +               println("allocated 1000 chunks of", chunk, "and used ",
> sys1-sys, "memory")
> +       }
> +}
> +
> +func send(c chan int) {
> +       c <- 1
> +}
> +
> +func main() {
> +}
> +
>
>
>

[r, 2011-10-13 19:07:06]

LGTM
nice spec change

[gri, 2011-10-13 19:14:54]

LGTM
(spec change)

http://codereview.appspot.com/5265044/diff/4001/doc/go_spec.html
File doc/go_spec.html (right):

http://codereview.appspot.com/5265044/diff/4001/doc/go_spec.html#newcode2
doc/go_spec.html:2: <!-- subtitle Version of September 29, 2011 -->
I assume the date will self-correct once the previous CL is submitted/synced.

[rsc, 2011-10-13 19:54:27]

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

runtime: run goroutines during init

Fixes issue 583.
Fixes issue 1776.
Fixes issue 2001.
Fixes issue 2112.

R=golang-dev, bradfitz, r, gri
CC=golang-dev
http://codereview.appspot.com/5265044

[rog, 2011-10-14 06:44:33]

i'm really happy about this.
i hope it works out.

On 13 October 2011 20:54,  <rsc@golang.org> wrote:
> *** Submitted as
> http://code.google.com/p/go/source/detail?r=962818141467 ***
>
> runtime: run goroutines during init
>
> Fixes issue 583.
> Fixes issue 1776.
> Fixes issue 2001.
> Fixes issue 2112.
>
> R=golang-dev, bradfitz, r, gri
> CC=golang-dev
> http://codereview.appspot.com/5265044
>
>
> http://codereview.appspot.com/5265044/
>