Repository navigation
claim_executor floor: dedup same-entry resolves within a batch (resolve once, share the graph) — §2 redundancy cleanup - #5619
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.
Summary
claim_executor's floor walk (run_walk) spawned one OS thread per runnable in a batch, and eachthread re-resolved its entry's full import closure from scratch. In CI batch-2, six gate witnesses
(
rust/emit/layering/resolved_imports/extdeps/generated) all live in one file(
dsl/tools/floor_effect_gate_witness.dag), so its identical resolved-and-typechecked graph wasbuilt and held 6× concurrently — pure duplicated work (DESIGN §2).
This PR partitions each batch into resolve-groups by
entry: SingleClaims sharing one fileresolve (and typecheck) that file exactly once and run every claim sequentially on the one shared
interpreter context. Distinct entries and the discovery-corpus node keep their own concurrent
threads. Resolve is a pure function of
(source_roots, entry), so sharing the graph acrosssame-entry claims is semantically identical — correctness by construction.
This is an honest §2 redundancy / CPU cleanup. It is not the floor's OOM lever: the dominant
self-RSS is the discovery-corpus path (per-shard footprint ×
spawn_width), and the cgroupworst-max (the hard-cap input) is driven by the rust-gate
cargo testrustc child processes — neitheris touched here. The corpus-path footprint is a separate follow-on, rooted in the budget/width
authority.
Measured (by-execution, controlled before/after; OLD vs patched binary, same 6-same-entry plan)
A proxy witness with a byte-identical import closure to the real gate file (106 modules / 1300
items vs 106 / 1301) was used so no effectful gate runs:
→ ~0.57 GiB of self-RSS removed on the gate group, plus ~5× redundant parse/typecheck CPU. Per-gate
resolve is ~148 MB (the larger ~0.9 GiB figure seen via
claim_batchis its whole-tree mock-keyprecompute, which the executor's SingleClaim path does not run).
Correctness (DESIGN §5 — the floor harness must never fail open)
A discriminating control ran three same-entry claims
[pass, FAIL, pass]through the grouped path:1 resolve-groupfor the three (grouping confirmed),(shared ctx not poisoned), exit code 1 (fail-closed preserved).
Same-entry ctx reuse is the already-exercised discovery-corpus pattern (
run_discovery_rowsrunshundreds of claims on one per-entry ctx); this change does not touch the cross-entry parse/typecheck
cache, so the #5295 partial-type poisoning shape does not apply.
Test plan
cargo test -p v1-compiler --bin claim_executor— 4 passed (grouping coalesces same-entryclaims, preserves every claim exactly once, empty-entry sentinel stays an isolated fail-closed
unit, discovery node stays isolated).
cargo clippy -p v1-compiler --bin claim_executor --all-targets -- -D warnings— clean.cargo fmt --all --check— clean.Reviewer note —
checks_state"cancelled" is a FALSE negativeThe dashboard shows
checks_state: failingbecause a CANCELLED check-run sits beside a SUCCESS onthis head (the known cancelled-sibling pin). The real floor run is GREEN —
cirun28005219762passed (andrust_testspassed). The CANCELLED entries are a duplicate-triggerartifact, not a failure; per the lockdown manager they are intentionally not rerun (a rerun moves
no bits but re-fires reviews / resets approvals). Read this PR as floor-green.