ref: Use a OnceCell instead of async RwLock in Shared Cache (#984)

This avoids some internal indirection and locking, and makes a couple of
functions non-async, such as the service startup path.

crates/symbolicator-service/src/services/bitcode.rs

@@ -23,7 +23,7 @@ use crate::types::Scope;
 use crate::utils::futures::{m, measure};
 
 use super::fetch_file;
-use super::shared_cache::SharedCacheService;
+use super::shared_cache::SharedCacheRef;
 
 /// Handle to a valid BCSymbolMap.
 ///
@@ -163,11 +163,11 @@ pub struct BitcodeService {
 impl BitcodeService {
     pub fn new(
         difs_cache: Cache,
-        shared_cache_svc: Arc<SharedCacheService>,
+        shared_cache: SharedCacheRef,
         download_svc: Arc<DownloadService>,
     ) -> Self {
         Self {
-            cache: Arc::new(Cacher::new(difs_cache, shared_cache_svc)),
+            cache: Arc::new(Cacher::new(difs_cache, shared_cache)),
             download_svc,
         }
     }

crates/symbolicator-service/src/services/cacher.rs

@@ -15,7 +15,7 @@ use tempfile::NamedTempFile;
 use tokio::fs;
 
 use crate::cache::{Cache, CacheEntry, CacheError, ExpirationTime};
-use crate::services::shared_cache::{CacheStoreReason, SharedCacheKey, SharedCacheService};
+use crate::services::shared_cache::{CacheStoreReason, SharedCacheKey, SharedCacheRef};
 use crate::types::Scope;
 use crate::utils::futures::CallOnDrop;
 
@@ -41,7 +41,7 @@ pub struct Cacher<T: CacheItemRequest> {
     current_computations: ComputationMap<T::Item>,
 
     /// A service used to communicate with the shared cache.
-    shared_cache_service: Arc<SharedCacheService>,
+    shared_cache: SharedCacheRef,
 }
 
 impl<T: CacheItemRequest> Clone for Cacher<T> {
@@ -50,16 +50,16 @@ impl<T: CacheItemRequest> Clone for Cacher<T> {
         Cacher {
             config: self.config.clone(),
             current_computations: self.current_computations.clone(),
-            shared_cache_service: Arc::clone(&self.shared_cache_service),
+            shared_cache: Arc::clone(&self.shared_cache),
         }
     }
 }
 
 impl<T: CacheItemRequest> Cacher<T> {
-    pub fn new(config: Cache, shared_cache_service: Arc<SharedCacheService>) -> Self {
+    pub fn new(config: Cache, shared_cache: SharedCacheRef) -> Self {
         Cacher {
             config,
-            shared_cache_service,
+            shared_cache,
             current_computations: Arc::new(Mutex::new(BTreeMap::new())),
         }
     }
@@ -199,22 +199,22 @@ impl<T: CacheItemRequest> Cacher<T> {
         // - we refreshed the local cache time, so we also refresh the shared cache time.
         let needs_reupload = expiration.was_touched();
         if entry.is_ok() && version == T::VERSIONS.current && needs_reupload {
-            let shared_cache_key = SharedCacheKey {
-                name: self.config.name(),
-                version: T::VERSIONS.current,
-                local_key: key.clone(),
-            };
-            match fs::File::open(&item_path)
-                .await
-                .context("Local cache path not available for shared cache")
-            {
-                Ok(fd) => {
-                    self.shared_cache_service
-                        .store(shared_cache_key, fd, CacheStoreReason::Refresh)
-                        .await;
-                }
-                Err(err) => {
-                    sentry::capture_error(&*err);
+            if let Some(shared_cache) = self.shared_cache.get() {
+                let shared_cache_key = SharedCacheKey {
+                    name: self.config.name(),
+                    version: T::VERSIONS.current,
+                    local_key: key.clone(),
+                };
+                match fs::File::open(&item_path)
+                    .await
+                    .context("Local cache path not available for shared cache")
+                {
+                    Ok(fd) => {
+                        shared_cache.store(shared_cache_key, fd, CacheStoreReason::Refresh);
+                    }
+                    Err(err) => {
+                        sentry::capture_error(&*err);
+                    }
                 }
             }
         }
@@ -260,10 +260,11 @@ impl<T: CacheItemRequest> Cacher<T> {
         };
 
         let temp_fd = tokio::fs::File::from_std(temp_file.reopen()?);
-        let shared_cache_hit = self
-            .shared_cache_service
-            .fetch(&shared_cache_key, temp_fd)
-            .await;
+        let shared_cache_hit = if let Some(shared_cache) = self.shared_cache.get() {
+            shared_cache.fetch(&shared_cache_key, temp_fd).await
+        } else {
+            false
+        };
 
         let mut entry = Err(CacheError::NotFound);
         if shared_cache_hit {
@@ -337,13 +338,13 @@ impl<T: CacheItemRequest> Cacher<T> {
         // TODO: Not handling negative caches probably has a huge perf impact.  Need to
         // figure out negative caches.  Maybe put them in redis with a TTL?
         if !shared_cache_hit && entry.is_ok() {
-            self.shared_cache_service
-                .store(
+            if let Some(shared_cache) = self.shared_cache.get() {
+                shared_cache.store(
                     shared_cache_key,
                     tokio::fs::File::from_std(file),
                     CacheStoreReason::New,
-                )
-                .await;
+                );
+            }
         }
 
         // we just created a fresh cache, so use the initial expiration times
@@ -661,9 +662,7 @@ mod tests {
             Arc::new(AtomicIsize::new(1)),
         )
         .unwrap();
-        let runtime = tokio::runtime::Handle::current();
-        let shared_cache = Arc::new(SharedCacheService::new(None, runtime.clone()).await);
-        let cacher = Cacher::new(cache, shared_cache);
+        let cacher = Cacher::new(cache, Default::default());
 
         let request = TestCacheItem::new("some_cache_key");
 
@@ -705,9 +704,7 @@ mod tests {
             Arc::new(AtomicIsize::new(1)),
         )
         .unwrap();
-        let runtime = tokio::runtime::Handle::current();
-        let shared_cache = Arc::new(SharedCacheService::new(None, runtime.clone()).await);
-        let cacher = Cacher::new(cache, shared_cache);
+        let cacher = Cacher::new(cache, Default::default());
 
         let request = TestCacheItem::new("some_cache_key");
 
@@ -741,9 +738,7 @@ mod tests {
             Arc::new(AtomicIsize::new(1)),
         )
         .unwrap();
-        let runtime = tokio::runtime::Handle::current();
-        let shared_cache = Arc::new(SharedCacheService::new(None, runtime.clone()).await);
-        let cacher = Cacher::new(cache, shared_cache);
+        let cacher = Cacher::new(cache, Default::default());
 
         let request = TestCacheItem::new("0");
 

crates/symbolicator-service/src/services/cficaches.rs

@@ -21,7 +21,7 @@ use crate::utils::futures::{m, measure};
 use crate::utils::sentry::ConfigureScope;
 
 use super::derived::{derive_from_object_handle, DerivedCache};
-use super::shared_cache::SharedCacheService;
+use super::shared_cache::SharedCacheRef;
 
 /// The supported cficache versions.
 ///
@@ -74,13 +74,9 @@ pub struct CfiCacheActor {
 }
 
 impl CfiCacheActor {
-    pub fn new(
-        cache: Cache,
-        shared_cache_svc: Arc<SharedCacheService>,
-        objects: ObjectsActor,
-    ) -> Self {
+    pub fn new(cache: Cache, shared_cache: SharedCacheRef, objects: ObjectsActor) -> Self {
         CfiCacheActor {
-            cficaches: Arc::new(Cacher::new(cache, shared_cache_svc)),
+            cficaches: Arc::new(Cacher::new(cache, shared_cache)),
             objects,
         }
     }

crates/symbolicator-service/src/services/il2cpp.rs

@@ -21,7 +21,7 @@ use crate::types::Scope;
 use crate::utils::futures::{m, measure};
 
 use super::fetch_file;
-use super::shared_cache::SharedCacheService;
+use super::shared_cache::SharedCacheRef;
 
 /// Handle to a valid [`LineMapping`].
 ///
@@ -114,11 +114,11 @@ pub struct Il2cppService {
 impl Il2cppService {
     pub fn new(
         difs_cache: Cache,
-        shared_cache_svc: Arc<SharedCacheService>,
+        shared_cache: SharedCacheRef,
         download_svc: Arc<DownloadService>,
     ) -> Self {
         Self {
-            cache: Arc::new(Cacher::new(difs_cache, shared_cache_svc)),
+            cache: Arc::new(Cacher::new(difs_cache, shared_cache)),
             download_svc,
         }
     }

crates/symbolicator-service/src/services/mod.rs

@@ -9,8 +9,6 @@
 //! The internal services require a separate asynchronous runtimes dedicated for I/O-intensive work,
 //! such as downloads and access to the shared cache.
 
-use std::sync::Arc;
-
 use anyhow::{Context, Result};
 
 use crate::cache::Caches;
@@ -41,7 +39,7 @@ use self::symbolication::SymbolicationActor;
 use self::symcaches::SymCacheActor;
 pub use fetch_file::fetch_file;
 
-pub async fn create_service(
+pub fn create_service(
     config: &Config,
     io_pool: tokio::runtime::Handle,
 ) -> Result<(SymbolicationActor, ObjectsActor)> {
@@ -52,8 +50,7 @@ pub async fn create_service(
 
     let downloader = DownloadService::new(config, io_pool.clone());
 
-    let shared_cache = SharedCacheService::new(config.shared_cache.clone(), io_pool.clone()).await;
-    let shared_cache = Arc::new(shared_cache);
+    let shared_cache = SharedCacheService::new(config.shared_cache.clone(), io_pool);
 
     let objects = ObjectsActor::new(
         caches.object_meta,

crates/symbolicator-service/src/services/objects/data_cache.rs

@@ -262,7 +262,6 @@ mod tests {
     use crate::services::download::DownloadService;
     use crate::services::objects::data_cache::Scope;
     use crate::services::objects::{FindObject, ObjectPurpose, ObjectsActor};
-    use crate::services::shared_cache::SharedCacheService;
     use crate::test::{self, tempdir};
 
     use symbolic::common::DebugId;
@@ -293,10 +292,8 @@ mod tests {
             ..Config::default()
         };
 
-        let runtime = tokio::runtime::Handle::current();
-        let download_svc = DownloadService::new(&config, runtime.clone());
-        let shared_cache_svc = Arc::new(SharedCacheService::new(None, runtime).await);
-        ObjectsActor::new(meta_cache, data_cache, shared_cache_svc, download_svc)
+        let download_svc = DownloadService::new(&config, tokio::runtime::Handle::current());
+        ObjectsActor::new(meta_cache, data_cache, Default::default(), download_svc)
     }
 
     #[tokio::test]

crates/symbolicator-service/src/services/objects/mod.rs

@@ -17,7 +17,7 @@ use meta_cache::FetchFileMetaRequest;
 pub use data_cache::ObjectHandle;
 pub use meta_cache::ObjectMetaHandle;
 
-use super::shared_cache::SharedCacheService;
+use super::shared_cache::SharedCacheRef;
 
 mod data_cache;
 mod meta_cache;
@@ -86,12 +86,12 @@ impl ObjectsActor {
     pub fn new(
         meta_cache: Cache,
         data_cache: Cache,
-        shared_cache_svc: Arc<SharedCacheService>,
+        shared_cache: SharedCacheRef,
         download_svc: Arc<DownloadService>,
     ) -> Self {
         ObjectsActor {
-            meta_cache: Arc::new(Cacher::new(meta_cache, Arc::clone(&shared_cache_svc))),
-            data_cache: Arc::new(Cacher::new(data_cache, shared_cache_svc)),
+            meta_cache: Arc::new(Cacher::new(meta_cache, Arc::clone(&shared_cache))),
+            data_cache: Arc::new(Cacher::new(data_cache, shared_cache)),
             download_svc,
         }
     }

crates/symbolicator-service/src/services/ppdb_caches.rs

@@ -19,7 +19,7 @@ use crate::utils::sentry::ConfigureScope;
 use super::cacher::{CacheItemRequest, CacheKey, CacheVersions, Cacher};
 use super::derived::{derive_from_object_handle, DerivedCache};
 use super::objects::{FindObject, ObjectHandle, ObjectMetaHandle, ObjectPurpose, ObjectsActor};
-use super::shared_cache::SharedCacheService;
+use super::shared_cache::SharedCacheRef;
 
 /// The supported ppdb_cache versions.
 ///
@@ -64,13 +64,9 @@ pub struct PortablePdbCacheActor {
 }
 
 impl PortablePdbCacheActor {
-    pub fn new(
-        cache: Cache,
-        shared_cache_svc: Arc<SharedCacheService>,
-        objects: ObjectsActor,
-    ) -> Self {
+    pub fn new(cache: Cache, shared_cache: SharedCacheRef, objects: ObjectsActor) -> Self {
         Self {
-            ppdb_caches: Arc::new(Cacher::new(cache, shared_cache_svc)),
+            ppdb_caches: Arc::new(Cacher::new(cache, shared_cache)),
             objects,
         }
     }