Add another Cache for accessing files in ArtifactBundles

In a specific `stresstest` using a JS event that has a particularly large sourcemap, this leads to a speedup of up to 1_000x

Cargo.toml

@@ -1,7 +1,12 @@
 [workspace]
 resolver = "2"
 members = ["crates/*"]
-default-members = ["crates/symbolicator", "crates/symbolicli", "crates/symsorter", "crates/wasm-split"]
+default-members = [
+    "crates/symbolicator",
+    "crates/symbolicli",
+    "crates/symsorter",
+    "crates/wasm-split",
+]
 
 [profile.dev]
 # For debug builds, we do want to optimize for both debug-ability, and fast iteration times.
@@ -47,4 +52,4 @@ cpp_demangle = { git = "https://github.com/getsentry/cpp_demangle", branch = "se
 # symbolic = { path = "../../../symbolic/symbolic", features = ["common-serde", "debuginfo", "demangle", "minidump-serde", "symcache"] }
 # See also https://doc.rust-lang.org/cargo/reference/overriding-dependencies.html
 # [patch.crates-io]
-# symbolic = { path = "../symbolic/symbolic" }
+# symbolic-debuginfo = { path = "../symbolic/symbolic-debuginfo" }

crates/symbolicator-js/src/lookup.rs

@@ -28,7 +28,7 @@ use std::fmt::{self, Write};
 use std::fs::File;
 use std::io::{self, BufWriter};
 use std::sync::Arc;
-use std::time::SystemTime;
+use std::time::{Duration, SystemTime};
 
 use futures::future::BoxFuture;
 use reqwest::Url;
@@ -163,6 +163,7 @@ impl SourceMapLookup {
     pub async fn new(service: SourceMapService, request: SymbolicateJsStacktraces) -> Self {
         let SourceMapService {
             objects,
+            files_in_bundles,
             sourcefiles_cache,
             bundle_index_cache,
             sourcemap_caches,
@@ -213,6 +214,7 @@ impl SourceMapLookup {
 
         let fetcher = ArtifactFetcher {
             objects,
+            files_in_bundles,
             sourcefiles_cache,
             sourcemap_caches,
             api_lookup,
@@ -555,10 +557,67 @@ struct IndividualArtifact {
     resolved_with: ResolvedWith,
 }
 
+type FileInBundleCacheInner = moka::sync::Cache<(RemoteFileUri, FileKey), CachedFileEntry>;
+
+#[derive(Clone)]
+pub struct FileInBundleCache {
+    cache: FileInBundleCacheInner,
+}
+
+impl std::fmt::Debug for FileInBundleCache {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.debug_struct("FileInBundleCache").finish()
+    }
+}
+
+impl FileInBundleCache {
+    pub fn new() -> Self {
+        const GIGS: u64 = 1 << 30;
+        let cache = FileInBundleCacheInner::builder()
+            .max_capacity(2 * GIGS)
+            .time_to_idle(Duration::from_secs(60 * 60))
+            .name("file-in-bundle")
+            .weigher(|_k, v| {
+                let content_size = v
+                    .entry
+                    .as_ref()
+                    .map(|cached_file| cached_file.contents.len())
+                    .unwrap_or_default();
+                (std::mem::size_of_val(v) + content_size)
+                    .try_into()
+                    .unwrap_or(u32::MAX)
+            })
+            .build();
+        Self { cache }
+    }
+
+    fn try_get(
+        &self,
+        bundle_uris: impl Iterator<Item = RemoteFileUri>,
+        mut key: FileKey,
+    ) -> Option<CachedFileEntry> {
+        for bundle_uri in bundle_uris {
+            // XXX: this is a really horrible workaround for not being able to look up things via `(&A, &B)` instead of `&(A, B)`.
+            let lookup_key = (bundle_uri, key);
+            if let Some(file_entry) = self.cache.get(&lookup_key) {
+                return Some(file_entry);
+            }
+            key = lookup_key.1;
+        }
+        None
+    }
+
+    fn insert(&self, bundle_uri: &RemoteFileUri, key: &FileKey, file_entry: &CachedFileEntry) {
+        let key = (bundle_uri.clone(), key.clone());
+        self.cache.insert(key, file_entry.clone())
+    }
+}
+
 struct ArtifactFetcher {
     metrics: JsMetrics,
 
     // other services:
+    files_in_bundles: FileInBundleCache,
     objects: ObjectsActor,
     sourcefiles_cache: Arc<SourceFilesCache>,
     sourcemap_caches: Arc<Cacher<FetchSourceMapCacheInternal>>,
@@ -973,6 +1032,13 @@ impl ArtifactFetcher {
             return None;
         }
 
+        if let Some(file_entry) = self
+            .files_in_bundles
+            .try_get(self.artifact_bundles.keys().rev().cloned(), key.clone())
+        {
+            return Some(file_entry);
+        }
+
         // If we have a `DebugId`, we try a lookup based on that.
         if let Some(debug_id) = key.debug_id() {
             let ty = key.as_type();
@@ -988,11 +1054,13 @@ impl ArtifactFetcher {
                         *bundle_resolved_with,
                     );
                     tracing::trace!(?key, "Found file in artifact bundles by debug-id");
-                    return Some(CachedFileEntry {
+                    let file_entry = CachedFileEntry {
                         uri: CachedFileUri::Bundled(bundle_uri.clone(), key.clone()),
                         entry: CachedFile::from_descriptor(key.abs_path(), descriptor),
                         resolved_with: ResolvedWith::DebugId,
-                    });
+                    };
+                    self.files_in_bundles.insert(bundle_uri, key, &file_entry);
+                    return Some(file_entry);
                 }
             }
         }
@@ -1012,11 +1080,13 @@ impl ArtifactFetcher {
                             *resolved_with,
                         );
                         tracing::trace!(?key, url, "Found file in artifact bundles by url");
-                        return Some(CachedFileEntry {
+                        let file_entry = CachedFileEntry {
                             uri: CachedFileUri::Bundled(bundle_uri.clone(), key.clone()),
                             entry: CachedFile::from_descriptor(Some(abs_path), descriptor),
                             resolved_with: *resolved_with,
-                        });
+                        };
+                        self.files_in_bundles.insert(bundle_uri, key, &file_entry);
+                        return Some(file_entry);
                     }
                 }
             }

crates/symbolicator-js/src/service.rs

@@ -10,11 +10,12 @@ use symbolicator_service::services::SharedServices;
 
 use crate::api_lookup::SentryLookupApi;
 use crate::bundle_index_cache::BundleIndexCache;
-use crate::lookup::FetchSourceMapCacheInternal;
+use crate::lookup::{FetchSourceMapCacheInternal, FileInBundleCache};
 
 #[derive(Debug, Clone)]
 pub struct SourceMapService {
     pub(crate) objects: ObjectsActor,
+    pub(crate) files_in_bundles: FileInBundleCache,
     pub(crate) sourcefiles_cache: Arc<SourceFilesCache>,
     pub(crate) bundle_index_cache: Arc<BundleIndexCache>,
     pub(crate) sourcemap_caches: Arc<Cacher<FetchSourceMapCacheInternal>>,
@@ -44,8 +45,11 @@ impl SourceMapService {
             in_memory,
         ));
 
+        let files_in_bundles = FileInBundleCache::new();
+
         Self {
             objects,
+            files_in_bundles,
             sourcefiles_cache,
             bundle_index_cache: Arc::new(bundle_index_cache),
             sourcemap_caches: Arc::new(Cacher::new(caches.sourcemap_caches.clone(), shared_cache)),