Rewatch: replace wave scheduler with DAG + critical-path priority#8374
Rewatch: replace wave scheduler with DAG + critical-path priority#8374jfrolich wants to merge 2 commits intorescript-lang:masterfrom
Conversation
Instead of compiling modules in topologically-sorted waves (which stall on the slowest file per wave), schedule them from a priority queue on a work-stealing dispatcher. Priority is the length of the module's longest downstream dependency chain, so bottlenecks start first. Workers run inside a `rayon::in_place_scope` bounded to `rayon::current_num_threads()` so the heap ordering actually decides dispatch. Completions flow back over `std::sync::mpsc`; the dispatcher updates pending-dep counters, propagates the dirty flag, and pushes newly-ready modules back onto the heap. `BuildState` mutations happen after the scope exits so worker reads and main-thread writes stay disjoint. Cycle detection now triggers when the heap drains without completing the universe. Signed-Off-By: Jaap Frolich <jaap@tella.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
@codex review |
- Restore the `.unwrap()` on `package.namespace.to_suffix()` for mlmap
dispatch so a missing namespace suffix panics loudly instead of being
silently papered over.
- Drop the defensive `.unwrap_or_default()` on the post-completion
dependents lookup and the `if *count == 0 { continue }` guard on the
pending-deps decrement; both can only fire when the graph itself is
inconsistent, and hiding that would mask bugs. Align with the
unwrap-style assertions the old scheduler used.
- Sort `results_buffer` by module name before applying results so the
returned `compile_errors` / `compile_warnings` strings and the per-
package `.compiler.log` writes are deterministic across runs despite
non-deterministic completion order.
Signed-Off-By: Jaap Frolich <jaap@tella.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a22a9437d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if !is_clean { | ||
| dirty_set.insert(dep.clone()); | ||
| } |
There was a problem hiding this comment.
When a dependency finishes with !is_clean, this code only updates the local dirty_set; it does not immediately set module.compile_dirty on the dependent. Because the dispatcher stops spawning new work after the first compile error (has_errors), any dependents that were marked dirty but never scheduled in this run can remain compile_dirty = false in build_state, so a subsequent build may skip recompiling stale modules even though an upstream .cmi changed. The previous scheduler wrote compile_dirty = true directly during propagation, which avoided this incremental-build inconsistency after failed builds.
Useful? React with 👍 / 👎.
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
2 participants
Summary
par_iterper wave) with a work-stealing DAG dispatcher. Waves stall on the slowest file per round; DAG scheduling lets a module start as soon as its own dependencies finish.rewatch/src/build/compile.rs; no changes tobsc, the wire protocol, orrescript.jsonsemantics.Design
rayon::in_place_scope, bounded torayon::current_num_threads()concurrent tasks so theBinaryHeapordering actually decides dispatch (spawning everything would lose priority to rayon's deque).std::sync::mpsc. The dispatcher on the main thread:pending_depscounters for dependents,&BuildState; allBuildStatemutations (compile_state, timestamps,compile_dirty) happen after the scope exits, keeping worker reads and main-thread writes disjoint without any interior mutability.dependency_cycle::find/formatpath — same error message as before.Test plan
cargo build --manifest-path rewatch/Cargo.tomlcargo clippy --manifest-path rewatch/Cargo.toml --all-targets --all-features -- -D warningscargo fmt --check --manifest-path rewatch/Cargo.tomlcargo test --manifest-path rewatch/Cargo.toml— 68 unit tests passmake test-rewatch— full integration suite passes, including:09-dependency-cycle(cycle snapshot unchanged)13-no-infinite-loop-with-cycle16-snapshots-unchanged)Notes
clean_modules(which the old code built but never read) is gone.🤖 Generated with Claude Code