Skip to content

Commit

Permalink
Do not persist SourceMap contents in FileInBundleCache (#1359)
Browse files Browse the repository at this point in the history
According to metrics, the cache is quite effective in ST and on LPQ instances. Less so in production, most likely because the cache is too small.

To increase the effective size of the cache, we do not store the actual sourcemap contents in it anymore. These contents are only used for sourcemapcache generation, and are not usually required.
  • Loading branch information
Swatinem authored Feb 7, 2024
1 parent e7c0c7a commit 11d7533
Show file tree
Hide file tree
Showing 6 changed files with 238 additions and 92 deletions.
32 changes: 25 additions & 7 deletions crates/symbolicator-js/src/bundle_lookup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@ use std::time::Duration;

use symbolicator_sources::RemoteFileUri;

use crate::interface::ResolvedWith;
use crate::lookup::{CachedFileEntry, FileKey};

type FileInBundleCacheInner = moka::sync::Cache<(RemoteFileUri, FileKey), CachedFileEntry>;
type FileInBundleCacheInner =
moka::sync::Cache<(RemoteFileUri, FileKey), (CachedFileEntry, ResolvedWith)>;

/// A cache that memoizes looking up files in artifact bundles.
#[derive(Clone)]
Expand Down Expand Up @@ -33,9 +35,10 @@ impl FileInBundleCache {
.name("file-in-bundle")
.weigher(|_k, v| {
let content_size = v
.0
.entry
.as_ref()
.map(|cached_file| cached_file.contents.len())
.map(|cached_file| cached_file.contents.as_deref().unwrap_or_default().len())
.unwrap_or_default();
(std::mem::size_of_val(v) + content_size)
.try_into()
Expand All @@ -54,12 +57,12 @@ impl FileInBundleCache {
&self,
bundle_uris: impl Iterator<Item = RemoteFileUri>,
mut key: FileKey,
) -> Option<CachedFileEntry> {
) -> Option<(RemoteFileUri, CachedFileEntry, ResolvedWith)> {
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);
if let Some((file_entry, resolved_with)) = self.cache.get(&lookup_key) {
return Some((lookup_key.0, file_entry, resolved_with));
}
key = lookup_key.1;
}
Expand All @@ -70,8 +73,23 @@ impl FileInBundleCache {
///
/// Files are inserted under a specific bundle so that e.g. files with the same
/// `abs_path` belonging to different events are disambiguated.
pub fn insert(&self, bundle_uri: &RemoteFileUri, key: &FileKey, file_entry: &CachedFileEntry) {
pub fn insert(
&self,
bundle_uri: &RemoteFileUri,
key: &FileKey,
resolved_with: ResolvedWith,
file_entry: &CachedFileEntry,
) {
let mut file_entry = file_entry.clone();
if matches!(key, FileKey::SourceMap { .. }) {
if let Ok(cached_file) = file_entry.entry.as_mut() {
// SourceMaps are usually very large, and we only need them for
// `sourcemapcache` creation. So do not persist the actual contents here.
// Rather load them later in `sourcemapcache` generation if needed.
cached_file.contents = None;
}
}
let key = (bundle_uri.clone(), key.clone());
self.cache.insert(key, file_entry.clone())
self.cache.insert(key, (file_entry, resolved_with))
}
}
1 change: 1 addition & 0 deletions crates/symbolicator-js/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ pub mod interface;
mod lookup;
mod metrics;
mod service;
mod sourcemap_cache;
mod symbolication;
mod utils;

Expand Down
Loading

0 comments on commit 11d7533

Please sign in to comment.