Skip the deinline size search in module phase (it is a guaranteed no-op)#506
Open
darkverbito wants to merge 1 commit into
Open
Skip the deinline size search in module phase (it is a guaranteed no-op)#506darkverbito wants to merge 1 commit into
darkverbito wants to merge 1 commit into
Conversation
In module phase, deinline_opt's inline/deinline size search can never keep a flip: flip_helper's guard only permits the non-inline -> inline direction for NoInlinePreference synthetics, and the module convention keeps those synthetics non-inline because that is the smaller representation, so the inline direction is always larger and always rejected. Running the search anyway still performs O(functions) full code generations per root set, each of which is itself superlinear in program size. On large multi-export modules (many synthetic let-binding helpers) this dominates compile time and makes compilation appear to hang. Return the unchanged program in module phase. This is output-identical (the search never updates best_compileform there) and removes the blow-up. Adds a regression test that compiles a multi-export module built from a chain of helpers with deep let stacks and runs both exports.
Contributor
|
thanks for the analysis. our workflows require signed prs, so if you force push with signatures, i can fully review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Compiling a multi-export module (the
(export ...)form) whose exports share achain of helpers that each contain a deep nested
letstack causescompile_modernto spend an unbounded-looking amount of time at 100% CPU andeffectively never finish. The cost is superlinear in the number of synthetic
let-binding helpers, so adding one helper or one
letlevel multiplies compiletime.
The work is performed by the deinline inline/deinline size search in
src/compiler/optimize/deinline.rs(deinline_opt). In module phase that searchis a guaranteed no-op — it can never keep a flip — yet it still runs a full
codegenof the whole program for every synthetic function in every root set, onevery improvement pass. On large multi-export modules this dominates compile time
to the point that the compiler appears to hang.
This PR skips the search in module phase (where it is provably a no-op) and adds a
regression test.
Affected versions
Observed on
0.4.5and reproduces unchanged on currentmain(
deinline_opthas not been touched since the0.4.5tag).The previous
0.4.1compiles the same input in well under a second. Theregression is the
0.4.1 -> 0.4.5change todeinline_opt, which (a) removed theearly
returnthat skipped the search when there was nothing to deinline, and(b) began building the dependency graph with
with_constants: truein modulephase. The net effect is that the size search now always runs in module phase over
a larger function set.
This is dialect-independent: it reproduces under both
*standard-cl-23*and*standard-cl-25*(the multi-exportcompile_modulepath is shared).Minimal reproduction
A fully synthetic, addition-only module (so the export results stay small and
exactly assertable). A chain of
Nhelpers, each a depth-Dletstack, with acouple of exports:
Compile time grows superlinearly with
NandD— the signature of the searchblow-up. Measured on current
main(run -i <dir> repro.clsp, release build):mainReal multi-export handler modules push this into the tens-of-minutes range.
Root cause
In
deinline_opt:letbinding has become a synthetic helper withSyntheticType::NoInlinePreference.that, for every function in the set, flips its inline status, runs a full
codegenof the whole program, and keeps the flip only if it shrank theprogram — repeating until no improvement.
flip_helper's guard is... && (!is_module_compile || !*inline). In modulephase this only ever flips a
NoInlinePreferencesynthetic fromnon-inline to inline. The module convention keeps these synthetics
non-inline because that is the smaller representation, so the inline direction
is always larger and the flip is always rejected.
Therefore, in module phase, the search never updates
best_compileform— itis a guaranteed no-op. But it still performs
O(functions)fullcodegencallsper root set (and
codegenis itself superlinear in program size), so the totalcost explodes on modules with many synthetic helpers.
The existing source comment already notes that "no program in my test set lost
weight by switching inline off after losing weight by switching it on, and the
cost of this search can be high."
Fix
Skip the search in module phase, where it is provably a no-op. The change adds an
early
return Ok(compileform)whenopts.module_phase().is_some(), before thedepgraph build and search loop. Because the search never updates
best_compileformin module phase, returning the unchanged program isoutput-identical and removes the blow-up. The now-unused module-phase depgraph
branch (and its
DepgraphOptionsimport) is removed.Verification
706 passed; 0 failed; 1 ignored, the 1 ignored being pre-existing).cargo fmt --checkandcargo clippyare clean on the changed files.untouched, so the "recompilation matches" output-stability checks are
unaffected.
above).
test_module_phase_deinline_does_not_blow_up) thatcompiles a multi-export let-stack module
(
resources/tests/module/deinline_module_blowup.clsp) and runs both exports,asserting their results.