Split transactional commit stores by connected components of transaction blocks - #91
Merged
Conversation
Contributor
Author
This was referenced Aug 13, 2026
dpmills
force-pushed
the
dmills/txn-store-split
branch
from
August 13, 2026 05:39
1401d61 to
5a0d635
Compare
dpmills
force-pushed
the
dmills/txn-store-split
branch
2 times, most recently
from
August 13, 2026 21:57
3b33ad1 to
eac2ffc
Compare
dpmills
force-pushed
the
dmills/txn-store-split
branch
from
August 13, 2026 23:42
eac2ffc to
f8d2300
Compare
dpmills
force-pushed
the
dmills/txn-store-split
branch
from
August 14, 2026 19:49
f8d2300 to
d4a5a2b
Compare
dpmills
force-pushed
the
dmills/txn-store-split
branch
from
August 14, 2026 21:34
d4a5a2b to
19b0fd5
Compare
An error occurred while trying to automatically change base from
dmills/explicit-register-reads
to
dmills/mut-typing-fixes
August 14, 2026 21:41
dpmills
force-pushed
the
dmills/txn-store-split
branch
from
August 14, 2026 22:00
19b0fd5 to
5574f0d
Compare
dpmills
force-pushed
the
dmills/txn-store-split
branch
2 times, most recently
from
August 15, 2026 03:51
9180257 to
82e2a20
Compare
dpmills
marked this pull request as ready for review
August 17, 2026 22:27
dpmills
force-pushed
the
dmills/txn-store-split
branch
2 times, most recently
from
August 18, 2026 18:20
8fbd850 to
fb84c73
Compare
sortalongo
approved these changes
Aug 18, 2026
Two `for` loops over `:=` accumulators have always compiled to two induction stores, one letrec each. Two `with begin():` loops over unrelated `Mut(_, Txn)` registers compiled to **one** commit store, because `transact_phase` unioned every writer site's footprint into a single key set. That coupled two things that should be independent: their commit clocks, and their completion. The second is observable — a store is terminal only when *every* writer has drained, and a fed-out read is an `AsOf`, non-terminal until its store is — so a finite register's trailing read could not settle if any unrelated register had a live-source writer. This partitions the keys, giving the transaction path one store per set of registers some block actually relates. A `with begin():` block, and the two reasons land on the same set — its footprint. **Atomicity**: a writing block produces one commit record, so the keys it writes advance at one tick and the keys it reads come from that tick's snapshot. **Snapshot consistency**: a read-only block's reads are latched at one frontier, which `build_snapshot` realizes by handing `AsOf` a single register record. Nothing else is a reason to share; `partition_keys` is union-find over exactly those two, and its docs carry the argument. That is user-visible, not an implementation detail, so it is stated in the ordering model as a consequence a program may rely on ([chl-spec.md](docs/chl-spec.md#85-ordering-and-concurrency)): registers no block relates have no order between them, and one register's history can complete while another's is still open. A read-only block is unwrapped onto the spine and leaves no `WriterSite`, so its footprint was being discarded; `strip` now keeps it. Not hypothetical — `registers_read_together_share_a_store` has the same writers as `unrelated_registers_get_separate_stores` and differs only in the read. `build_letrec` becomes `plan_store` plus `splice_stores`. A store sits below everything its bindings need and above everything reading its keys; each spine statement gets a **level** — 0 if it reads no store, else one past the last store it reads, transitively — and rides inside that store. A statement cannot read two stores at once, which is what makes the level well-defined: reading two registers together is a block, and the partition put those keys in one store precisely so the read has one snapshot. `CrossDomain` wraps the whole nest, since an accumulator a decision reads must be outside every store reading it. Both mutability paths ended in the same four steps — trailing reads, feed hoists, causality assert, `LetRec` — now `mut_elim::close_recurrence_group`. It puts the assert where neither caller can forget it, and states as load-bearing an ordering both had right by coincidence: a read hoisted over a feed would break `channelize`'s outermost-first collection silently. Nine, four of them asserting the *planned graph* rather than only values — two programs can agree on every number and differ in store count. `a_finite_register_completes_despite_a_live_unrelated_writer` is the payoff, checked against the base rather than assumed: there the read yields empty forever. Recognition and op-conversion needed no changes.
dpmills
force-pushed
the
dmills/txn-store-split
branch
from
August 19, 2026 03:45
fb84c73 to
2648f6c
Compare
groundlar
added a commit
that referenced
this pull request
Aug 21, 2026
… surfaces (#98) `Clone` was derived on `Expr`, so it copied `node_id`: a subtree reaching the output at two positions left both nodes sharing one identity, which makes the pane projection ambiguous and collapses the pair into a single entry in every `NodeId`-keyed walk. Only one pipeline boundary — `post-lowering` — checked for that. This adds the other seven (`post-inline`, `post-transact`, `post-letrec-run`, `post-desugar`, `post-live-read`, `post-lambda-elim`, `post-planning`), fixes the two violations they surface, and then **removes the class of defect entirely by making `Clone` freshen**. ## The two violations the new boundaries surfaced **`simplify`'s zip-distribute** placed the left operand on both legs with bare clones, so the legs shared an identity. It survived because the follow-on product-beta consumes one copy per leg whenever the arms read different slots (`⟨.0, .1⟩`); only same-slot arms (`⟨.0, .0⟩`) leave both live. The first leg is now the survivor, the second a freshened sibling. **`transact_phase::subst_var_with`** freshened the whole replacement, satisfying uniqueness by *deleting* the occurrence's id — and the read site is user-written, so its span went too. It now root-carries, like the two `subst_env`s below it. `planning::groupby`'s key lift was flagged by the same audit and is already safe: it leaves the predicate domain through `lambda_elim::run`, which re-mints every node. `groupby_recognition_lifts_the_key_without_aliasing` pins that, so an elim that started preserving ids fails there rather than at a pane boundary. ## `Clone` freshens, and the opt-out is named A clone is a **sibling**, not the same node. The hand-written `Clone` mints a new `NodeId` for every node it copies and reports each `(origin, fresh)` pair through `on_copy`. The recursion is the derive's — `node.clone()` clones the children, each reaching the same impl — so the freshen is deep by construction and **fused** into the copy rather than being the second walk `fresh_copy` made. `fresh_copy`, `freshen_node_ids_deep`, `freshen_interior_node_ids`, `freshen_node_id` and monomorphization's `freshen_clone_node_ids` are all deleted; 34 call sites become a plain `clone()`. **The decision inverts rather than disappearing.** The safe default is now automatic, and a copy that is *not a new node* says so through `clone_preserving_ids`. Two shapes qualify — a snapshot taken for rollback or comparison, and a test comparing trees across a pass — plus the moves-out-of-a-borrow, where Rust forces a clone, the source is dropped, and the copy takes its position. Those last were *already* id-preserving here; only the spelling flips, since `clone()` used to preserve and `fresh_copy()` opted into freshening. What it is **not** for is silencing a leak. An `Unexplained` or `ParentUnknown` means a copy was made with no frame open, or against an origin the table never recorded — a *recording* gap, whose honest fix is a bracket. ### Measured, because the objection was about cost Release, min-of-5, against the unchanged parent, on the instrumented top of the stack (so the pane gates were live): | program | compile time | peak RSS | ids | |---|---|---|---| | `stress_10_10_3_10` | 1.067 → **1.030 s** (0.97×) | +2.2% | 2.09× | | `stress_10_10_5_12` | 3.702 → **3.472 s** (0.94×) | +1.3% | 1.74× | | `stress_10_10_8_12` | 21.631 → **20.569 s** (0.95×) | +0.5% | 1.36× | Compile time *improves*, consistently and with a mechanism: one fused walk replaces copy-then-walk. Isolating capture with `CAMBRA_LINEAGE=0/1`, the lineage table goes 5.2 MB → 10.3 MB on a 1 GB compile; with capture off the two arms agree to 0.05%, so the freshening itself costs no memory. Even the unmitigated arm — freshening everywhere, no opt-outs — was faster than baseline at 2–3× the ids. ## Lowering records its predicates `collect_tree_ids` now reaches refinement predicates, so the lowering fold has to explain them. It could not: lowering builds a predicate from already-lowered sub-expressions plus the nodes minted and copied to join them up, seals it into a `Refinement` via `refined_data_fun`, and those assembly nodes live in a type slot outside the `walk_children` domain. 1331 of them across 80 pipeline tests. **Nearly half are copies that exist only because `Clone` freshens** — they used to alias already-tagged main-tree ids — which is why this rides in the same commit. `LoweringContext::tag_predicate` sweeps the finished predicate at the three sites that build one. A copy-frame cannot do this job: `flush_into_lowering` deliberately asserts a lowering frame captures no mints (a frame-flushed step carries no span, so a mint routed through one would be silently unresolvable), and a predicate is a mixed mint/copy region. **The sweep skips nodes that are already recorded, and that is the load-bearing part.** The fold is last-write-wins, so a blanket sweep leaves every node perfectly *explained* while silently replacing its real span and label with the sweep's coarse ones — 318 nodes on the corpus, with every gate green. `the_predicate_sweep_skips_already_recorded_nodes` pins it, because no leak class can. `assert_unique_node_ids` stays narrow: it answers uniqueness, not explanation, and predicates legitimately alias main-tree ids at inline's blind spot. ## What this PR cannot prove on its own This commit gates on the lowering fold and uniquify's id-stability tripwire, and that is all — the `NodeId`-keyed table and the pane boundaries arrive in #107. The pane snapshots, `check()`'s scratch copy, the register-init stash and the type-domain discharges carry no local test; they are verified on the instrumented stack and land here on that evidence. Two markers record what the next commit must do: - **`TODO(mono-frame)`** in `specialize_use`: when the recorder arrives, its frame must open **before** the clone, since the clone is what fires `on_copy` — a frame entered after it leaves the whole specialization `Unexplained`. - **`TODO(predicate-domain)`** on `lineage::preserving_ids`: one legitimate caller (`PredMemo`'s rebuild), slated for removal once predicate mints are recorded. Inference's predicate producers are deliberately out of scope here — there is no pass recorder over inference yet, so a bracket would be a no-op with no test that could fail. Full reading, including the eight distinct intents that were hiding under one `clone` and the exhaustive compiler-oracle sweep of all 118 production clone sites: `freshening-clone-report` in the vault. --- ## Rebased onto `main` (`7a9d1201`) Restacked onto current `main`, which brought #91 (commit stores split by connected components of transaction blocks) and #92 (`await_final` terminal reads). **One new commit rides here:** `ccl(transact): a dropped as-of read moves its body rather than freshening it`. `rewrite_as_of_reads`' dead-binding sweep (`drop_dead_as_of_reads`) does `*e = (**body).clone()` to drop a `let` whose as-of read nothing reads. That was a move when `clone()` preserved ids; with the inversion above it re-mints the whole continuation instead, stranding every id below the dropped binding — and on a reply chain that subtree is the rest of the program. It now `mem::take`s the body, which copies nothing at all. The site arrived upstream *after* the clone audit in this PR ran, so it was never one of the 118 sites that sweep classified. Read that claim as "118 as of the pre-rebase tree"; this is the 119th, and it is the one case the sweep's own argument predicts — a `clone()` that was a move, at a call site nobody re-read when the default flipped. **The second placement of a register init is gone.** The trailing `final_or_default(reg_x, init)` read that placed a key's seed a second time is now an `as_of_read` with no seed operand (#92), so the freshening this PR added there had nothing left to freshen and was dropped rather than ported. **Boundary rename:** `assert_unique_node_ids(&expr, "post-live-read")` is now `"post-as-of-read"`, following upstream's rename of `rewrite_live_reads` → `rewrite_as_of_reads`. **Stale numbers.** The compile-time / peak-RSS / id-ratio table above was measured before the restack and has **not** been re-run. The mechanism it reports — one fused walk replacing copy-then-walk — is unchanged by the rebase.
groundlar
added a commit
that referenced
this pull request
Aug 21, 2026
) The mutability-elimination phases each carried their own substitution: a `subst_env` in `mut_elim`, another in `transact_phase`, and a single-name `subst_var_with` beside it. `Subst` already is a simultaneous map, so these were a second engine reimplementing it — and drifting, since a property fixed in one copy did not reach the others. This collapses all three onto `Subst::discharge_env_in_place` / `discharge_env`, and fixes two properties only one caller had. ## Two properties the copies got wrong **Simultaneous, not a fold.** The env form is a constructor over `Subst`, not a loop of single-name discharges: a sequential fold re-substitutes into a replacement that mentions another key, sending `{a ↦ b, b ↦ 0}` to `0` instead of `b`. Caller ranges are key-free today, but that is a property of the callers, not of the operation. **Root-carry.** Each replaced occurrence keeps its own `NodeId` — the replacement's root is built at the occurrence's id, inheriting the read site's span, and only the interior is freshened. A whole-subtree freshen discards both. ## The arrow species `Compose` has its ends recomputed from the rewritten elements: substituting a `Var` whose type was an unresolved placeholder can concretize them, and the `Compose.ty == Fun(first_domain, last_codomain)` invariant must follow. `FunKind` and the Pi binder belong to the composition rather than to its elements, so they survive — a data arrow (`⤇`) is no longer downgraded to a compute one (`⇒`). `ccl_utils::free_among` answers the free-variable question for a whole candidate set in one traversal; the per-name `is_free` made selecting a live subset of a wide environment cost `|candidates| × |expr|`. ## Rebased onto #98's freshening `Clone` #98 now makes `Clone` mint a fresh `NodeId` for every node it copies, which changes how the root-carry above is *achieved* without changing what it guarantees. Previously the replacement was cloned (sharing ids), then its interior was freshened explicitly via `freshen_interior_node_ids`, leaving the root free to take the occurrence's id. Now the clone freshens root and interior alike, and `re_root` overwrites the root with the occurrence's id immediately after. So `freshen_interior_node_ids` is gone from these sites — it is what `Clone` does — and the sequence is simply `rep.clone().re_root(occurrence_id)`. The guarantee is unchanged: each replaced occurrence keeps its own `NodeId` and its read site's span, and N reads still give N distinct roots. The one cost is that the clone mints a root id which the re-root then discards — one stranded id per substituted occurrence, which folds as a death rather than a defect, and which the occurrence itself never leaves the live set for. That trade was taken deliberately over adding a `clone_at(node_id)` constructor, to keep #98's diff minimal. --- ## Rebased onto `main` (`7a9d1201`) **The arrow-species fix landed upstream independently**, as #113 — the identical `Type::fun` → `Type::fun_like` one-liner in `subst.rs`. So this commit no longer carries that change. What it still carries on that point is the part upstream did *not* land: the two tests that pin the property (`a_compose_keeps_its_fun_kind_and_binder_across_a_discharge`, `a_data_compose_is_not_downgraded_to_a_compute_arrow` — #113's whole `subst.rs` diff is 5 insertions and no test), the failure mode spelled out at the call site, and the `rewrite_expr` doc. "The arrow species" section above should be read as what the commit now *asserts* rather than what it fixes. The claim was re-verified against upstream's `ty.rs` rather than assumed, because #113 is titled *"Relate the two function kinds by equality instead of ordering `Data` below `Compute`"* and could have moved it: `Type::fun` still stamps `None`/`Compute`, `Type::pi` still stamps `Some(n)`/`Compute`, `data_fun` still stamps `Data`, and `fun_like` copies `name` and `kind` off its exemplar. What #113 changed is how the two kinds *relate in the solver* — they are now incomparable and pinned rather than ordered, and `FunKind::Var` joined the enum — none of which touches what these constructors stamp. **The substitution collapse itself is unchanged**, and upstream's rewritten `transact_phase` (#91/#92) brought new call sites for the engines this PR deletes; they are converted to `Subst::discharge_env` the same way. `free_among`, `discharge_env_in_place`'s one-traversal selection, and root-carry are as described above.
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.
Two
forloops over:=accumulators have always compiled to two induction stores, one letrec each. Twowith begin():loops over unrelatedMut(_, Txn)variables compiled to one commit store, becausetransact_phaseunioned every writer site's footprint into a single key set. That coupled two things that should be independent: their commit clocks, and their completion. The second is observable — a store is terminal only when every writer has drained, and a fed-out read is anAsOf, non-terminal until its store is — so a finite variable's trailing read could not settle if any unrelated variable had a live-source writer. This partitions the keys, giving the transaction path one store per set of variables some block actually relates.Planning and placement come apart
build_letrecbecomesplan_storeplussplice_stores. A store sits below everything its bindings need and above everything reading its keys; each spine statement gets a level — 0 if it reads no store, else one past the last store it reads, transitively — and rides inside that store. A statement cannot read two stores at once, which is what makes the level well-defined: reading two variables together is a block, and the partition put those keys in one store precisely so the read has one snapshot.CrossDomainwraps the whole nest, since an accumulator a decision reads must be outside every store reading it.Both mutability paths ended in the same four steps — trailing reads, feed hoists, causality assert,
LetRec— nowmut_elim::close_recurrence_group. It puts the assert where neither caller can forget it, and states as load-bearing an ordering both had right by coincidence: a read hoisted over a feed would breakchannelize's outermost-first collection silently.Tests
Nine, four of them asserting the planned graph rather than only values — two programs can agree on every number and differ in store count.
a_finite_mut_var_completes_despite_a_live_unrelated_writeris the payoff, checked against the base rather than assumed: there the read yields empty forever. Recognition and op-conversion needed no changes.