Repository navigation
DDIR: demand projection for dead fields - #1001
Open
frankmcsherry wants to merge 12 commits into
Open
frankmcsherry wants to merge 12 commits into
frankmcsherry wants to merge 12 commits into
Conversation
frankmcsherry
commented
Oct 22, 2026
frankmcsherry
left a comment
Member
Author
There was a problem hiding this comment.
I reviewed projection.rs and the server hook. The pass looks sound, the numbers are strong, and the points below are about landing it.
What I checked, and found right
- Barriers.
distinct,min,collectand reduce lambdas are opaque: their output demands every input field, and the dead fields of their output are masked by aProjectinserted after them (rewrite_scope). countis not a barrier, correctly. It's the sum of a key's diffs, so merging rows that differ only in dead values leaves it unchanged.- Joins. Merged rows add their diffs before the product,
(a1 + a2)·b = a1·b + a2·b, so masking a join input's dead values is exact. Both keys are always demanded. - Presence. Every fact of a filter's or flat map's output demands what the predicate or list reads, so rows are never kept or dropped differently.
- Root exports are all seeded in
pushdown. Published traces keep their full schema, and imported traces are never rewritten. - The explain-text probe is conservative against today's printer. I checked corgi's
explainat the pinned fdbb5e2:- a tuple prints only node ids,
(%a %b), so field positions can't shift; - nested bodies are indented, so they're skipped and their op counts as reading all of its inputs;
- anything unparsed falls back to "all fields".
- a tuple prints only node ids,
Before landing
- Enabling the pass should never make an install fail.
pushdownreturnsErrwhen shape inference reports conflicts or unknowns before the rewrite, andbuild_scopeerrors propagate too. The server hook?s both.- The post-rewrite check already falls back to the original program. Do the same up front: return
Ok(0)and install the original. Then the pass is purely optional. - Likewise, the
unreachable!inhalf_join, and thepanic!s inPlannedScope::at, would be better as that same fallback.
- The lambda's text and graph disagree after masking.
mask_lambdaandmask_half_joinbuild text withLambda::compose, then replace.graphwith a hand-built graph. The hand-built graph anchors dead fields'Unitto the input, so dead upstream work is dropped.- So the text no longer parses to the graph that runs. Text is what dedup compares and what serde writes, so a round trip brings the dead work back.
- It would be better if the composed text alone gave the lean graph: for example, corgi's
optimizefolding a field of a tuple through thelet. Failing that, a comment atLambdasaying the text is no longer authoritative. - Separately,
mask_half_joinrepeats most ofmask_lambdaand could share it.
- Replace the text probe with corgi #112's API when it merges.
demand_projectionanswers exactly "which input fields does this output read", structurally.- That deletes
parse_graph,input_pathsand most ofgraph_fields, about 150 lines, and the dependence onexplain's format. Your module doc already says this.
- Split the PR.
- The pass is
projection.rs, the server hook, andsuite.py --skip-check. examples/self_opt, about 780 lines, is the #919 self-optimizer port, checked against Rust on the 78.ddpprograms. It's a different thing to review, and could be its own PR, together withdemand.ddpandexamples/demand.rs.
- The pass is
- Default on, later. This is a certain optimization, so the end state is on by default, without
DDIR_PROJECTION. A 50,000-case sweep with the pass on, using the widened harness (half joins, frames, feed-from), is running overnight. If it's clean, I'd make the pass the default, in this PR or the next. - Small: the per-install
eprintln!on worker 0 should go, or move behind the same flag.
A question. How does the pass interact with explain? The fallback comment says some explanation plans "require the producer's original shape". Does explaining a masked program explain the masked program or the original one? Either is defensible, but it should be one, written down.
🤖 Generated with Claude Code
frankmcsherry
force-pushed
the
codex/ddir-projection-final
branch
from
October 22, 2026 03:23
6f66ddf to
61cd2fd
Compare
This was referenced Oct 22, 2026
frankmcsherry
marked this pull request as ready for review
October 22, 2026 13:50
This branch has not been deployed
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
Compute key and value field demand backward through scopes and loops to a fixed point. Mask dead producer fields with
()while preserving tuple positions and the external schema. Corgi removes work feeding the masked fields.distinct,min,collect, and reduce lambdas keep their complete inputs. This PR does not move work through joins or change the physical plan.The pass now uses Corgi
acff1dfeand its typeddemand_transform/demand_projectioninstead of parsing explain text. Install validates the original program first, creates generated sources, then tries the optional rewrite and rechecks its shapes and exports. Any analysis or shape failure falls back to the original program. Composed lambda source is documented as descriptive when its graph has been rewritten. The self-optimizer and demand examples are in draft #1002.SF0.1 results
One release server binary (SHA-256 prefix
fc5ff230243a), 41 SNB reads installed together, four workers, one measured mixed-mode round. Naive queries usesnb/queries.pyfromb97d3d47before #996; curated queries use #996./usr/bin/time -lreports whole-server instructions and maximum RSS. All four runs passed; all 248 recorded answer hashes and effective bindings matched. Python reference checking was disabled for this timed comparison. These are single runs, so RSS has run-to-run variation.The pass saves 25.5% of instructions on naive plans and 7.9% on curated plans.
Validation
cargo test --workspace --all-targetswith the pass off.DDIR_PROJECTION=1 cargo test --workspace --all-targetspassed after the hook and demand API changes.server_shapes,server_install, andserver_columnar_sourcestests passed with the flag on.cargo clippy -p interactive -p ddir-server --all-targetscompleted with existing repository warnings.DIFFTEST_SEED=0 DIFFTEST_CASES=5000 DDIR_PROJECTION=1release sweep passed 5,000/5,000 cases, zero refused or failed. The coordinator reported about 5.2M overnight cases on the earlier pass without a wrong answer.Landing and follow-up
I recommend landing this opt-in and making it the default, removing
DDIR_PROJECTION, in a small next PR. Physical removal of()positions with Corgicompact_unit_fieldsis separate. Curated IC12's request-first class path and BI15's edge semijoin restrict rows and require planner choices beyond automatic field demand. Theexplainquestion remains with Frank.