Repository navigation
Floor latency: whole-closure Rust emit asks the cheap half of each question first - #12454
Merged
Merged
Conversation
Three places in the seed Rust emitter did corpus-sized work whose answer the cheaper half already decided, or repeated one fold four times. - closure_needs_module_filename_stub asked "does anything reference this module" (a walk of every module's reference candidates) before "is the module already in the closure" (one name per module). In every whole-closure emit -- the stage0 regen -- the module IS in the closure, so both stub decisions walked the whole closure to produce a Bool the conjunction then discarded. Absence is now asked first, reading each module's own source index rather than a merge of all of them. - build_emit_rust_context merged every module's source indices four times over the same input; it now merges once. - struct_candidates_by_field_names built and SORTED each struct's whole key list before comparing a single name, once per struct per anonymous record literal; presence is now asked first (a hash lookup that short-circuits on the first missing field) and the count reads the keys unsorted. The literal's names are unique (02_parse refuses a repeated field init), so presence plus equal count is set equality, as before. Emitted text is unchanged: the seed built from this mirror regenerates all 161 stage0 mirrors equal to the committed ones (first_generation_equal=true), and its whole-closure compile.emit took 4 min against 5 min for the seed before it on the same host. The struct-candidate scan is still one pass over every type summary per literal; an index keyed by field set is the next step. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
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
1 participant
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.
Part of the floor-latency program (target: required floor under 30 min). This PR shortens the stage0 build lane, which after the job split runs as the merge queue's
generatedjob: two whole-closure emits per queued landing, 4m45s and 5m20s on merge-queue run 36339106604.What. Three places in the seed Rust emitter (
src/v1/05_emit_rust.dag) did corpus-sized work whose answer a cheaper half already decided:closure_needs_module_filename_stubwalked every module's reference candidates before checking whether the module is in the closure at all. In a whole-closure emit it always is, so both stub decisions walked the closure only to throw the result away. Absence is now checked first.build_emit_rust_contextmerged all modules' source indices four times over the same input. It now merges once.struct_candidates_by_field_namesbuilt and sorted each struct's key list once per struct per anonymous record literal. Presence is now checked first, and the count no longer sorts. The parser refuses repeated field inits, so presence plus equal count is still set equality.Output unchanged. A seed built from this mirror regenerates all 161 stage0 mirrors equal to the committed ones (
first_generation_equal=true).Measured on one host, same load window: whole-closure
compile.emit5 min → 4 min. A baseline perf profile of that emit putsstruct_candidates_by_field_namesat 36% inclusive. The remaining cost is one pass over every type summary per literal; an index keyed by field set is the follow-up.Overlap with #12448. That PR removes the emitter's 2^depth double renders in the same
.dagfile and the same generated mirror, so whichever lands second regeneratesv1_compiler_emit_rust.rs. Both leave emitted text unchanged, so the regeneration is mechanical.🤖 Generated with Claude Code