Code review - Issue 6655045: code review 6655045: cmd/5l: reorder some struct fields to reduce memory con...https://codereview.appspot.com/2012-10-12T07:51:20+00:00rietveld
Message from unknown
2012-10-11T16:12:54+00:00minux1urn:md5:17bc2e1ae308f8201b97d648ed646140
Message from unknown
2012-10-11T16:13:05+00:00minux1urn:md5:c7decbf2612fc92fba2b6c4612a1cde8
Message from unknown
2012-10-11T16:21:59+00:00minux1urn:md5:4c9a9c7f49b8a11b32ef99401e1b1882
Message from minux.ma@gmail.com
2012-10-11T16:22:09+00:00minux1urn:md5:a930ebe70750460b39d863f775b0c804
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
I'd like you to review this change to
https://code.google.com/p/go/
Message from minux.ma@gmail.com
2012-10-11T18:43:31+00:00minux1urn:md5:26cbca6a2a2eeece2aee94755f9e9cd5
Note: the same field reorder isn't applicable to cmd/8l or cmd/6l.
Message from r@golang.org
2012-10-11T20:52:12+00:00rurn:md5:f1b16455069c99a7e5b0c14aea4d5fa6
LGTM
Message from dave@cheney.net
2012-10-12T02:41:02+00:00dfcurn:md5:56d76690d37d3710ee456baa4c83c02c
On 2012/10/11 20:52:12, r wrote:
> LGTM
LGTM. Are there similar savings inside 5g ?
Message from unknown
2012-10-12T05:36:35+00:00minux1urn:md5:f022286e50aae2980965a9c044e666a8
Message from unknown
2012-10-12T05:37:57+00:00minux1urn:md5:2326b1a7be1ea8da02701b55bcbf8344
Message from minux.ma@gmail.com
2012-10-12T05:39:33+00:00minux1urn:md5:a0682c1db58c2b1c5225f2a837eba3f5
*** Submitted as http://code.google.com/p/go/source/detail?r=1baa80f7f4a4 ***
cmd/5l: reorder some struct fields to reduce memory consumption
Valgrind Massif result when linking godoc:
On amd64:
old new -/+
mem_heap_B 185844612 175358047 -5.7%
mem_heap_extra_B 773404 773137 -0.0%
On 386/ARM:
old new -/+
mem_heap_B 141775701 131289941 -7.4%
mem_heap_extra_B 737011 736955 -0.0%
R=golang-dev, r, dave
CC=golang-dev
http://codereview.appspot.com/6655045
Message from minux.ma@gmail.com
2012-10-12T06:01:16+00:00minux1urn:md5:40e45d51e385c5dc6dabca42f81dc3ec
On Fri, Oct 12, 2012 at 10:41 AM, <dave@cheney.net> wrote:
> Are there similar savings inside 5g ?
>
As I didn't suspect gc would use much memory, I haven't looked at gc.
But I suspect there are similar saving in gc.
Message from minux.ma@gmail.com
2012-10-12T06:32:19+00:00minux1urn:md5:2766b1d25ec8659abcb2a4e0dd7f66be
On Fri, Oct 12, 2012 at 2:00 PM, minux <minux.ma@gmail.com> wrote:
>
> On Fri, Oct 12, 2012 at 10:41 AM, <dave@cheney.net> wrote:
>
>> Are there similar savings inside 5g ?
>>
> As I didn't suspect gc would use much memory, I haven't looked at gc.
>
> But I suspect there are similar saving in gc.
>
The same reordering is applicable to gc, but i think gc orders fields for
better documentations (group related fields together), and the space
saving won't be big if i can't reorder the fields freely.
Russ said that gc put every number (even 0 is) as bigint/bigfloat, and
removing
that could bring us big space savings. Also, he said that we could reuse
used
number (or cache small frequently used numbers, say -1, 0, 1-100, 128, 256,
etc
in an array).
Message from remyoudompheng@gmail.com
2012-10-12T06:41:05+00:00remyoudomphengurn:md5:887fbc4486e80e5ba4b9a081b34121b4
On 2012/10/12 minux <minux.ma@gmail.com> wrote:
> The same reordering is applicable to gc, but i think gc orders fields for
> better documentations (group related fields together), and the space
> saving won't be big if i can't reorder the fields freely.
>
> Russ said that gc put every number (even 0 is) as bigint/bigfloat, and
> removing
> that could bring us big space savings. Also, he said that we could reuse
> used
> number (or cache small frequently used numbers, say -1, 0, 1-100, 128, 256,
> etc
> in an array).
As far as I remember, the largest memory usage when compiling the
dreadful output of test/rotate.go was essentially in Nodes. It's not
clear what we can do for that.
Rémy.
Message from minux.ma@gmail.com
2012-10-12T07:07:23+00:00minux1urn:md5:19f1c649edf967fda50f50af6b3769b1
On Fri, Oct 12, 2012 at 2:41 PM, Rémy Oudompheng
<remyoudompheng@gmail.com>wrote:
> As far as I remember, the largest memory usage when compiling the
> dreadful output of test/rotate.go was essentially in Nodes. It's not
> clear what we can do for that.
>
Yeah, true.
By freely reordering struct Node, I can make the 6g's maximum RSS
smaller by 1.43% (from 5357168 KiB to 5281024 KiB, -76144 KiB).
After reordering struct Sym, RSS go down by 3776 KiB (to 5277248 KiB).
After reordering struct Type, RSS go down by 9168 KiB (to 5268080 KiB).
To see the maximum RSS saved this way, I added #pragma pack to gc/go.h,
and the RSS is now 5198800 KiB (down by 158368 KiB, a merely 3%).
No big improvements (and doing this will severely affect code documentation
quality), so maybe we need to use different type of Node struct for
different
kinds of node (which is a pretty big task).
Message from dave@cheney.net
2012-10-12T07:14:04+00:00dfcurn:md5:e188c5b9104c05570ae2880c6c061f5f
5.3Gb to compile! That is getting into Firefox territory.
http://www.phoronix.com/scan.php?page=news_item&px=MTA3MTQ
On Fri, Oct 12, 2012 at 6:07 PM, minux <minux.ma@gmail.com> wrote:
>
> On Fri, Oct 12, 2012 at 2:41 PM, Rémy Oudompheng <remyoudompheng@gmail.com>
> wrote:
>>
>> As far as I remember, the largest memory usage when compiling the
>> dreadful output of test/rotate.go was essentially in Nodes. It's not
>> clear what we can do for that.
>
> Yeah, true.
>
> By freely reordering struct Node, I can make the 6g's maximum RSS
> smaller by 1.43% (from 5357168 KiB to 5281024 KiB, -76144 KiB).
>
> After reordering struct Sym, RSS go down by 3776 KiB (to 5277248 KiB).
>
> After reordering struct Type, RSS go down by 9168 KiB (to 5268080 KiB).
>
> To see the maximum RSS saved this way, I added #pragma pack to gc/go.h,
> and the RSS is now 5198800 KiB (down by 158368 KiB, a merely 3%).
>
> No big improvements (and doing this will severely affect code documentation
> quality), so maybe we need to use different type of Node struct for
> different
> kinds of node (which is a pretty big task).
>
Message from minux.ma@gmail.com
2012-10-12T07:51:20+00:00minux1urn:md5:b0b0aff20d34ac4db039a03bdffb41b7
On Fri, Oct 12, 2012 at 3:14 PM, Dave Cheney <dave@cheney.net> wrote:
> 5.3Gb to compile! That is getting into Firefox territory.
> http://www.phoronix.com/scan.php?page=news_item&px=MTA3MTQ
That's the very reason that test is not enabled by default, or
all the builders will blow up. :-)