code review 5531073: runtime: madvise and SysUnused for Darwin

runtime: madvise and SysUnused for Darwin

SysUnused is a direct call to madvise MADV_FREE.

[dave_cheney.net, 2012-01-11 09:42:50]

Hello sebastien.paolacci@gmail.com, rsc@golang.org (cc:
golang-dev@googlegroups.com),

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

[dave_cheney.net, 2012-01-14 05:09:39]

ping

[Sebastien Paolacci, 2012-01-14 11:15:10]

Hi Dave,

Thanks for taking care of that. At glance, it looks good to me.

Could you please import CL 5451057 and check that observed usage (RSS)
matches what's reported in the 'consumed' trace section?

in test/bench/shootout,
$ 6g binary-tree.go && 6l -o binary-tree binary-tree.6
$ GOSCVGPERIOD=1 GOGCTRACE=1 ./binary-tree -n=18 2>&1 | grep scvg
scvg0: inuse: 26, idle: 24, sys: 51, released: 0, consumed: 51 (MB)
scvg1: inuse: 23, idle: 27, sys: 51, released: 0, consumed: 51 (MB)
scvg2: 10 MB released
scvg2: inuse: 20, idle: 30, sys: 51, released: 10, consumed: 40 (MB)
scvg3: inuse: 34, idle: 16, sys: 51, released: 10, consumed: 40 (MB)
scvg4: inuse: 21, idle: 29, sys: 51, released: 9, consumed: 42 (MB)
scvg5: inuse: 37, idle: 13, sys: 51, released: 7, consumed: 43 (MB)
scvg6: inuse: 28, idle: 22, sys: 51, released: 1, consumed: 49 (MB)
scvg7: inuse: 54, idle: 21, sys: 76, released: 0, consumed: 76 (MB)

Thanks,
Sebastien

[dave_cheney.net, 2012-01-14 11:57:50]

odessa(~/go/test/bench/shootout) % 6g binary-tree.go && 6l -o
binary-tree binary-tree.6odessa(~/go/test/bench/shootout) %
GOSCVGPERIOD=1 GOGCTRACE=1 ./binary-tree -n=18 2>&1 | grep scvgscvg0:
inuse: 25, idle: 25, sys: 51, released: 0, consumed: 51 (MB)scvg1:
inuse: 20, idle: 30, sys: 51, released: 0, consumed: 51 (MB)scvg2: 10
MB releasedscvg2: inuse: 35, idle: 15, sys: 51, released: 10,
consumed: 40 (MB)scvg3: inuse: 28, idle: 22, sys: 51, released: 10,
consumed: 40 (MB)scvg4: inuse: 35, idle: 15, sys: 51, released: 9,
consumed: 41 (MB)scvg5: inuse: 27, idle: 23, sys: 51, released: 6,
consumed: 44 (MB)scvg6: inuse: 28, idle: 22, sys: 51, released: 2,
consumed: 48 (MB)scvg7: inuse: 38, idle: 33, sys: 72, released: 0,
consumed: 72 (MB)


2012/1/14 Sébastien Paolacci <sebastien.paolacci@gmail.com>:
> Hi Dave,
>
> Thanks for taking care of that. At glance, it looks good to me.
>
> Could you please import CL 5451057 and check that observed usage (RSS)
> matches what's reported in the 'consumed' trace section?
>
> in test/bench/shootout,
> $ 6g binary-tree.go && 6l -o binary-tree binary-tree.6
> $ GOSCVGPERIOD=1 GOGCTRACE=1 ./binary-tree -n=18 2>&1 | grep scvg
> scvg0: inuse: 26, idle: 24, sys: 51, released: 0, consumed: 51 (MB)
> scvg1: inuse: 23, idle: 27, sys: 51, released: 0, consumed: 51 (MB)
> scvg2: 10 MB released
> scvg2: inuse: 20, idle: 30, sys: 51, released: 10, consumed: 40 (MB)
> scvg3: inuse: 34, idle: 16, sys: 51, released: 10, consumed: 40 (MB)
> scvg4: inuse: 21, idle: 29, sys: 51, released: 9, consumed: 42 (MB)
> scvg5: inuse: 37, idle: 13, sys: 51, released: 7, consumed: 43 (MB)
> scvg6: inuse: 28, idle: 22, sys: 51, released: 1, consumed: 49 (MB)
> scvg7: inuse: 54, idle: 21, sys: 76, released: 0, consumed: 76 (MB)
>
> Thanks,
> Sebastien

[Sebastien Paolacci, 2012-01-14 12:40:17]

Is the process actually using what's reported here (you should see a
50MB -> 40MB -> 50MB -> 75MB sequence in e.g `top')?

[minux1, 2012-01-14 16:16:50]

LGTM.

[Sebastien Paolacci, 2012-01-15 09:19:23]

Would be better, I think, if it was possible to evaluate the exact
behavior first. There's no urgency in that CL since not having it
doesn't break anything.

There's a kind of "weak" agreement in the posix family about those
hints semantic, I wouldn't be surprised if some variations were to
appear here.

Could someone give a try with the more bsd-ish MADV_FREE flag (instead
of MAD_DONTNEED)?

Thanks,
Sebastien

On Sat, Jan 14, 2012 at 5:16 PM,  <minux.ma@gmail.com> wrote:
> LGTM.
>
> http://codereview.appspot.com/5531073/

[dave_cheney.net, 2012-01-15 09:28:23]

Sure thing, I'll give it a go. 

Sent from my iPhone

On 15/01/2012, at 20:19, Sébastien Paolacci <sebastien.paolacci@gmail.com>
wrote:

> Would be better, I think, if it was possible to evaluate the exact
> behavior first. There's no urgency in that CL since not having it
> doesn't break anything.
> 
> There's a kind of "weak" agreement in the posix family about those
> hints semantic, I wouldn't be surprised if some variations were to
> appear here.
> 
> Could someone give a try with the more bsd-ish MADV_FREE flag (instead
> of MAD_DONTNEED)?
> 
> Thanks,
> Sebastien
> 
> On Sat, Jan 14, 2012 at 5:16 PM,  <minux.ma@gmail.com> wrote:
>> LGTM.
>> 
>> http://codereview.appspot.com/5531073/

[dave_cheney.net, 2012-01-15 09:48:36]

Hello sebastien.paolacci@gmail.com, rsc@golang.org, minux.ma@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[dave_cheney.net, 2012-01-15 09:50:27]

No observed difference sadly. /usr/bin/dtruss cannot properly instrument a Go
program so I'm not sure if I can confirm if madvise is even being called.

[rsc, 2012-01-17 00:03:30]

http://codereview.appspot.com/5531073/diff/11002/src/pkg/runtime/mem_darwin.c
File src/pkg/runtime/mem_darwin.c (right):

http://codereview.appspot.com/5531073/diff/11002/src/pkg/runtime/mem_darwin.c...
src/pkg/runtime/mem_darwin.c:26: // MADV_DONTNEED does not appear to be
It sure looks like all the implementations should be using MADV_FREE.

[rsc, 2012-01-17 00:04:09]

LGTM

But we need to fix the comments.
MADV_FREE is what we want, always,
as far as I can tell.  Nothing should be
mentioning MADV_DONTNEED.

[dave_cheney.net, 2012-01-17 00:10:07]

Thanks Russ. I will fix and resubmit tonight. 

Sent from my iPhone

On 17/01/2012, at 11:04, rsc@golang.org wrote:

> LGTM
> 
> But we need to fix the comments.
> MADV_FREE is what we want, always,
> as far as I can tell.  Nothing should be
> mentioning MADV_DONTNEED.
> 
> 
> http://codereview.appspot.com/5531073/

[rsc, 2012-01-17 01:46:11]

If you want to include a comment, I think it would be

// Linux's MADV_DONTNEED is like BSD's MADV_FREE.

[dave_cheney.net, 2012-01-17 08:19:58]

Hello sebastien.paolacci@gmail.com, rsc@golang.org, minux.ma@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[rsc, 2012-01-19 20:50:52]

LGTM

[rsc, 2012-01-19 20:51:32]

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

runtime: madvise and SysUnused for Darwin

SysUnused is a direct call to madvise MADV_FREE.

R=sebastien.paolacci, rsc, minux.ma
CC=golang-dev
http://codereview.appspot.com/5531073

Committer: Russ Cox <rsc@golang.org>