code review 136380043: cmd/gc: emit write barriers

cmd/gc: emit write barriers

A write *p = x that needs a write barrier (not all do)
now turns into runtime.writebarrierptr(p, x)
or one of the other variants.

The write barrier implementations are trivial.
The goal here is to emit the calls in the correct places
and to incur the cost of those function calls in the Go 1.4 cycle.

Performance on the Go 1 benchmark suite below.
Remember, the goal is to slow things down (and be correct).

We will look into optimizations in separate CLs, as part of
the process of comparing Go 1.3 against tip in order to make
sure Go 1.4 runs at least as fast as Go 1.3.

benchmark                          old ns/op      new ns/op      delta
BenchmarkBinaryTree17              3118336716     3452876110     +10.73%
BenchmarkFannkuch11                3184497677     3211552284     +0.85%
BenchmarkFmtFprintfEmpty           89.9           107            +19.02%
BenchmarkFmtFprintfString          236            287            +21.61%
BenchmarkFmtFprintfInt             246            278            +13.01%
BenchmarkFmtFprintfIntInt          395            458            +15.95%
BenchmarkFmtFprintfPrefixedInt     343            378            +10.20%
BenchmarkFmtFprintfFloat           477            525            +10.06%
BenchmarkFmtManyArgs               1446           1707           +18.05%
BenchmarkGobDecode                 14398047       14685958       +2.00%
BenchmarkGobEncode                 12557718       12947104       +3.10%
BenchmarkGzip                      453462345      472413285      +4.18%
BenchmarkGunzip                    114226016      115127398      +0.79%
BenchmarkHTTPClientServer          114689         112122         -2.24%
BenchmarkJSONEncode                24914536       26135942       +4.90%
BenchmarkJSONDecode                86832877       103620289      +19.33%
BenchmarkMandelbrot200             4833452        4898780        +1.35%
BenchmarkGoParse                   4317976        4835474        +11.98%
BenchmarkRegexpMatchEasy0_32       150            166            +10.67%
BenchmarkRegexpMatchEasy0_1K       393            402            +2.29%
BenchmarkRegexpMatchEasy1_32       125            142            +13.60%
BenchmarkRegexpMatchEasy1_1K       1010           1236           +22.38%
BenchmarkRegexpMatchMedium_32      232            301            +29.74%
BenchmarkRegexpMatchMedium_1K      76963          102721         +33.47%
BenchmarkRegexpMatchHard_32        3833           5463           +42.53%
BenchmarkRegexpMatchHard_1K        119668         161614         +35.05%
BenchmarkRevcomp                   763449047      706768534      -7.42%
BenchmarkTemplate                  124954724      134834549      +7.91%
BenchmarkTimeParse                 517            511            -1.16%
BenchmarkTimeFormat                501            514            +2.59%

benchmark                         old MB/s     new MB/s     speedup
BenchmarkGobDecode                53.31        52.26        0.98x
BenchmarkGobEncode                61.12        59.28        0.97x
BenchmarkGzip                     42.79        41.08        0.96x
BenchmarkGunzip                   169.88       168.55       0.99x
BenchmarkJSONEncode               77.89        74.25        0.95x
BenchmarkJSONDecode               22.35        18.73        0.84x
BenchmarkGoParse                  13.41        11.98        0.89x
BenchmarkRegexpMatchEasy0_32      213.30       191.72       0.90x
BenchmarkRegexpMatchEasy0_1K      2603.92      2542.74      0.98x
BenchmarkRegexpMatchEasy1_32      254.00       224.93       0.89x
BenchmarkRegexpMatchEasy1_1K      1013.53      827.98       0.82x
BenchmarkRegexpMatchMedium_32     4.30         3.31         0.77x
BenchmarkRegexpMatchMedium_1K     13.30        9.97         0.75x
BenchmarkRegexpMatchHard_32       8.35         5.86         0.70x
BenchmarkRegexpMatchHard_1K       8.56         6.34         0.74x
BenchmarkRevcomp                  332.92       359.62       1.08x
BenchmarkTemplate                 15.53        14.39        0.93x

[rsc, 2014-09-08 21:04:56]

Hello rlh (cc: dvyukov, golang-codereviews@googlegroups.com, iant, khr, r),

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

[rlh, 2014-09-08 21:51:00]

https://codereview.appspot.com/136380043/diff/40001/src/cmd/gc/walk.c
File src/cmd/gc/walk.c (right):

https://codereview.appspot.com/136380043/diff/40001/src/cmd/gc/walk.c#newcode...
src/cmd/gc/walk.c:2014: // No write barrier for initialization to constant.
When would this be possible. The check for nil is at 2011, what other
constant literals exist?

[rsc, 2014-09-09 18:04:47]

https://codereview.appspot.com/136380043/diff/40001/src/cmd/gc/walk.c
File src/cmd/gc/walk.c (right):

https://codereview.appspot.com/136380043/diff/40001/src/cmd/gc/walk.c#newcode...
src/cmd/gc/walk.c:2014: // No write barrier for initialization to constant.
On 2014/09/08 21:51:00, rlh wrote:
> When would this be possible. The check for nil is at 2011, what other
> constant literals exist?

The check at 2011 is checking for the C variable r being NULL (except Ken calls
it N here), which indicates an initialization to zero (for example, in a
declaration without an initializer).

An explicit nil in the source code turns into an OLITERAL node here. So would
"hello" (a different OLITERAL node, of course). Those are the only ones I can
think of that involve pointers.

[rlh, 2014-09-09 18:18:05]

On 2014/09/09 18:04:47, rsc wrote:
> https://codereview.appspot.com/136380043/diff/40001/src/cmd/gc/walk.c
> File src/cmd/gc/walk.c (right):
> 
>
https://codereview.appspot.com/136380043/diff/40001/src/cmd/gc/walk.c#newcode...
> src/cmd/gc/walk.c:2014: // No write barrier for initialization to constant.
> On 2014/09/08 21:51:00, rlh wrote:
> > When would this be possible. The check for nil is at 2011, what other
> > constant literals exist?
> 
> The check at 2011 is checking for the C variable r being NULL (except Ken
calls
> it N here), which indicates an initialization to zero (for example, in a
> declaration without an initializer).
> 
> An explicit nil in the source code turns into an OLITERAL node here. So would
> "hello" (a different OLITERAL node, of course). Those are the only ones I can
> think of that involve pointers.

LGTM

[rsc, 2014-09-11 16:17:44]

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

cmd/gc: emit write barriers

A write *p = x that needs a write barrier (not all do)
now turns into runtime.writebarrierptr(p, x)
or one of the other variants.

The write barrier implementations are trivial.
The goal here is to emit the calls in the correct places
and to incur the cost of those function calls in the Go 1.4 cycle.

Performance on the Go 1 benchmark suite below.
Remember, the goal is to slow things down (and be correct).

We will look into optimizations in separate CLs, as part of
the process of comparing Go 1.3 against tip in order to make
sure Go 1.4 runs at least as fast as Go 1.3.

benchmark                          old ns/op      new ns/op      delta
BenchmarkBinaryTree17              3118336716     3452876110     +10.73%
BenchmarkFannkuch11                3184497677     3211552284     +0.85%
BenchmarkFmtFprintfEmpty           89.9           107            +19.02%
BenchmarkFmtFprintfString          236            287            +21.61%
BenchmarkFmtFprintfInt             246            278            +13.01%
BenchmarkFmtFprintfIntInt          395            458            +15.95%
BenchmarkFmtFprintfPrefixedInt     343            378            +10.20%
BenchmarkFmtFprintfFloat           477            525            +10.06%
BenchmarkFmtManyArgs               1446           1707           +18.05%
BenchmarkGobDecode                 14398047       14685958       +2.00%
BenchmarkGobEncode                 12557718       12947104       +3.10%
BenchmarkGzip                      453462345      472413285      +4.18%
BenchmarkGunzip                    114226016      115127398      +0.79%
BenchmarkHTTPClientServer          114689         112122         -2.24%
BenchmarkJSONEncode                24914536       26135942       +4.90%
BenchmarkJSONDecode                86832877       103620289      +19.33%
BenchmarkMandelbrot200             4833452        4898780        +1.35%
BenchmarkGoParse                   4317976        4835474        +11.98%
BenchmarkRegexpMatchEasy0_32       150            166            +10.67%
BenchmarkRegexpMatchEasy0_1K       393            402            +2.29%
BenchmarkRegexpMatchEasy1_32       125            142            +13.60%
BenchmarkRegexpMatchEasy1_1K       1010           1236           +22.38%
BenchmarkRegexpMatchMedium_32      232            301            +29.74%
BenchmarkRegexpMatchMedium_1K      76963          102721         +33.47%
BenchmarkRegexpMatchHard_32        3833           5463           +42.53%
BenchmarkRegexpMatchHard_1K        119668         161614         +35.05%
BenchmarkRevcomp                   763449047      706768534      -7.42%
BenchmarkTemplate                  124954724      134834549      +7.91%
BenchmarkTimeParse                 517            511            -1.16%
BenchmarkTimeFormat                501            514            +2.59%

benchmark                         old MB/s     new MB/s     speedup
BenchmarkGobDecode                53.31        52.26        0.98x
BenchmarkGobEncode                61.12        59.28        0.97x
BenchmarkGzip                     42.79        41.08        0.96x
BenchmarkGunzip                   169.88       168.55       0.99x
BenchmarkJSONEncode               77.89        74.25        0.95x
BenchmarkJSONDecode               22.35        18.73        0.84x
BenchmarkGoParse                  13.41        11.98        0.89x
BenchmarkRegexpMatchEasy0_32      213.30       191.72       0.90x
BenchmarkRegexpMatchEasy0_1K      2603.92      2542.74      0.98x
BenchmarkRegexpMatchEasy1_32      254.00       224.93       0.89x
BenchmarkRegexpMatchEasy1_1K      1013.53      827.98       0.82x
BenchmarkRegexpMatchMedium_32     4.30         3.31         0.77x
BenchmarkRegexpMatchMedium_1K     13.30        9.97         0.75x
BenchmarkRegexpMatchHard_32       8.35         5.86         0.70x
BenchmarkRegexpMatchHard_1K       8.56         6.34         0.74x
BenchmarkRevcomp                  332.92       359.62       1.08x
BenchmarkTemplate                 15.53        14.39        0.93x

LGTM=rlh
R=rlh
CC=dvyukov, golang-codereviews, iant, khr, r
https://codereview.appspot.com/136380043

[gobot, 2014-09-11 16:28:04]

This CL appears to have broken the linux-amd64-race builder.
See http://build.golang.org/log/44a39e19787bf2e7e041129a637357f7c156faaa

[josharian, 2014-09-11 17:09:25]

> This CL appears to have broken the linux-amd64-race builder.
> See http://build.golang.org/log/44a39e19787bf2e7e041129a637357f7c156faaa

Real. This broke all the race builders.

[rsc, 2014-09-11 20:36:45]

On Thu, Sep 11, 2014 at 1:08 PM, Josh Bleecher Snyder <josharian@gmail.com>
wrote:

> > This CL appears to have broken the linux-amd64-race builder.
> > See http://build.golang.org/log/44a39e19787bf2e7e041129a637357f7c156faaa
>
> Real. This broke all the race builders.
>

I suspect that the implementation of writebarrierptr and friends needs to
make explicit calls into the race runtime routines to report the writes,
since the race detector does not instrument code in package runtime. But I
don't know exactly what those calls should be. Leaving for Dmitriy.

Russ

[DMorsing, 2014-09-12 09:43:58]

https://codereview.appspot.com/136380043/diff/60001/src/cmd/gc/walk.c
File src/cmd/gc/walk.c (right):

https://codereview.appspot.com/136380043/diff/60001/src/cmd/gc/walk.c#newcode...
src/cmd/gc/walk.c:2019: if(r->op == ONAME && strncmp(r->sym->name, "statictmp_",
10) == 0)
This breaks if you have user code with variables containing statictmp_

Example:

package main

var statictmp_foo *int
var foo *int

func main() {
	var z int
	statictmp_foo = &z
	foo = statictmp_foo
}

This only emits one write barrier when &z is assigned to statictmp_foo.

[rsc, 2014-09-12 11:28:05]

On Fri, Sep 12, 2014 at 5:43 AM, <daniel.morsing@gmail.com> wrote:

>
> https://codereview.appspot.com/136380043/diff/60001/src/cmd/gc/walk.c
> File src/cmd/gc/walk.c (right):
>
> https://codereview.appspot.com/136380043/diff/60001/src/
> cmd/gc/walk.c#newcode2019
> src/cmd/gc/walk.c:2019: if(r->op == ONAME && strncmp(r->sym->name,
> "statictmp_", 10) == 0)
> This breaks if you have user code with variables containing statictmp_
>

Yes, I know. Don't do that. I intend to clean up the magic names at some
point.

Russ