feat(ir): share identical post-ASAP subtrees across workload queries - #356
Merged
Merged
Conversation
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.
Why
A workload can contain multiple queries that use the same selected summary producer. If their post-ASAP trees were constructed separately, structurally equal producers can still have different
Rcidentities. A downstream executor then has to rediscover their equivalence or risks treating them as separate computations.ASAPPlanner should expose that sharing in its own post-ASAP DAG, so downstream systems can bind the selected computation without implementing another semantic common-subexpression pass. This supports ASAPQuery-backend's workload migration and is reusable by other consumers of the IR.
What
Add an opt-in
share_common_summary_subtreesoperation for a set of selected workload roots. It makes structurally equal post-ASAP subtrees reference one node while preserving every caller-supplied root ID and the input order.Equality includes the expression, output schema and result guarantee—not just the summary family or source name. Distinct readouts remain distinct even when their producer is shared.
How
Traverse children before parents, rebuild each node with its shared children, and reuse an equal node from a pool local to the call. Memoize already visited input identities to retain existing sharing. Compare canonical child identities and node-local fields instead of recursively expanding shared descendants. Exact-expression values, summary updates, readout parameters and guarantees use typed equality plus a serialization check that distinguishes signed zero; serialization alone never establishes equality for nonfinite values. No source, grouping, parameter or accuracy coercions are introduced.
This represents selected sharing explicitly in Planner's IR instead of asking each backend to infer semantic equivalence from physical fingerprints. It is not a new optimizer, a process-wide cache, or a change to query results.
Before this PR
Consider two dashboard queries:
Assume planning has selected the same DDSketch parameters, source/window expression, grouping, producer schema and producer guarantee for both, within one data/maintenance scope. Only the quantile readout differs. Separately constructed trees can look like this:
A and B are structurally equal but have different object identities. An identity-based binding pass sees two producers. The equal work is not expressed as one shared node, even though both queries could consume it.
After this PR
Pass both selected roots together to
share_common_summary_subtrees:Both readouts now reference the same producer
Rc; Q95 and Q99 remain separately addressable result roots. A downstream binder can attach one compatible physical producer and let both consumers read it. For N observations in one source partition, this enables one N-update maintenance stream rather than two separate N-update streams. That is the execution opportunity exposed by the DAG, not a measured speedup or an execution guarantee made by this PR.If producer parameters, schemas, guarantees or source expressions differ, those producers are not merged. Different snapshots or independent maintenance scopes must use separate calls, even if their expressions look identical. The caller owns that scope decision; the downstream runtime still validates implementation/lifecycle compatibility and enforces shared execution.
Verification
The focused
post_asap::csetests check:Review fixes add regressions for signed zero in both root orders, nested exact expressions, nonfinite values, compatible versus incompatible quantile producers, and a repeated shared diamond. The signed-zero and diamond regressions were first run against the original implementation: both failed, respectively on incorrect coalescing and the five-second runaway guard.
Local verification on fix commit
2ce5d03:cargo test -p asap-types --lib— 160 passed; all 7 focused CSE tests pass.cargo fmt --all -- --check,cargo clippy --workspace --all-targets --all-features --locked -- -D warnings, andcargo test --workspace --all-features --lockedpassed. GitHub CI has been retriggered for the updated head.Measured review reproducer: two equivalent
x_n = x_(n-1) + x_(n-1)DAGs, depth 24, with 50 unique input nodes total. On the same local debug-build setup (Rust 1.97.1), the original took 25.66 seconds; three post-fix runs took 2.036, 2.049 and 2.032 milliseconds. These are local diagnostic measurements, not an end-to-end query benchmark or performance SLA.The dashboard above is an explanatory workload example; this PR's tests establish the IR identity behavior using structural fixtures, including sketch producer/readout nodes, not a running DDSketch backend. Screenshots are not applicable. Review and regression design were performed by the implementation author, not an independent reviewer.
Scope limits: this pass does not select candidates, price complete workloads, serialize stable runtime identities, or schedule/materialize shared producers. Pool lookup remains linear in pool size, so the overall pass is not claimed to be linear-time.
KeepPreAsapexpressions remain opaque: comparing separately allocated exact expressions still incurs their structural comparison/serialization cost. The fix removes repeated expansion along post-ASAP child edges; it is not a general optimization of pre-ASAP equality. Downstream binding and runtime acceptance are separate migration work.