code review 7307122: runtime: allocate heap metadata at run time

runtime: allocate heap metadata at run time

Before, the mheap structure was in the bss,
but it's quite large (today, 256 MB, much of
which is never actually paged in), and it makes
Go binaries run afoul of exec-time bss size
limits on some BSD systems.

Fixes issue 4447.

[rsc, 2013-02-15 04:46:38]

Hello golang-dev@googlegroups.com,

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

[dave_cheney.net, 2013-02-15 06:35:57]

Thanks rsc, I'll run some tests on 32 bit systems tonight.

[minux1, 2013-02-15 06:37:37]

https://codereview.appspot.com/7307122/diff/5001/src/pkg/runtime/race.c
File src/pkg/runtime/race.c (right):

https://codereview.appspot.com/7307122/diff/5001/src/pkg/runtime/race.c#newco...
src/pkg/runtime/race.c:43: sz = (byte*)runtime·mheap - noptrdata;
this seems to make some assumption based on
mheap layout (in .noptrbss).

[remyoudompheng, 2013-02-15 06:57:50]

On 15 Feb 2013 05:46, <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://code.google.com/p/go/
>
>
> Description:
> runtime: allocate heap metadata at run time
>
> Before, the mheap structure was in the bss,
> but it's quite large (today, 256 MB, much of
> which is never actually paged in), and it makes
> Go binaries run afoul of exec-time bss size
> limits on some BSD systems.

Fixes issue 4447?

[dave_cheney.net, 2013-02-15 11:44:05]

Here is a data point from a 32bit atom system

220887(~/go/test/bench/go1) % go version
go version devel +41ab0ca75ea3 Fri Feb 15 16:55:51 2013 +1100 linux/386
220887(~/go/test/bench/go1) % ~/go/misc/benchcmp {old,new}.txt
benchmark                 old ns/op    new ns/op    delta
BenchmarkBinaryTree17    2147483647   2147483647  -17.72%
BenchmarkFannkuch11      2147483647   2147483647   +2.43%
BenchmarkGobDecode        117198594     90299821  -22.95%
BenchmarkGobEncode         44873231     37907496  -15.52%
BenchmarkGzip            2147483647   2147483647   -1.18%
BenchmarkGunzip           582749174    584835948   +0.36%
BenchmarkJSONEncode       380500998    379802906   -0.18%
BenchmarkJSONDecode       902289013    894152629   -0.90%
BenchmarkMandelbrot200     33310667     33329347   +0.06%
BenchmarkParse             37263346     37267311   +0.01%
BenchmarkRevcomp         2147483647   2147483647   -0.26%
BenchmarkTemplate        1518277080   1550416780   +2.12%

benchmark                  old MB/s     new MB/s  speedup
BenchmarkGobDecode             6.55         8.50    1.30x
BenchmarkGobEncode            17.10        20.25    1.18x
BenchmarkGzip                  5.72         5.79    1.01x
BenchmarkGunzip               33.30        33.18    1.00x
BenchmarkJSONEncode            5.10         5.11    1.00x
BenchmarkJSONDecode            2.15         2.17    1.01x
BenchmarkParse                 1.55         1.55    1.00x
BenchmarkRevcomp              54.07        54.21    1.00x
BenchmarkTemplate              1.28         1.25    0.98x

These results are consistent across several runs

[dave_cheney.net, 2013-02-15 12:23:40]

Results are less consistent for linux/arm, nexus7.

nexus7(~/go/test/bench/go1) % go version
go version devel +41ab0ca75ea3 Fri Feb 15 16:55:51 2013 +1100 linux/arm
nexus7(~/go/test/bench/go1) % ~/go/misc/benchcmp {old,new}.txt
benchmark                 old ns/op    new ns/op    delta
BenchmarkBinaryTree17    2147483647   2147483647   +0.71%
BenchmarkFannkuch11      2147483647   2147483647   +5.19%
BenchmarkGobDecode        124440750    122867300   -1.26%
BenchmarkGobEncode         45218240     43464740   -3.88%
BenchmarkGzip            2147483647   2147483647   -0.50%
BenchmarkGunzip           710857000    704859600   -0.84%
BenchmarkJSONEncode       501282600    504572600   +0.66%
BenchmarkJSONDecode      1127346000   1148390000   +1.87%
BenchmarkMandelbrot200     38328919     38360100   +0.08%
BenchmarkParse             49816260     51052780   +2.48%
BenchmarkRevcomp           93411650     92312500   -1.18%
BenchmarkTemplate        1397807000   1364438000   -2.39%

benchmark                  old MB/s     new MB/s  speedup
BenchmarkGobDecode             6.17         6.25    1.01x
BenchmarkGobEncode            16.97        17.66    1.04x
BenchmarkGzip                  4.43         4.45    1.00x
BenchmarkGunzip               27.30        27.53    1.01x
BenchmarkJSONEncode            3.87         3.85    0.99x
BenchmarkJSONDecode            1.72         1.69    0.98x
BenchmarkParse                 1.16         1.13    0.97x
BenchmarkRevcomp              27.21        27.53    1.01x
BenchmarkTemplate              1.39         1.42    1.02x

[iant, 2013-02-15 15:12:26]

https://codereview.appspot.com/7307122/diff/5001/src/pkg/runtime/race.c
File src/pkg/runtime/race.c (right):

https://codereview.appspot.com/7307122/diff/5001/src/pkg/runtime/race.c#newco...
src/pkg/runtime/race.c:43: sz = (byte*)runtime·mheap - noptrdata;
On 2013/02/15 06:37:37, minux wrote:
> this seems to make some assumption based on
> mheap layout (in .noptrbss).

Agreed.  This code needs to change.  I think it should simply be

runtime∕race·MapShadow(noptrdata, enoptrbss - noptrdata);

[rsc, 2013-02-15 18:13:02]

Hello golang-dev@googlegroups.com, dave@cheney.net, minux.ma@gmail.com,
remyoudompheng@gmail.com, iant@golang.org (cc: golang-dev@googlegroups.com),

Please take another look.

[iant, 2013-02-15 18:58:26]

LGTM

[rsc, 2013-02-15 19:27:09]

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

runtime: allocate heap metadata at run time

Before, the mheap structure was in the bss,
but it's quite large (today, 256 MB, much of
which is never actually paged in), and it makes
Go binaries run afoul of exec-time bss size
limits on some BSD systems.

Fixes issue 4447.

R=golang-dev, dave, minux.ma, remyoudompheng, iant
CC=golang-dev
https://codereview.appspot.com/7307122