golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.07k stars 17.68k forks source link

cmd/compile: merge temporaries more aggressively #8740

Open josharian opened 10 years ago

josharian commented 10 years ago
CL 12829043 added a compiler pass that merges temporary variables with equal types and
non-overlapping lifetimes.

I believe that "equal types" is sufficient but not necessary and that
"equivalent w.r.t. GC" is both necessary and sufficient. (Please correct me if
I am wrong!) This should allow more temporaries to be merged, further reducing stack
usage.
josharian commented 9 years ago

I took a quick hack at this. It does appear to offer some nice stack use wins, but it breaks our dwarf generation. Also, twobitwalktype1 is a compiler performance bottleneck. This will need to wait at the least until the compiler transition to Go is completed.

My ugly hack code: https://github.com/josharian/go/compare/issue-8740. From the commit message there:

In godoc binary

Top 20 changed frame sizes, by absolute change

2152    1304    -848    -39.4%  golang.org/x/tools/godoc/analysis       "".(*analysis).doCallgraph 
2496    1976    -520    -20.8%  golang.org/x/tools/godoc/analysis       "".Run 
952     448     -504    -52.9%  html/template   "".(*escaper).escapeListConditionally 
1408    920     -488    -34.7%  golang.org/x/tools/go/ssa       "".(*builder).stmt 
1680    1192    -488    -29.0%  golang.org/x/tools/go/loader    "".(*Config).Load 
2160    1688    -472    -21.9%  golang.org/x/tools/go/types     "".(*Checker).collectObjects 
1904    1472    -432    -22.7%  golang.org/x/tools/go/ssa       "".(*builder).expr0 
728     320     -408    -56.0%  html/template   "".(*escaper).commit 
552     168     -384    -69.6%  encoding/gob    "".registerBasics 
1608    1232    -376    -23.4%  crypto/x509     "".buildExtensions 
1208    840     -368    -30.5%  go/doc  "".playExample 
1792    1424    -368    -20.5%  crypto/x509     "".parseCertificate 
1112    760     -352    -31.7%  golang.org/x/tools/godoc        "".(*handlerServer).GetPageInfo 
1384    1032    -352    -25.4%  golang.org/x/tools/go/ssa       "".(*Program).CreateTestMainPackage 
1792    1440    -352    -19.6%  golang.org/x/tools/go/types     "".(*Checker).builtin 
1968    1616    -352    -17.9%  golang.org/x/tools/go/types     "".(*Checker).stmt 
1160    824     -336    -29.0%  golang.org/x/tools/go/ssa       "".(*Package).Build 
1680    1368    -312    -18.6%  golang.org/x/tools/go/types     "".(*Checker).exprInternal 
2384    2088    -296    -12.4%  go/build        "".(*Context).Import 
1848    1568    -280    -15.2%  golang.org/x/tools/godoc/analysis       "".(*analysis).doChannelPeers 

Top 20 changed frame sizes, by percent change

552     168     -384    -69.6%  encoding/gob    "".registerBasics 
168     72      -96     -57.1%  archive/zip     type..eq."".checksumReader dupok 
728     320     -408    -56.0%  html/template   "".(*escaper).commit 
952     448     -504    -52.9%  html/template   "".(*escaper).escapeListConditionally 
144     72      -72     -50.0%  text/template   "".sortKeys 
200     104     -96     -48.0%  net/http        type..eq."".conn dupok 
136     72      -64     -47.1%  archive/zip     type..eq."".fileWriter dupok 
136     72      -64     -47.1%  crypto/cipher   type..eq."".StreamWriter dupok 
136     72      -64     -47.1%  golang.org/x/tools/go/ssa       type..eq."".DebugRef dupok 
136     72      -64     -47.1%  golang.org/x/tools/go/types     type..eq."".operand dupok 
336     184     -152    -45.2%  encoding/gob    "".init 
120     72      -48     -40.0%  compress/flate  "".(*compressor).initDeflate 
2152    1304    -848    -39.4%  golang.org/x/tools/godoc/analysis       "".(*analysis).doCallgraph 
128     80      -48     -37.5%  go/doc  "".(*Package).Filter 
64      40      -24     -37.5%  compress/flate  "".(*huffmanBitWriter).writeCode 
512     328     -184    -35.9%  golang.org/x/tools/go/ast/astutil       "".AddNamedImport 
1408    920     -488    -34.7%  golang.org/x/tools/go/ssa       "".(*builder).stmt 
232     152     -80     -34.5%  net/http        "".init 
560     368     -192    -34.3%  text/template   "".(*Template).Clone 
120     80      -40     -33.3%  go/parser       "".(*parser).tryIdentOrType 
dvyukov commented 9 years ago

Nice! This can significantly reduce memory consumption and frequency of GCs for servers with lots of goroutines.

rsc commented 9 years ago

The compare link doesn't seem to work anymore. Can you refresh it somehow?

josharian commented 9 years ago

CL 10251

gopherbot commented 9 years ago

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

bradfitz commented 8 years ago

/cc @randall77

josharian commented 8 years ago

I have a prototype of this for SSA, although it needs some cleanup. It still helps some, although not as dramatically as before SSA. I plan to finish it up and mail it early in the Go 1.8 cycle.

quentinmit commented 8 years ago

@josharian Have you had a chance to look at this yet for 1.8?

josharian commented 8 years ago

I did. My recollection (now fuzzy) is that it helped but not enough to inspire me to finish re-implementing it.

I'll also admit to being a bit confused at the moment about the relationship between the stackalloc pass and the temporary merging code found in pgen.go. I've implemented this independently in both places, but never tried applying it to both at the same time.