Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: re-enable rotate tests #9933

Closed
rsc opened this issue Feb 19, 2015 · 7 comments
Closed

test: re-enable rotate tests #9933

rsc opened this issue Feb 19, 2015 · 7 comments
Milestone

Comments

@rsc
Copy link
Contributor

rsc commented Feb 19, 2015

disabled because they use too much memory to compile in the current draft of the new Go compiler.
reenable once the memory issues are fixed.

@rsc rsc added this to the Go1.5 milestone Feb 19, 2015
rsc added a commit that referenced this issue Feb 19, 2015
They use too much memory in the current Go compiler draft.
This should fix some builders.

Reenabling is #9933.

Change-Id: Ib5ef348b2c55d2012ffed765f2a6df99dec171f4
Reviewed-on: https://go-review.googlesource.com/5302
Reviewed-by: Russ Cox <[email protected]>
@josharian
Copy link
Contributor

Unsurprisingly, memory profiling shows that gc.Node and gc.Prog dominate memory usage here. I have several unmailed work-in-progress CLs to shrink gc.Node.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/10059 mentions this issue.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/10120 mentions this issue.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/10121 mentions this issue.

josharian added a commit that referenced this issue May 15, 2015
This CL was generated by updating Val in go.go
and then running:

sed -i "" 's/\.U\.[SBXFC]val = /.U = /' *.go
sed -i "" 's/\.U\.Sval/.U.\(string\)/g' *.go *.y
sed -i "" 's/\.U\.Bval/.U.\(bool\)/g' *.go *.y
sed -i "" 's/\.U\.Xval/.U.\(\*Mpint\)/g' *.go *.y
sed -i "" 's/\.U\.Fval/.U.\(\*Mpflt\)/g' *.go *.y
sed -i "" 's/\.U\.Cval/.U.\(\*Mpcplx\)/g' *.go *.y

No functional changes. Passes toolstash -cmp.

This reduces the size of gc.Node from 424 to 392 bytes.
This in turn reduces the permanent (pprof -inuse_space)
memory usage while compiling the test/rotate?.go tests:

test	old(MB)	new(MB)	change
rotate0	379.49	364.78	-3.87%
rotate1	373.42	359.07	-3.84%
rotate2	381.17	366.24	-3.91%
rotate3	374.30	359.95	-3.83%

CL 8445 was similar to this; gri asked that Val's implementation
be hidden first. CLs 8912, 9263, and 9267 have at least
isolated the changes to the cmd/internal/gc package.

Updates #9933.

Change-Id: I83ddfe003d48e0a73c92e819edd3b5e620023084
Reviewed-on: https://go-review.googlesource.com/10059
Reviewed-by: Russ Cox <[email protected]>
josharian added a commit that referenced this issue May 15, 2015
Name will be converted from an anonymous to a
named field in a subsequent, automated CL.

No functional changes. Passes toolstash -cmp.

This reduces the size of gc.Node from 424 to 400 bytes.
This in turn reduces the permanent (pprof -inuse_space)
memory usage while compiling the test/rotate?.go tests:

test	old(MB)	new(MB)	change
rotate0	379.49	367.30	-3.21%
rotate1	373.42	361.59	-3.16%
rotate2	381.17	368.77	-3.25%
rotate3	374.30	362.48	-3.15%

Updates #9933.

Change-Id: I21479527c136add4f1efb9342774e3be3e276e83
Reviewed-on: https://go-review.googlesource.com/10120
Reviewed-by: Russ Cox <[email protected]>
josharian added a commit to josharian/go that referenced this issue May 15, 2015
Rearrange Node fields to enable better struct packing.
This reduces readability in favor of shrinking
the size of Nodes.

This reduces the size of Node from 368 to 344.
This reduces the memory usage to compile the
rotate tests by about 5.6%.

No functional changes. Passes toolstash -cmp.

Updates golang#9933.

Change-Id: I2764c5847fb1635ddc898e2ee385d007d67f03c5
@gopherbot
Copy link
Contributor

CL https://golang.org/cl/10141 mentions this issue.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/10210 mentions this issue.

@gopherbot
Copy link
Contributor

CL https://golang.org/cl/10211 mentions this issue.

josharian added a commit that referenced this issue May 18, 2015
Param will be converted from an anonymous to a
named field in a subsequent, automated CL.

Reduces Node size from 368 to 328.
Reduces inuse_space on the rotate tests by about 3%.

No functional changes. Passes toolstash -cmp.

Updates #9933.

Change-Id: I5867b00328abf17ee24aea6ca58876bae9d8bfed
Reviewed-on: https://go-review.googlesource.com/10210
Reviewed-by: Russ Cox <[email protected]>
josharian added a commit that referenced this issue May 18, 2015
Rearrange Node fields to enable better struct packing.
This reduces readability in favor of shrinking
the size of Nodes.

This reduces the size of Node from 328 to 312.
This reduces the memory usage to compile the
rotate tests by about 4.4%.

No functional changes. Passes toolstash -cmp.

Updates #9933.

Change-Id: I2764c5847fb1635ddc898e2ee385d007d67f03c5
Reviewed-on: https://go-review.googlesource.com/10141
Reviewed-by: Russ Cox <[email protected]>
josharian added a commit to josharian/go that referenced this issue May 18, 2015
Memory usage has been reduced.
The tests are still slow,
but that is issue golang#10571.

/usr/bin/time shows significant variation
in the peak memory usage compiling with tip.
This is unsurprising, given GC.

Using Go 1.4.2, memory is stable at 410mb.
Using tip at a21cf5b,
which does not include some relevant CLs,
memory ranges from 524mb (+28%) to 593mb (+45%),
with a mean of 563mb (+37%), with n=43.

DO NOT SUBMIT -- trybot bait for the moment

Fixes golang#9933.

Change-Id: Id31f3ae086ec324abf70e8f1a8044c4a0c27e274
josharian added a commit to josharian/go that referenced this issue May 26, 2015
Memory usage has been reduced.
The tests are still slow,
but that is issue golang#10571.

/usr/bin/time shows significant variation
in the peak memory usage compiling with tip.
This is unsurprising, given GC.

Using Go 1.4.2, memory is stable at 410mb.
Using tip at 791bb4f,
memory ranges from 506mb (+23%) to 571mb (+39%),
with a mean of 536mb (+31%), with n=50.

Fixes golang#9933.

Change-Id: Id31f3ae086ec324abf70e8f1a8044c4a0c27e274
@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants