Repository navigation
refactor(code-index): route chunk fan-outs through one yielding helper - #1491
Closed
ScriptedAlchemy wants to merge 2 commits into
Closed
ScriptedAlchemy wants to merge 2 commits into
ScriptedAlchemy wants to merge 2 commits into
Conversation
`try_for_each_chunk_ordered` and `admit_all` each wrapped their own `par_iter` in `with_yielded_background_cpu_permits`. That wrap is the nested-admission fix from #1449, and the merge resolution in d039bee kept both `par_iter`s while dropping both wraps, leaving the base wedge-prone for sixteen merges until 76a8e78 restored it. Move the fan-out into a private `fan_out` module whose single `map_yielding` performs the yield internally and is the only place in `chunks` that imports `rayon`, so a bare `par_iter` elsewhere in the file no longer compiles and the yield cannot be separated from the join. Outputs are collected in input order, so the ordered sweep's lowest-index failure is the first `Some`. No behaviour change. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
Scan `chunks.rs` and assert that every pool token (`rayon`, `par_iter`, `par_chunks`, `par_bridge`) outside comments sits inside `mod fan_out`, and that `fan_out` itself carries the yield. Token literals are built with `concat!` so the test's own source does not count. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
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
chunks.rs(try_for_each_chunk_ordered,admit_all) behind one privatefan_out::map_yieldingthat performs thewith_yielded_background_cpu_permitswrap internally.rayonis now imported only insidemod fan_out, so a barepar_iteranywhere else inchunks.rsfails to compile.chunks.rstomod fan_outand pins the yield to that module.9fbf169fe; not re-rebased as the tip moves.Motivation
The nested-admission deadlock fix from #1449 (
f81df3b34) was two call-site wraps around twopar_iters. The merge resolution ind039bee89(#1435) kept bothpar_iters and dropped both wraps; the base stayed wedge-prone for sixteen merges (#1435 through #1383, including630702adbwhere the hang was re-observed) until76a8e7838(#1460) restored them. A fix that a conflict resolution can silently detach from the code it protects is not durable. Welding the yield to the fan-out inside one helper, and making that helper the only route to the pool in the module, means the yield cannot be kept out while the fan-out stays in.Changes
crates/tracedecay-code-index/src/chunks.rsmod fan_outwithpub(super) fn map_yielding<I, R>(items, map) -> Vec<R>for anyIntoParallelIteratorwhose iterator is indexed. It yields the caller's admitted units around the join and collects outputs in input order.try_for_each_chunk_ordered:map_yielding(chunks, |chunk| admit(&mut || operation(chunk)).err())then the firstSomein input order. This equals the previousfilter_map+min_by_key(index)because indexedcollectpreserves order.admit_all:map_yielding(chunks, |chunk| with_background_cpu_permit(|| self.admit(chunk))).into_iter().collect().use rayon::prelude::*removed; the import lives insidemod fan_out.every_pool_fan_out_in_chunks_goes_through_the_yielding_helper:include_str!("chunks.rs"), skip comment lines, assert norayon/par_iter((also matchesinto_par_iter() /par_chunks/par_bridgetoken outsidemod fan_out, and assert the module containswith_yielded_background_cpu_permits(. Token literals are built withconcat!so the test's own source is not an occurrence. It caught itself on the first run (identifiers namedrayon_tokens), which is the evidence it scans what it claims to.< PARALLEL_CHUNK_THRESHOLD) untouched. No behaviour change on the parallel path beyond collectingOption<ChunkingFailureV1>per chunk instead offilter_map.Test plan
All on the fork point
9fbf169fe,--profile perf,CARGO_INCREMENTAL=0, own target.cargo test -p tracedecay-code-index --libwhole binary ×3rayon_tokensidentifiers before the rename; passes after — the scan sees real code linescargo fmt --checkchunks.rscargo clippy -p tracedecay-code-index --profile perf --no-deps --all-targets --keep-going -- -D warningsE0425intests/code_index_suite/chunk_incremental.rs:362,366(base red, reproduced identically with this diff stashed)npm run lint:commit -- --from origin/codex/... --to HEADcargo test -p tracedecay-code-index-runtime --lib -- reconcile9fbf169fe(publication_store.rs:2640.reused), identically with this diff stashed; being fixed separately. The same filter completed 2/2 (168 tests) ond1586e4aeearlier today with identical wrappers in place.cargo nextest run --workspace --no-fail-fast— CI (expected red until the base compile fixes land; merge only on real SUCCESS)Checklist
CHANGELOG.md— internal refactor + test, no user-facing change.envfiles includedBase reds seen while verifying (not fixed here)
crates/tracedecay-code-index-runtime/src/code_index_scheduler/publication_store.rs:2640—.reusedonChangedCodeChunkSetV1; the runtime crate does not build at9fbf169fe.crates/tracedecay-code-index/tests/code_index_suite/chunk_incremental.rs:362,366—parallelism::force_install_failure_for_testis no longer exported from the lib to integration tests.code_index_scheduler::tests::reconcile::concurrent_query_admissions_claim_one_pending_wake_before_worker_coalescingfails alone 2/3 ond1586e4ae(admitted == 0); present sinced62bf238d, unrelated to background-CPU admission.