next.js

10e70b61 - Turbopack: Implement `TaskInput` for `ReadRef`, use a `ReadRef` input for `AssetIdent::new` (#83356)

Turbopack: Implement `TaskInput` for `ReadRef`, use a `ReadRef` input for `AssetIdent::new` (#83356) @lukesandberg and I noticed this opportunity. ### Hypothesis `AssetIdent::new` is a dummy constructor function used for caching value with unique `Vc`s. It is called frequently. The `AssetIdent` struct is large and contains three different `Vec`s, so it's likely non-trivial to clone. - When an uncached turbo task function is called, its argument is boxed and cloned once to be used as cache key. There's two copies of the argument at this point: A cache key, and a local copy for the about-to-be-executed function. - This function immediately constructs a cell, which moves its local copy of the argument into a `triomphe::Arc`, preserving it. So we end up with two persistent copies of the task input. This doesn't matter when the struct is small and cheap to clone, but it does matter here. The solution is to wrap `AssetIdent` in a `ReadRef` before passing it to a `turbo_tasks::function`. `ReadRef` does not impact equality (defers to inner value), it's cheap to clone, and `ReadRef::cell` provides a fast-path that re-uses the inner `triomphe::Arc` for the newly constructed cell: https://github.com/vercel/next.js/blob/5cf762235a1e9f02443f45a69a7afd1880317f77/turbopack/crates/turbo-tasks/src/read_ref.rs#L247-L262 ### Tradeoffs If there's a cache hit, this approach constructs and throws away a temporary `Arc`, that the original implementation wouldn't. So in cases where there's a very high cache hit rate, or where the `Clone` would've been very cheap, this may come with a CPU time tradeoff. That doesn't seem to be the case here (cache hits are about 30%). ### Rough Benchmark Does not save a measurable amount of memory ``` cargo run --release -p next-build-test -- generate ~/shadcn-ui/apps/v4/ > ~/shadcn-ui/apps/v4/project_options.json cd ~/shadcn-ui/apps/v4/ pnpm i command time -v ~/next.js/target/release/next-build-test run sequential 1 1 '/sink' ``` maxrss (kbytes) before (5 runs): ``` 2355284 2325432 2320960 2281920 2385740 ``` maxrss (kbytes) after (5 runs): ``` 2361084 2323468 2364040 2353300 2388904 ``` This shows it's slightly worse, but only 1%, so it's easily within margin-of-error: <img src="https://app.graphite.dev/user-attachments/assets/a1a7e1c8-bc05-4262-91d8-c3590f5a1253.png" width=500>