Avoid recomputing all_assets_map on every change (#2464)

* Avoid recomputing all_assets_map on every change

* fix snapshot test and update snapshot

Co-authored-by: Tobias Koppers <tobias.koppers@googlemail.com>

crates/turbopack-ecmascript/src/chunk/source_map.rs

@@ -1,9 +1,10 @@
 use anyhow::Result;
 use turbo_tasks::{primitives::StringVc, ValueToString, ValueToStringVc};
 use turbo_tasks_fs::{File, FileSystemPathVc};
-use turbo_tasks_hash::encode_hex;
+use turbo_tasks_hash::{encode_hex, Xxh3Hash64Hasher};
 use turbopack_core::{
     asset::{Asset, AssetContentVc, AssetVc},
+    chunk::ModuleIdVc,
     code_builder::CodeVc,
     reference::{AssetReference, AssetReferenceVc, AssetReferencesVc},
     resolve::{ResolveResult, ResolveResultVc},
@@ -29,10 +30,9 @@ impl EcmascriptChunkSourceMapAssetVc {
 impl Asset for EcmascriptChunkSourceMapAsset {
     #[turbo_tasks::function]
     async fn path(&self) -> Result<FileSystemPathVc> {
-        Ok(self.chunk.path().append(&format!(
-            ".{}.map",
-            self.chunk.versioned_content().version().id().await?
-        )))
+        // NOTE(alexkirsz) We used to include the chunk's version id in the path,
+        // but this caused `all_assets_map` to be recomputed on every change.
+        Ok(self.chunk.path().append(".map"))
     }
 
     #[turbo_tasks::function]
@@ -51,17 +51,17 @@ impl Asset for EcmascriptChunkSourceMapAsset {
 #[turbo_tasks::value]
 pub struct EcmascriptChunkEntrySourceMapAsset {
     chunk_path: FileSystemPathVc,
-    hash: u64,
+    id: ModuleIdVc,
     code: CodeVc,
 }
 
 #[turbo_tasks::value_impl]
 impl EcmascriptChunkEntrySourceMapAssetVc {
     #[turbo_tasks::function]
-    pub fn new(chunk_path: FileSystemPathVc, hash: u64, code: CodeVc) -> Self {
+    pub fn new(chunk_path: FileSystemPathVc, id: ModuleIdVc, code: CodeVc) -> Self {
         EcmascriptChunkEntrySourceMapAsset {
             chunk_path,
-            hash,
+            id,
             code,
         }
         .cell()
@@ -71,10 +71,14 @@ impl EcmascriptChunkEntrySourceMapAssetVc {
 #[turbo_tasks::value_impl]
 impl Asset for EcmascriptChunkEntrySourceMapAsset {
     #[turbo_tasks::function]
-    fn path(&self) -> FileSystemPathVc {
-        let hash = encode_hex(self.hash);
+    async fn path(&self) -> Result<FileSystemPathVc> {
+        // NOTE(alexkirsz) We used to asset's hash in the path, but this caused
+        // `all_assets_map` to be recomputed on every change.
+        let mut hasher = Xxh3Hash64Hasher::new();
+        hasher.write_value(self.id.await?);
+        let hash = encode_hex(hasher.finish());
         let truncated_hash = &hash[..6];
-        self.chunk_path.append(&format!(".{}.map", truncated_hash))
+        Ok(self.chunk_path.append(&format!(".{}.map", truncated_hash)))
     }
 
     #[turbo_tasks::function]
@@ -131,7 +135,7 @@ impl AssetReference for EcmascriptChunkSourceMapAssetReference {
                     EcmascriptChunkEntrySourceMapAsset {
                         chunk_path: path,
                         code: item.code_vc,
-                        hash: item.hash,
+                        id: item.chunk_item.id(),
                     }
                     .keyed_cell(EcmascriptChunkEntrySourceMapAssetCellKey(item.chunk_item))
                     .into(),

crates/turbopack-tests/tests/snapshot.rs

@@ -242,39 +242,12 @@ fn remove_file(root: &str, path: &str) -> Result<()> {
     Ok(())
 }
 
-/// Removes annoying hash fingerprints from files paths.
-///
-/// If the hash changes whenever the contents of the file changes, then git will
-/// show it as a brand new file instead of a diff of an exsting file. This makes
-/// reviewing changes extremely difficult.
-async fn remove_hash_fingerprint(asset: AssetVc) -> Result<FileSystemPathVc> {
-    let path = asset.path();
-
-    if EcmascriptChunkSourceMapAssetVc::resolve_from(asset)
-        .await?
-        .is_some()
-    {
-        // .map files have a hash like foo.js.abc123.map.
-        let mut p = PathBuf::from(&path.await?.path);
-        if p.extension() == Some(OsStr::new("map")) {
-            p.set_extension("");
-            debug_assert_ne!(p.extension(), Some(OsStr::new("js")));
-            p.set_extension("abc123.map");
-        }
-
-        return Ok(path
-            .root()
-            .join(p.to_str().context("path is expected to be normal")?));
-    }
-    Ok(path)
-}
-
 async fn walk_asset(
     asset: AssetVc,
     seen: &mut HashSet<String>,
     queue: &mut VecDeque<AssetVc>,
 ) -> Result<()> {
-    let path = remove_hash_fingerprint(asset).await?;
+    let path = asset.path();
     let path_str = path.await?.path.clone();
 
     if !seen.insert(path_str.to_string()) {