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

cmd/compile: generate static data into fewer Progs #14786

Closed
josharian opened this issue Mar 12, 2016 · 10 comments
Closed

cmd/compile: generate static data into fewer Progs #14786

josharian opened this issue Mar 12, 2016 · 10 comments

Comments

@josharian
Copy link
Contributor

When compiling the unicode package, every single int in the static data (e.g. the RangeTables) gets its own Prog. (It also gets its own Node and Mpint, but that's another matter; see also #4617.) This blows through the Prog cache that @randall77 put into place and a lot more. Rather than generating a sequence of linked ADATA Progs and then writing them out, we should create fewer ADATA Progs containing more data, or maybe write it out even more directly like obj.Setuintxx.

I'm not sure how or where the right place to do this is. Advice welcomed.

@josharian josharian added this to the Unplanned milestone Mar 12, 2016
@josharian
Copy link
Contributor Author

For that matter, does data need to go into Progs at all?

@bradfitz
Copy link
Contributor

@mdempsky
Copy link
Member

I suspect there's no actual requirement anymore.

In particular, I notice there's already a TYPE_SCONST type that allows passing string values, but it's arbitrarily capped to 100 bytes in obj.savedata. I suspect we could remove that restriction (and maybe allow To.Val to contain either []byte or string), and the compiler could cut down on a lot of Progs.

@mdempsky
Copy link
Member

https://golang.org/cl/20607 generalizes cmd/internal/obj to support arbitrary length string constants and to update cmd/compile and cmd/asm to make use of them. Seems to work.

@mdempsky
Copy link
Member

I'm trying to benchmark CL 20607, but compilebench keeps giving me wildly noisy timings on my workstation. Will try to test on some other machines.

@josharian BTW, what were you doing to measure how many Progs were generated and/or to determine that it was problematic?

@gopherbot
Copy link

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

@josharian
Copy link
Contributor Author

I doubt that CL 20607 will address this directly; rather it lays one useful piece of groundwork. There'll also be work in sinit.go and elsewhere to use these newfound powers.

The Progs in unicode mostly get generated in sinit.go, func staticassign, case OSTRUCTLIT, call to gdata. gdata then jumps through a bunch of hoops (gc Gins, x86 gins, gc Prog, gc Naddr, etc.) and spits out a single Prog with a single int attached. Similar things happen throughout sinit.go.

To measure how many Progs were being generated, I just stuck some fmt.Printlns in "*Link".NewProg (cmd/internal/obj/util.go) for the new(Prog) branch that printed out ctxt.Cursym.Name and ran the output through pct.

@josharian
Copy link
Contributor Author

Started.

@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Mar 13, 2016
This makes the output of compiling with -S more
stable in the face of unimportant variation in the
order in which relocs are generated.
It is also more pleasant to read the relocs when
they are sorted.

Also, do some minor cleanup.

For #14786

Change-Id: Id92020b13fd21777dfb5b29c2722c3b2eb27001b
Reviewed-on: https://go-review.googlesource.com/20641
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
@gopherbot
Copy link

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

gopherbot pushed a commit that referenced this issue Mar 14, 2016
Updates #14786.

Change-Id: I5fe889886f772167386cd10390ac50abc1383937
Reviewed-on: https://go-review.googlesource.com/20607
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Josh Bleecher Snyder <josharian@gmail.com>
@golang golang locked and limited conversation to collaborators Mar 14, 2017
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

4 participants