Stabilize dynamic generic-join ordering - #968
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## ezr-better-parallel #968 +/- ##
=======================================================
+ Coverage 86.77% 86.82% +0.05%
=======================================================
Files 95 95
Lines 30572 30659 +87
=======================================================
+ Hits 26528 26620 +92
+ Misses 4044 4039 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Merging this PR will improve performance by 14.2%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ⚡ | Simulation | tests[luminal-llama] |
657.1 ms | 501.7 ms | +30.96% |
| ⚡ | Simulation | tests[eggcc-2mm] |
8.3 s | 7.7 s | +7.92% |
| ⚡ | Simulation | tests[proof_testing_repro-665-set-union] |
17.1 s | 16.3 s | +5.37% |
Tip
Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.
Comparing codex/dvo-ordering-policy (f001a63) with ezr-better-parallel (a384d25)2
Footnotes
-
227 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
ezr-better-parallel(90ebda4) during the generation of this report, so e02928c was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
7198256 to
f001a63
Compare
yihozhang
left a comment
yihozhang
left a comment
There was a problem hiding this comment.
Left some comments for your agent!
| } | ||
| } | ||
|
|
||
| // We prioritize variables by |
There was a problem hiding this comment.
keep this comment
Edit: This PR is proposing a new sort criterion. We need to document it and briefly talk about the intuition.
| // Count how many times each atom has been refined so far. | ||
| for position in 0..range.start { | ||
| match &instrs[order.get(position)] { | ||
| let use_physical_refinements = instrs |
There was a problem hiding this comment.
Please describe in a comment what use_physical_refinement is.
| ( | ||
| -refinement_count(stage, physical_refinements), | ||
| size, | ||
| -stable_refinement, |
There was a problem hiding this comment.
What is a stable refinement? I doubt this criterion matters for most benchmarks, since it's rather unlikely that two possible stages would already have the same size.
| }; | ||
|
|
||
| // The cached logical prefix remains the stable locality signal. | ||
| for stage in &instrs[..range.start] { |
There was a problem hiding this comment.
Why in other places we use instrs[order.get(...)] while here we use instrs[..]?
yihozhang
left a comment
yihozhang
left a comment
There was a problem hiding this comment.
I also wonder if we should just shuffle instrs in place (instead of shuffling the permutation order over it), as this has a slightly slightly more local/predictable access pattern.
Locally, comparing against the base shared-trie (two PRs back)
Benchmark Before (s) After (s) Δ (s) Δ %
────────────────────────────────────────────────────────────────────────────────────────
hardboiled_conv1d_32.egg 0.117 0.114 -0.002 -2.1% ▼ faster
hardboiled_conv1d_128.egg 0.286 0.284 -0.002 -0.7% ▼ faster
luminal-llama.egg 0.082 0.083 + 0.001 + 1.2% ▲ slower
python_array_optimize.egg 0.228 0.230 + 0.002 + 0.8% ▲ slower
cykjson.egg 0.034 0.032 -0.002 -6.3% ▼ faster
eggcc-extraction.egg 0.200 0.209 + 0.008 + 4.2% ▲ slower
llama.egg 0.370 0.377 + 0.006 + 1.8% ▲ slower
paged_llama.egg 1.667 1.682 + 0.015 + 0.9% ▲ slower
qwen.egg 0.441 0.419 -0.021 -4.8% ▼ faster
qwen3_moe.egg 0.574 0.554 -0.020 -3.5% ▼ faster
whisper.egg 1.274 1.179 -0.095 -7.4% ▼ faster
Summary: 6 faster · 5 slower · 0 unchanged · 0 missing
Overall average Δ: -1.46%
Comparing against #954
Benchmark Before (s) After (s) Δ (s) Δ %
────────────────────────────────────────────────────────────────────────────────────────
hardboiled_conv1d_32.egg 0.111 0.113 + 0.002 + 1.9% ▲ slower
hardboiled_conv1d_128.egg 0.280 0.296 + 0.016 + 5.6% ▲ slower
luminal-llama.egg 0.101 0.083 -0.018 -17.9% ▼ faster
python_array_optimize.egg 0.234 0.239 + 0.006 + 2.5% ▲ slower
cykjson.egg 0.031 0.032 + 0.001 + 4.4% ▲ slower
eggcc-extraction.egg 0.208 0.218 + 0.009 + 4.5% ▲ slower
llama.egg 0.365 0.373 + 0.008 + 2.1% ▲ slower
paged_llama.egg 1.653 1.689 + 0.037 + 2.2% ▲ slower
qwen.egg 0.420 0.409 -0.011 -2.6% ▼ faster
qwen3_moe.egg 0.553 0.558 + 0.005 + 0.9% ▲ slower
whisper.egg 1.166 1.118 -0.048 -4.1% ▼ faster
Summary: 3 faster · 8 slower · 0 unchanged · 0 missing
Overall average Δ: -0.05%
so looks like real but acceptable overhead. Though I'm wondering if we can eliminate it by simply reverting to the old heuristic code.
3 participants
Stacked on #954.
This isolates the dynamic variable-ordering fixes ahead of the packed-trie work:
The small explicit hash-code cast commit is a prerequisite for compiling the current #954 base with Rust 1.91.
Validation: