feat(bootstrapper): run phase items in parallel when possible

[dhellmann]

Summary

Restructure the bootstrap-parallel command's main loop to run independent work items concurrently using a ThreadPoolExecutor.

How it works

The bootstrap loop maintains a LIFO stack of WorkItem objects. Each iteration collects a batch from the top of the stack and submits it to the executor. BootstrapPhase.can_parallelize controls batch size: parallel phases collect all consecutive same-phase items (running them concurrently); serial phases pop only the single top item (allowing other phases to interleave while still processing one at a time). All items — serial or parallel — go through the same executor.

Phase breakdown after this change

Phase	Parallelism	Reason
RESOLVE	parallel	resolver is thread-safe; independent per-package
START	parallel	graph writes are locked
PREPARE_SOURCE	parallel	downloads/unpacking are independent
PREPARE_BUILD	serial	installs build system deps; must complete before backend queries
GET_BUILD_DEPS	parallel	backend queries are independent (each package has its own build env)
INSTALL_BUILD_DEPS	serial	installs build backend/sdist deps; dep's wheel must exist in mirror first
BUILD	parallel	compilation is independent per package
UPDATE_BUILD_SEQUENCE	serial	serialises writes to build-order.json
PROCESS_INSTALL_DEPS	parallel	hook + dep extraction are independent
COMPLETE	parallel	cleanup is independent

Thread-safety changes

• why dependency-chain stack converted to a contextvars.ContextVar; ThreadPoolExecutor copies the caller's context automatically for each task
• _state_lock guards all check-then-set write sites (_seen_requirements, _build_requirements, _build_stack, failed_packages, _failed_versions, dependency graph, progress bar)
• _wheel_dir_lock guards wheel directory operations to prevent TOCTOU races between _find_cached_wheel and update_wheel_mirror
• Progress bar updates via add_done_callback so items tick off as they complete rather than at batch boundaries

Testing

New e2e test test_bootstrap_parallel_multiple_versions bootstraps flit-core 3.0.0--3.12.0 with 5 workers, verifying that all buildable versions are produced and that each parallel phase logged at least one parallel batch.

Closes: #1149

[coderabbitai]

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 553f3aa6-84c5-43fe-a7c2-8f48e5580d63

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

📝 Walkthrough

Walkthrough

This PR implements parallel execution for the iterative bootstrap loop. The Bootstrapper now batches same-phase WorkItems and processes them concurrently via ThreadPoolExecutor. A new BootstrapPhase.UPDATE_BUILD_SEQUENCE separates build-order recording from install-dependency processing. Thread-safety is enforced via _state_lock (dependency graph, build-order, progress) and _wheel_dir_lock (wheel discovery/mirror updates). The requirement resolver's session cache is protected with threading.Lock. Command-level integration passes --max-workers through to the bootstrap subcommand, with bootstrap enforcing serial execution. Comprehensive unit tests validate phase parallelization, ordering constraints, lock acquisition, and test-mode behavior. A new e2e test confirms multi-version bootstrap with parallel batching.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely summarizes the main change: enabling parallel execution of bootstrap phase items where safe.
Linked Issues check	✅ Passed	All code changes implement the objective from #1149: determining which bootstrap phases can run concurrently and implementing parallel execution via ThreadPoolExecutor with appropriate synchronization.
Out of Scope Changes check	✅ Passed	All changes are scoped to #1149: thread-safety additions, batching/parallel execution logic, new UPDATE_BUILD_SEQUENCE phase, test coverage, and CLI wiring for max_workers.
Description check	✅ Passed	The PR description clearly explains the parallel executor restructuring, phase breakdown table, thread-safety changes, and testing approach.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/fromager/commands/bootstrap.py (1)

193-202: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Add max_workers parameter to bootstrap() function signature and pass it through to Bootstrapper.

bootstrap_parallel() forwards max_workers via ctx.invoke(bootstrap, ..., max_workers=max_workers, ...), but the bootstrap() function does not declare this parameter, so the forwarded value is rejected or ignored. Additionally, the Bootstrapper is hardcoded with max_workers=1 at line 200, preventing any configuration of parallel bootstrap execution. Either add max_workers: int | None to the bootstrap() signature and pass it through to the Bootstrapper constructor instead of hardcoding 1, or remove the forwarded kwarg from bootstrap_parallel().

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/fromager/commands/bootstrap.py` around lines 193 - 202, The bootstrap
function is missing a max_workers parameter and always constructs Bootstrapper
with max_workers=1; update the bootstrap(...) function signature to accept
max_workers: int | None (default as needed) and pass that value into the
Bootstrapper constructor instead of the hardcoded 1 so ctx.invoke(bootstrap,
..., max_workers=max_workers, ...) from bootstrap_parallel() is honored; ensure
the parameter name matches exactly (max_workers) and propagate it through any
intermediate calls or default handling in bootstrap to the Bootstrapper
instantiation.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@e2e/test_bootstrap_parallel_multiple_versions.sh`:
- Around line 70-110: The current test inspects "starting batch: phase=${phase}
items=N" which is logged before work begins and therefore doesn't prove
concurrency; change the check in the phase loop to detect actual overlapping
execution by parsing per-item start/finish logs with timestamps (or
worker/thread IDs) in bootstrap.log and asserting that for at least one phase an
item start timestamp occurs before another item's finish timestamp (or two
different worker IDs run concurrently) for that phase; update the grep/sed logic
that currently looks for "starting batch: phase=${phase} items=" and instead
search for per-item "start"/"finish" log lines (or worker-id tokens) and
implement an overlap detection routine to set PHASES_WITHOUT_PARALLEL_BATCH when
no overlap is found for a phase (keep using PARALLEL_PHASES and
PHASES_WITHOUT_PARALLEL_BATCH identifiers).

In `@src/fromager/bootstrapper.py`:
- Around line 716-720: The lock _wheel_dir_lock currently protects
update_wheel_mirror calls from _download_prebuilt() but not the identical
server.update_wheel_mirror(self.ctx) invocation inside _build_wheel(), leaving a
TOCTOU race with _find_cached_wheel; fix by acquiring the same _wheel_dir_lock
around the server.update_wheel_mirror(self.ctx) call in _build_wheel() (and any
other places that move files out of wheels_build) so that _download_prebuilt(),
_build_wheel(), and _find_cached_wheel() serialize on the same lock and
eliminate the race window.
- Around line 457-465: The ThreadPoolExecutor block with futures and run_one
allows started workers to perform irreversible writes even if one future fails;
modify run_one to avoid side effects (return intended mutations/data instead of
writing wheels/state), collect all_new_items from futures, then perform a single
commit/apply step that writes to disk only if every future succeeded;
alternatively, if run_one cannot be made pure, detect the phase and fall back to
sequential execution (no ThreadPoolExecutor) so writes are atomic per-item or
wrap mutations behind an explicit commit function (e.g., apply_batch_changes)
invoked only after all futures complete successfully.

In `@tests/test_bootstrapper.py`:
- Around line 614-662: The tests mock _dispatch_phase away so _phase_resolve
never runs (first test) and the second only exercises a single RESOLVE (no
siblings), so neither actually exercises parallel RESOLVE batching; update the
tests to let _phase_resolve run by removing or changing the _dispatch_phase
patch and instead arrange for a RESOLVE phase with multiple items (e.g., patch
_dispatch_phase to return a list containing two RESOLVE work items or call
bootstrap with two top-level Requirement objects) so bt._resolver.resolve is
invoked concurrently twice; for the test_mode failure case, likewise ensure two
sibling RESOLVE items are present so the first resolve raises (via the existing
mock_resolve) and the second succeeds, validating bt.failed_packages records the
failure while the sibling completes.

---

Outside diff comments:
In `@src/fromager/commands/bootstrap.py`:
- Around line 193-202: The bootstrap function is missing a max_workers parameter
and always constructs Bootstrapper with max_workers=1; update the bootstrap(...)
function signature to accept max_workers: int | None (default as needed) and
pass that value into the Bootstrapper constructor instead of the hardcoded 1 so
ctx.invoke(bootstrap, ..., max_workers=max_workers, ...) from
bootstrap_parallel() is honored; ensure the parameter name matches exactly
(max_workers) and propagate it through any intermediate calls or default
handling in bootstrap to the Bootstrapper instantiation.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 2d93f827-752d-448c-a6c8-055c4f8afa79

📥 Commits

Reviewing files that changed from the base of the PR and between 2132ef3 and 502ee11.

📒 Files selected for processing (7)

• e2e/ci_bootstrap_parallel_suite.sh
• e2e/test_bootstrap_parallel_multiple_versions.sh
• src/fromager/bootstrap_requirement_resolver.py
• src/fromager/bootstrapper.py
• src/fromager/commands/bootstrap.py
• tests/test_bootstrapper.py
• tests/test_bootstrapper_iterative.py

[dhellmann]

After experimenting further, this approach isn't going to work. It's too hard to combine a stack with a topology-aware prioritization system. I keep running into issues because we're marking something seen when we start it, even though it's not built.

I'm going to salvage an idea or two and experiment with other approaches.