perf(dataflow): solve each function once instead of three times - #156
Open
rahlk wants to merge 1 commit into
Open
perf(dataflow): solve each function once instead of three times#156rahlk wants to merge 1 commit into
rahlk wants to merge 1 commit into
Conversation
The L4 serial tail (compute_summaries + assemble_sdg, ~48% of the dataflow layer) was re-deriving the same per-function solution three times over. Instrumenting solve_function on the flask fixture: 1,158 calls for 386 functions, and solve_function is 95% of the tail's wall time. Two independent causes, both fixed: - Singleton SCCs iterated twice. compute_summaries ran `while changed`, so a non-recursive function computed its summary, set changed=True, then recomputed an identical summary purely to observe convergence. The condensation DAG is walked bottom-up, so a one-member SCC with no self-edge has all callee summaries final already and cannot change on a second pass. Genuinely recursive SCCs (several members, or one calling itself) still iterate to fixpoint. - assemble_sdg re-solved from scratch. compute_summaries discarded its own intermediates (`new, _, _ = solve_function(...)`) and the assembler then called solve_function again per signature to recover the facts and DDG. compute_summaries now optionally hands back the converged (facts, ddg) and the assembler consumes them. Sound because a converged pass is by definition one in which no member changed, so those by-products already reflect the final summaries. The recompute path remains the default for callers whose summaries did not come from an immediately preceding run over the same infos. Measured: solve calls 3.0x -> 1.0x per function (386 for 386, zero in the assembler). Interleaved A/B/A/B on erpnext L4 with --ray, load recorded per run: FIXED 231s/257s vs BASELINE 263s/282s, means 244s vs 272s = 10.5% faster; the worst FIXED run still beats the best BASELINE run, so the result survives this machine's load swings. Output is unchanged: flask L4 matches the pre-change baseline exactly on callables (386), cfg (4,372), cdg (2,443), ddg (24,138), summary (3,449), param_in (1,608), param_out (1,201) and the full ddg provenance histogram. Full suite: 291 passed, 6 skipped. Closes #155.
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
1 participant
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.
Closes #155.
What
The L4 serial tail —
compute_summaries+assemble_sdg, ~48% of the dataflow layer — was re-deriving the same per-function solution three times. This makes it once.Instrumenting
solve_functionon the flask fixture: 1,158 calls for 386 functions, andsolve_functionis 95% of the tail's wall time. Two independent causes:compute_summariesranwhile changed:, so a non-recursive function computed its summary, setchanged=True, then recomputed an identical summary purely to observe convergence. The condensation DAG is walked bottom-up, so a one-member SCC with no self-edge already has final callee summaries and cannot change on a second pass. Recursive SCCs (several members, or one calling itself) still iterate to fixpoint.assemble_sdgre-solved from scratch.compute_summariesdiscarded its own intermediates (new, _, _ = solve_function(...)), and the assembler calledsolve_functionagain per signature to recover the facts and DDG. It now receives the converged(facts, ddg)instead. Sound because a converged pass is by definition one in which no member's summary changed, so those by-products already reflect the final summaries. The recompute path stays the default for any caller whosesummariesdid not come from an immediately preceding run over the sameinfos.Measured
Solve count: 3.0× → 1.0× per function (386 for 386, zero in the assembler).
Interleaved A/B/A/B on erpnext L4,
--ray, load recorded per run:Means 244s vs 272s → 10.5% faster. The result survives the load confound in its strongest form: the worst FIXED run (257s at load 21) still beats the best BASELINE run (263s at load 17), and FIXED wins both adjacent pairings.
Why 10.5% and not the ~3× the solve count implies:
solve_functionis 95% of the serial tail, but the tail is only ~27% of an L4 run — the symbol-table phase still dominates. That puts the ceiling near 18%, and 10.5% sits in that range once emission and the remaining assembler work are counted.Output unchanged
flask L4 matches the pre-change baseline exactly: callables 386, cfg 4,372, cdg 2,443, ddg 24,138, summary 3,449, param_in 1,608, param_out 1,201, and an identical ddg provenance histogram (
ssa6,693 /reaching-defs10,129 /points-to7,316). requests L4 verified sane.Full suite: 291 passed, 6 skipped.
Context
This is the follow-up to #154, where parallelising the other half of the dataflow layer with Ray measured 3.1× slower and was reverted. The serial tail turned out to be the right target, and the win was algorithmic rather than parallel.