From d85cbb303805d261fd4e0bebf6596756087e486f Mon Sep 17 00:00:00 2001 From: Iris Shi <0.0@owo.li> Date: Thu, 19 Dec 2024 12:15:35 +0800 Subject: [PATCH] various refactors --- mercury/src/internal/pack/cache_object.rs | 44 +++++++++++------------ mercury/src/utils/storable.rs | 28 +++++++++------ 2 files changed, 39 insertions(+), 33 deletions(-) diff --git a/mercury/src/internal/pack/cache_object.rs b/mercury/src/internal/pack/cache_object.rs index beee61fd..afac4246 100644 --- a/mercury/src/internal/pack/cache_object.rs +++ b/mercury/src/internal/pack/cache_object.rs @@ -7,9 +7,9 @@ use serde::{Deserialize, Serialize}; use threadpool::ThreadPool; use crate::hash::SHA1; +use crate::internal::object::types::ObjectType; use crate::internal::pack::entry::Entry; use crate::internal::pack::utils; -use crate::internal::object::types::ObjectType; use crate::utils::storable::{DefaultFileStorageStrategy, Storable}; // /// record heap-size of all CacheObjects, used for memory limit. @@ -69,9 +69,9 @@ impl HeapSize for CacheObject { /// If a [`CacheObject`] is [`ObjectType::HashDelta`] or [`ObjectType::OffsetDelta`], /// it will expand to another [`CacheObject`] of other types. To prevent potential OOM, /// we record the size of the expanded object as well as that of the object itself. - /// - /// Base objects, *i.e.*, [`ObjectType::Blob`], [`ObjectType::Tree`], [`ObjectType::Commit`], - /// and [`ObjectType::Tag`], will not be expanded, so the heap-size of the object is the same + /// + /// Base objects, *i.e.*, [`ObjectType::Blob`], [`ObjectType::Tree`], [`ObjectType::Commit`], + /// and [`ObjectType::Tag`], will not be expanded, so the heap-size of the object is the same /// as the size of the data. /// /// See [Comment in PR #755](https://github.com/web3infra-foundation/mega/pull/755#issuecomment-2543100481) for more details. @@ -107,6 +107,7 @@ impl Drop for CacheObject { } } +/// Allow CacheObject to be stored and loaded to/from a file. impl Storable for CacheObject { type Location = Path; type Strategy = DefaultFileStorageStrategy; @@ -160,7 +161,7 @@ impl CacheObject { } /// Get the [`SHA1`] hash of the object. - /// + /// /// If the object is a delta object, return [`None`]. pub fn base_object_hash(&self) -> Option { match &self.info { @@ -170,7 +171,7 @@ impl CacheObject { } /// Get the offset delta of the object. - /// + /// /// If the object is not an offset delta, return [`None`]. pub fn offset_delta(&self) -> Option { match &self.info { @@ -180,7 +181,7 @@ impl CacheObject { } /// Get the hash delta of the object. - /// + /// /// If the object is not a hash delta, return [`None`]. pub fn hash_delta(&self) -> Option { match &self.info { @@ -204,17 +205,10 @@ impl CacheObject { } } -/// trait alias for simple use -pub trait ArcWrapperBounds: - HeapSize + Storable + Send + Sync + 'static -{ -} -// You must impl `Alias Trait` for all the `T` satisfying Constraints -// Or, `T` will not satisfy `Alias Trait` even if it satisfies the Original traits -impl + Send + Sync + 'static> ArcWrapperBounds - for T -{ -} +/// trait alias for simplicity +pub trait ArcWrapperBounds: HeapSize + Storable + Send + Sync + 'static {} +// Auto implement the trait alias for all types that implement the required traits +impl + Send + Sync + 'static> ArcWrapperBounds for T {} /// Implementing encapsulation of Arc to enable third-party Trait HeapSize implementation for the Arc type /// Because of use Arc in LruCache, the LruCache is not clear whether a pointer will drop the referenced @@ -225,6 +219,7 @@ pub struct ArcWrapper { pool: Option>, pub store_path: Option, // path to store when drop } + impl ArcWrapper { /// Create a new ArcWrapper pub fn new(data: Arc, share_flag: Arc, pool: Option>) -> Self { @@ -285,7 +280,7 @@ impl Drop for ArcWrapper { if !complete_signal.load(Ordering::Acquire) { let res = data_copy.store(&path_copy); if let Err(e) = res { - println!("[f_save] {:?} error: {:?}", path_copy, e); + eprintln!("[{}] Error while storing {:?}: {:?}", std::any::type_name::(), path_copy, e); } } }); @@ -293,7 +288,7 @@ impl Drop for ArcWrapper { None => { let res = self.data.store(path); if let Err(e) = res { - println!("[f_save] {:?} error: {:?}", path, e); + eprintln!("[{}] Error while storing {:?}: {:?}", std::any::type_name::(), path, e); } } } @@ -307,6 +302,7 @@ mod test { use lru_mem::LruCache; + use crate::utils::storable::DefaultFileStorageStrategy; use crate::MERCURY_DEFAULT_TMP_DIR; use super::*; @@ -346,7 +342,7 @@ mod test { #[test] fn test_cache_object_with_lru() { let mut cache = LruCache::new(2048); - + let hash_a = SHA1::default(); let hash_b = SHA1::new(b"b"); // whatever different hash let a = CacheObject { @@ -481,7 +477,11 @@ mod test { let mut path = PathBuf::from(MERCURY_DEFAULT_TMP_DIR).join("test_arc_wrapper_drop_store"); fs::create_dir_all(&path).unwrap(); path.push("test_obj"); - let mut a = ArcWrapper::new(Arc::new(Test { a: 1024 }), Arc::new(AtomicBool::new(false)), None); + let mut a = ArcWrapper::new( + Arc::new(Test { a: 1024 }), + Arc::new(AtomicBool::new(false)), + None, + ); a.set_store_path(path.clone()); drop(a); diff --git a/mercury/src/utils/storable.rs b/mercury/src/utils/storable.rs index d97959e2..329356af 100644 --- a/mercury/src/utils/storable.rs +++ b/mercury/src/utils/storable.rs @@ -13,20 +13,20 @@ //! ## Examples //! //! ### Using `DefaultFileStorageStrategy` -//! +//! //! If you have a struct that implements `Serialize` and `Deserialize`, you can use //! [`Storable`] for free by specifying the `Location` type as `Path` and the `Strategy` //! as `DefaultFileStorageStrategy`: -//! +//! //! ```rust //! use std::path::Path; //! use serde::{Serialize, Deserialize}; -//! +//! //! #[derive(Serialize, Deserialize)] //! struct MyData { //! value: i32, //! } -//! +//! //! impl Storable for MyData { //! type Location = Path; //! type Strategy = DefaultFileStorageStrategy; @@ -260,23 +260,23 @@ impl StorageStrategy for DefaultFileStorageStrategy where T: Serialize + for<'de> Deserialize<'de> + Storable, { - fn load(location: &T::Location) -> Result { + fn load(location: &Path) -> Result { let data = fs::read(location)?; - let obj: T = + let obj = bincode::deserialize(&data).map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; Ok(obj) } - fn store(obj: &T, location: &T::Location) -> Result<(), io::Error> { + fn store(obj: &T, location: &Path) -> Result<(), io::Error> { if location.exists() { return Ok(()); } - let data = bincode::serialize(obj).unwrap(); let tmp_path = location.with_extension("temp"); let mut file = OpenOptions::new() .write(true) .create_new(true) .open(&tmp_path)?; + let data = bincode::serialize(obj).map_err(|e| io::Error::new(io::ErrorKind::Other, e))?; file.write_all(&data)?; fs::rename(tmp_path, location)?; Ok(()) @@ -304,7 +304,10 @@ mod tests { #[test] fn test_default_file_storage_strategy() { fs::create_dir_all(MERCURY_DEFAULT_TMP_DIR).unwrap(); - let data = MyData { value: 42, complex_field: (vec![1, 2, 3], "hello".to_string()) }; + let data = MyData { + value: 42, + complex_field: (vec![1, 2, 3], "hello".to_string()), + }; let path = PathBuf::from(MERCURY_DEFAULT_TMP_DIR).join("data.bin"); data.store(&path).unwrap(); let loaded = MyData::load(&path).unwrap(); @@ -315,11 +318,14 @@ mod tests { #[test] fn test_arc_indirect_storage_strategy() { fs::create_dir_all(MERCURY_DEFAULT_TMP_DIR).unwrap(); - let data = Arc::new(MyData { value: 42 , complex_field: (vec![1, 2, 3], "hello".to_string()) }); + let data = Arc::new(MyData { + value: 42, + complex_field: (vec![1, 2, 3], "hello".to_string()), + }); let path = PathBuf::from(MERCURY_DEFAULT_TMP_DIR).join("arcdata.bin"); data.store(&path).unwrap(); let loaded = MyData::load(&path).unwrap(); assert_eq!(*data, loaded); fs::remove_file(path).unwrap(); } -} \ No newline at end of file +}