next.js

dccb95c3 - Turbopack: Cache effect errors, replace the write_lock Mutex with a lazily created Notify instance (#93476)

Turbopack: Cache effect errors, replace the write_lock Mutex with a lazily created Notify instance (#93476) This is a mostly-by-hand refactor of some of the caching logic in https://github.com/vercel/next.js/pull/92300 - **Cache errors.** We often end up applying effects multiple times in frequent succession, and many filesystem effects already have their own backoff/retry loops, which can make them very slow when they fail, so it seems generally better to cache the failures. - **Add `EffectLastApplied` enum:** It's a matter of opinion, but I think the `EffectLastApplied` enum makes it easier to reason about the different states in the system. `Option` was difficult to reason about, and it would've been even more confiusing if it was an `Option<Result<(), Arc<dyn EffectError>>`. - `tokio::sync::Mutex<()>` is overkill here, `tokio::sync::Notify` is a simpler (and smaller) primitive that does the core of what we needed the `Mutex` for. We could also use `EventListener` here, but I didn't. (`EventListener` is really just smol's version of `Notify`) - We can lazily allocate the `Notify` instance inside of `last_applied`. We have to grab this lock anyways to check for the cached fast-path. If we're not in the fast-path, we can re-use the lock guard to create and store the `Notify` instance. - **Perf:** The old code's slow path had to always lock `last_applied` three times. Now, if there's no concurrent writer (common case), it only locks twice: Once before application, and once after. - **Perf:** Use a smallvec when computing unique indices to avoid a bunch of heap allocations. Unless there's a bug, we don't expect any of the keys to have more than one value (returns an error). - **Perf:** Store keys as `Box<[u8]>` instead of `Vec<u8>`, saving 8 bytes per key. Currently, if `effect.dyn_apply()` panics, we just do our best to mark the effect as unapplied to avoid a deadlock. This could alternatively poison the object. ## Memory Usage Memory usage can increase if we're storing errors (heap allocated, stored in `Arc`) we previously wouldn't have, but if we assume that errors are uncommon, this refactor decreases the size of the data structures involved. Before this change, eagerly allocating a `tokio::sync::Mutex` was costing 40 bytes per effect, making the entire `EffectEntryState` struct 64 bytes: <img src="https://app.graphite.com/user-attachments/assets/989ac229-9e2f-4370-8e32-b14e10e09248.png" width=500> <img src="https://app.graphite.com/user-attachments/assets/f2e14416-17c3-4961-8c00-fa7a81846fa0.png" width=500> After the changes, the entire `EffectLastApplied` enum is 40 bytes <img src="https://app.graphite.com/user-attachments/assets/18e4fc0a-e7f8-4fad-b9bc-c22ba3eefb6c.png" width=500> After wrapping the `EffectLastApplied` enum in a `parking_lot::Mutex`, it comes in at 48 bytes (after alignment), 16 bytes smaller than the old struct.