You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
cmd/compile: slow to compile 17,000 line switch statement? #62604
It's just that the author is being punished for writing that.
Reacted by Cuong Manh Le, udhos, Johan Brandhorst-Satzkorn, donuts are good, Eric Lagergren, Mauri de Souza Meneguzzo, Lomanic, Moritz Poldrack, محمّد, Justin Nuß and 11 more
glyphsnames: Use switch cases for lookups instead of huge maps
This seems to increase binaries about 0.3MB but decrease memory usage by around 1MB.
So maybe there's room for an improvement in our map literals. (#2559 (comment), etc)
But maybe the improvements in Go 1.21 (https://go.dev/doc/go1.21#linker) make that "decrease memory usage by around 1MB" comment outdated at this point.
@bradfitz As an aside, please note that recent versions of Gioui (starting at 0.0.1 for instance) do not depend on github.com/benoitkugler/textlayout anymore, and that the (very ugly) code for glyph names has been removed.
This is a bug in the compiler that causes unreasonably slow compilation on some large functions.
I'm leaning towards backporting the CL for this issue, as it might be pretty painful, even if it isn't a correctness bug.
The CL itself is super safe.
Happy to hear other opinions though.
Activity
seankhliao commentedon Sep 12, 2023
I think previously #57959
randall77 commentedon Sep 12, 2023
I don't think this is the same as #57959. At least, pprof points to a different cause. That was the prove pass, this one is the memcombine pass.
gopherbot commentedon Sep 12, 2023
Change https://go.dev/cl/527699 mentions this issue:
cmd/compile: reset memcombine correctly between basic blocksrobpike commentedon Sep 13, 2023
It's just that the author is being punished for writing that.
heschi commentedon Sep 13, 2023
cc @golang/compiler
bradfitz commentedon Sep 13, 2023
@robpike, I figured there must be some history there, and sure enough: benoitkugler/textlayout@24bac7a
So maybe there's room for an improvement in our map literals. (#2559 (comment), etc)
But maybe the improvements in Go 1.21 (https://go.dev/doc/go1.21#linker) make that "decrease memory usage by around 1MB" comment outdated at this point.
griesemer commentedon Sep 13, 2023
FWIW: A cursory glance at the respective type checker code didn't reveal any obvious problems.
benoitkugler commentedon Sep 13, 2023
@bradfitz As an aside, please note that recent versions of Gioui (starting at 0.0.1 for instance) do not depend on
github.com/benoitkugler/textlayoutanymore, and that the (very ugly) code for glyph names has been removed.randall77 commentedon Sep 15, 2023
@gopherbot Please open a backport issue for 1.21.
This is a bug in the compiler that causes unreasonably slow compilation on some large functions.
I'm leaning towards backporting the CL for this issue, as it might be pretty painful, even if it isn't a correctness bug.
The CL itself is super safe.
Happy to hear other opinions though.
gopherbot commentedon Sep 15, 2023
Backport issue(s) opened: #62668 (for 1.21).
Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.