next.js

a98213cb - Refactor effects system: dedup/conflict detection, simplified Effect trait (#92300)

Refactor effects system: dedup/conflict detection, simplified Effect trait (#92300) ### What? Refactors the turbo-tasks effects system to support duplicate detection, conflict detection, and state tracking across effect applications. Also adds a ptr_eq fast path to `ReadRef::PartialEq`. ### Why? The existing effects system had several problems: - **No duplicate detection**: The same effect (e.g., writing the same content to the same path) would be re-applied every time, wasting I/O - **No conflict detection**: Two effects writing different content to the same path could cause an invalidation loop where the two writes invalidate each other, keeping a CPU busy-looping. On production builds this would crash with `Dependency tracking is disabled so invalidation is not allowed` - **Unnecessarily generic**: The old `DynEffect`→`DynEffectApplyFuture` chain wrapped errors in `Arc<dyn EffectError>`, plus a `DynEffectApplyFuture` chain, all to work around the fact that `anyhow::Error` doesn't implement `std::error::Error` - **Complex dedup state machine**: `EffectInstance` used a `Mutex<EffectState>` with 4 states (`NotStarted`/`Started`/`Finished`/`Invalid`), `Event`/`EventListener`, and `spawn()` to ensure idempotent application — all of which is now unnecessary since dedup is handled at the `Effects::apply` level - **Task-local context**: `ApplyEffectsContext` was a task-local typed map used only for a directory-creation cache, requiring `spawn()` + scope propagation - **Slow `ReadRef` equality**: `ReadRef::PartialEq` always performed deep content comparison even when both refs pointed to the same allocation ### How? #### Changed files | File | Change | |------|--------| | `turbo-tasks/src/effect.rs` | Rewrote `Effect` trait, `DynEffect`, `EffectStateStorage`, `Effects::apply()` | | `turbo-tasks/src/lib.rs` | Updated re-exports (`EffectError`, `EffectStateStorage`) | | `turbo-tasks/src/read_ref.rs` | Added `ptr_eq` fast path to `ReadRef::PartialEq` | | `turbo-tasks-fs/src/lib.rs` | Updated `WriteEffect`/`WriteLinkEffect` impls, restored `AnyhowWrapper`, removed `register_write_invalidator` | | `turbo-tasks-fs/src/invalidator_map.rs` | Simplified — removed `WriteContent` enum, invalidators now use `()` values | | `turbo-tasks-fs/src/invalidation.rs` | Deleted — `Write`/`WriteKind` invalidation reason types no longer needed | #### New `Effect` trait (in `turbo-tasks/src/effect.rs`) ```rust pub trait Effect: TraceRawVcs + NonLocalValue + Send + Sync + 'static { type Error: EffectError; type Value: Clone + DynPartialEq + Eq + Send + Sync + 'static; fn key(&self) -> Vec<u8>; fn value(&self) -> &Self::Value; fn state_storage(&self) -> &EffectStateStorage; fn apply(&self) -> impl Future<Output = Result<(), Self::Error>> + Send; } ``` The `Error` associated type uses `dyn std::error::Error` (via `EffectError`) rather than `anyhow::Error` to encourage structured error types that can be downcast into `Issue`s — particularly for filesystem errors from `turbo-tasks-fs`. The current fs implementations use `AnyhowWrapper` as a bridge. #### `EffectStateStorage` New struct stored on `DiskFileSystemInner`, using a two-level locking scheme for performance: - `DashMap<Vec<u8>, Arc<EffectStateEntry>>` tracks per-key state - Each `EffectStateEntry` has a sync `parking_lot::Mutex<Option<Box<dyn Any>>>` for the last applied value (fast dedup reads) and a `tokio::sync::Mutex<()>` write lock (serializes concurrent writes to the same key) - The sync fast path avoids `.await` entirely when the stored value matches, critical for high-iteration scenarios (e.g., the `writeToDisk` test that calls `Effects::apply` 10,000+ times per route) #### `Effects::apply()` flow 1. Group effects by `key()`, cache the `(index, Arc<EffectStateEntry>)` pairs in a `OnceLock` — computed once on first call, reused on subsequent calls with **no DashMap lookup** in the hot path 2. Detect duplicates (same key + same value → keep one) and conflicts (same key + different value → error) 3. For each unique effect: - **Sync fast path**: check `last_applied` via `parking_lot::Mutex` — return immediately if value unchanged - **Slow path**: acquire `tokio::sync::Mutex` write lock, re-check, clear `last_applied` to `None` (prevents stale fast-path matches during write), apply the effect, then store the new value #### Equality dispatch `eq_value_dyn` now calls `turbo_dyn_eq_hash::DynPartialEq::dyn_partial_eq` instead of a manual downcast, using the existing `turbo-dyn-eq-hash` crate machinery. #### `ReadRef::PartialEq` ptr_eq fast path ```rust fn eq(&self, other: &Self) -> bool { Self::ptr_eq(self, other) || Self::as_raw_ref(self).eq(Self::as_raw_ref(other)) } ``` Short-circuits deep comparison when both `ReadRef`s share the same `Arc` allocation — the common case in the effects dedup path where the stored and new `ReadRef` originate from the same turbo-tasks cell. #### `Effect::value()` returns `&Self::Value` Avoids cloning `ReadRef` (Arc increment) on every `eq_value_dyn` comparison. The reference goes straight to the stored field. Only `value_dyn()` (called when actually storing a new value after a write) clones. #### Simplified internals - `EffectInstance` reduced from `Mutex<EffectState>` state machine to plain `Box<dyn DynEffect>` - `DynEffect` trait gains `key()`, `eq_value_dyn()`, `value_dyn()`, `state_storage()` for object-safe dispatch; `dyn_apply()` converts `Self::Error` to `anyhow::Error` via the `Into` blanket impl - `Effects::apply()` now performs grouping, dedup, conflict detection, and per-key state-aware application #### Removed - `EffectState` enum (4 variants) + associated event machinery - `ApplyEffectsContext` + `APPLY_EFFECTS_CONTEXT` task-local - `DiskFileSystemApplyContext` struct - `register_write_invalidator` (complex write-invalidation tracking) - `WriteContent` enum from `InvalidatorMap` - `Write`/`WriteKind` invalidation reason types (deleted `invalidation.rs`) #### Kept from original design - `EffectError` trait + `AnyhowWrapper` in `turbo-tasks-fs` — preserved for future structured filesystem error types (to produce typed `Issue`s from write failures) - `invalidate_from_write` — simplified to just remove+fire path invalidators for read-task invalidation after writes - Directory creation cache — moved from task-local `ApplyEffectsContext` to `DashMap<PathBuf, ()>` on `DiskFileSystemInner` #### Test changes - `test_symlink_stress`: Updated to deduplicate random updates per symlink index before writing (the test previously relied on silent conflict resolution) - `is_sync_and_send` test: Relaxed to `is_send` since the new `Effects::apply()` future holds a `Pin<Box<dyn Future + Send>>` across await points (not `Sync`, but `Send` is sufficient) ### Testing - [x] `cargo check --workspace` — zero errors, zero warnings - [x] `cargo clippy -p turbo-tasks --all-targets -- -D warnings -A deprecated` — clean - [x] `ast-grep scan` — clean (all `Err(anyhow!())` replaced with `bail!()`) - [x] `cargo test -p turbo-tasks --lib` — 50 passed - [x] `cargo test -p turbo-tasks-fs --lib` — 110 passed - [x] `cargo test -p turbo-tasks-backend --lib` — 35 passed - [x] `cargo test -p turbopack-tests` — 89 passed <!-- NEXT_JS_LLM_PR --> --------- Co-authored-by: Tobias Koppers <sokra@users.noreply.github.com> Co-authored-by: Claude <noreply@anthropic.com>