next.js
fb0ce72d - [turbopack] Simplify the definition of AssetIdent (#79805)

Commit
329 days ago
[turbopack] Simplify the definition of AssetIdent (#79805) Replace the `ResolvedVc<RcStr>` uses inside of `AssetIdent` with `RcStr` directly ## Why This simplifies construction and usage and provides more reasonably equality semantics. In principle, storing `ResolvedVc` would enable finer grained invalidations. e.g. if only the `query` changed and a task only depended upon the `modifiers` then that task wouldn't get invalidated. However, the entire `Assetident` is often consumed monolithically already. For example, it is when producing output chunks and constructing the modulegraph (via the `to_string` function). So modeling it more monolitically seems appropriate. Additionally, by accepting `RcStr` values as _values_ instead of `cells` we clarify equality semantics. So we should expect to get _more_ cache hits from `AssetIdent::new`. In principle this may make equality slower, but it is very rare that any of the RcStr instances involved are more than a handful of bytes, so this shouldn't be much of a concern. This makes the type slightly smaller, now it is `120 bytes` instead of `144 bytes`, which should help with performance a bit since there are many thousands of these in a build. ## What This was mostly done by just replacing the types and chasing build errors backwards to remove the `ResolvedVc` values at the source. This allowed me to replace many tiny turbotask functions with the `rcstr!` macro and remove many calls to `cell`. ## Performance ![image.png](https://graphite-user-uploaded-assets-prod.s3.amazonaws.com/AwJ29EfoPcPdLSwCZxAz/70962bb3-5a5a-4762-9580-a38141f84c1c.png) Testing `vercel-site` reveals a small improvement in MaxRSS which is expected due to 1. eliminating a bunch of turbotask cells, and 2 reducing the size of `AssetIdent`
Author
Parents
Loading