feat: improve performance of metadata ser/de

Author: DevBlocky

.github/workflows/ci.yml

@@ -32,6 +32,7 @@ jobs:
     name: Test ${{ matrix.name }}
     runs-on: ${{ matrix.os || 'ubuntu-latest' }}
     needs: [style]
+    continue-on-error: true
 
     strategy:
       matrix:

.gitignore

@@ -1,3 +1,3 @@
 .DS_Store
 /target
-cache/
+/cache

Cargo.lock

@@ -13,12 +13,11 @@ edition = "2024"
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 
 [dependencies]
+bson = "3.0.0"
 sled = "0.34.7"
 hex = "0.4.3"
 md5 = "0.8.0"
 rand = "0.9.2"
-bson = { version = "3.0.0", features = ["serde"] }
-serde = { version = "1.0.228", features = ["derive"] }
 tokio = { version = "1.48.0", features = ["fs", "io-util"] }
 bytes = "1.10.1"
 lru = "0.16.2"

Cargo.toml

@@ -1,19 +1,27 @@
-use criterion::{BenchmarkId, Criterion, criterion_group};
+use criterion::{BenchmarkId, Criterion, criterion_group, criterion_main};
 use rand::prelude::*;
 
+const CACHE_DIRECTORY: &str = "cache";
+
 fn make_executor_custom<F: FnOnce() -> forceps::CacheBuilder>(
     f: F,
 ) -> (forceps::Cache, tokio::runtime::Runtime) {
+    use std::{fs, io};
+
+    match fs::remove_dir_all(CACHE_DIRECTORY) {
+        Err(e) if e.kind() != io::ErrorKind::NotFound => panic!("{e}"),
+        _ => {}
+    }
+
     let rt = tokio::runtime::Builder::new_current_thread()
         .enable_all()
         .build()
         .unwrap();
-
     let cache = rt.block_on(async move { f().build().await.unwrap() });
     (cache, rt)
 }
 fn make_executor() -> (forceps::Cache, tokio::runtime::Runtime) {
-    make_executor_custom(|| forceps::CacheBuilder::default())
+    make_executor_custom(|| forceps::CacheBuilder::new(CACHE_DIRECTORY))
 }
 
 fn random_bytes(size: usize) -> Vec<u8> {
@@ -60,7 +68,7 @@ pub fn cache_read_const_key(c: &mut Criterion) {
             &tracking,
             move |b, &tracking| {
                 let (db, rt) = make_executor_custom(|| {
-                    forceps::CacheBuilder::default().track_access(tracking)
+                    forceps::CacheBuilder::new(CACHE_DIRECTORY).track_access(tracking)
                 });
                 const KEY: [u8; 4] = [0xDE, 0xAD, 0xBE, 0xEF];
                 let value = random_bytes(VALUE_SZ);
@@ -78,6 +86,7 @@ pub fn cache_read_const_key(c: &mut Criterion) {
 
 pub fn cache_remove_const_key(c: &mut Criterion) {
     c.bench_function("cache::remove_const_key", move |b| {
+        std::fs::remove_dir_all("./cache").unwrap();
         let (db, rt) = make_executor();
         const KEY: [u8; 4] = [0xDE, 0xAD, 0xBE, 0xEF];
         let value = random_bytes(VALUE_SZ);
@@ -104,23 +113,32 @@ pub fn cache_metadata_lookup(c: &mut Criterion) {
     });
 }
 
-criterion_group!(
-    benches,
-    cache_write_const_key,
-    cache_write_random_key,
-    cache_read_const_key,
-    cache_remove_const_key,
-    cache_metadata_lookup
-);
-
-fn main() {
-    // delete cache directory if it exists
-    // this is to make sure we're benching on a clean slate
-    if let Ok(_) = std::fs::read_dir("./cache") {
-        std::fs::remove_dir_all("./cache").unwrap();
-    }
-
-    benches();
+fn bench_config() -> Criterion {
+    Criterion::default()
+        .measurement_time(std::time::Duration::from_secs(10))
+        .configure_from_args()
+}
 
-    Criterion::default().configure_from_args().final_summary();
+criterion_group! {
+    name = benches;
+    config = bench_config();
+    targets =
+        cache_write_const_key,
+        cache_write_random_key,
+        cache_read_const_key,
+        cache_remove_const_key,
+        cache_metadata_lookup
 }
+
+criterion_main!(benches);
+// fn main() {
+//     // delete cache directory if it exists
+//     // this is to make sure we're benching on a clean slate
+//     if let Ok(_) = std::fs::read_dir("./cache") {
+//         std::fs::remove_dir_all("./cache").unwrap();
+//     }
+
+//     benches();
+
+//     Criterion::default().configure_from_args().final_summary();
+// }

benches/benchmarks.rs

@@ -1,3 +1,6 @@
+mod builder;
+pub use builder::CacheBuilder;
+
 use crate::{ForcepError, MetaDb, Metadata, Result, mem_cache::MemCache};
 use bytes::Bytes;
 use std::io;
@@ -20,17 +23,17 @@ async fn tempfile(dir: &path::Path) -> Result<(afs::File, path::PathBuf)> {
 }
 
 #[derive(Debug, Clone)]
-pub(crate) struct Options {
-    pub(crate) path: path::PathBuf,
-    pub(crate) dir_depth: u8,
-    pub(crate) track_access: bool,
+struct Options {
+    path: path::PathBuf,
+    dir_depth: u8,
+    track_access: bool,
 
     // maximum size of the in-memory lru in bytes
-    pub(crate) lru_size: usize,
+    lru_size: usize,
 
     // read and write buffer sizes
-    pub(crate) rbuff_sz: usize,
-    pub(crate) wbuff_sz: usize,
+    rbuff_sz: usize,
+    wbuff_sz: usize,
 }
 
 /// The main component of `forceps`, and  acts as the API for interacting with the on-disk cache.
@@ -93,12 +96,12 @@ impl Cache {
     /// ```
     #[inline]
     #[allow(clippy::new_ret_no_self)]
-    pub fn new<P: AsRef<path::Path>>(path: P) -> crate::CacheBuilder {
-        crate::CacheBuilder::new(path)
+    pub fn new<P: AsRef<path::Path>>(path: P) -> CacheBuilder {
+        CacheBuilder::new(path)
     }
 
     /// Creates a new Cache instance based on the CacheBuilder
-    pub(crate) async fn create(opts: Options) -> Result<Self> {
+    async fn create(opts: Options) -> Result<Self> {
         // create the base directory for the cache
         afs::create_dir_all(&opts.path)
             .await

src/cache.rs

@@ -1,10 +1,7 @@
-use crate::{
-    Result,
-    cache::{Cache, Options},
-};
+use crate::Result;
 use std::path;
 
-/// A builder for the [`Cache`] object. Exposes APIs for configuring the initial setup of the
+/// A builder for the [`Cache`](super::Cache) object. Exposes APIs for configuring the initial setup of the
 /// database.
 ///
 /// # Examples
@@ -24,11 +21,11 @@ use std::path;
 /// ```
 #[derive(Debug, Clone)]
 pub struct CacheBuilder {
-    opts: Options,
+    opts: super::Options,
 }
 
 impl CacheBuilder {
-    /// Creates a new [`CacheBuilder`], which can be used to customize and create a [`Cache`]
+    /// Creates a new [`CacheBuilder`], which can be used to customize and create a [`Cache`](super::Cache)
     /// instance.
     ///
     /// The `path` supplied is the base directory of the cache instance.
@@ -42,7 +39,7 @@ impl CacheBuilder {
     /// // Use other methods for configuration
     /// ```
     pub fn new<P: AsRef<path::Path>>(path: P) -> Self {
-        let opts = Options {
+        let opts = super::Options {
             path: path.as_ref().to_owned(),
             dir_depth: 2,
             track_access: false,
@@ -109,7 +106,7 @@ impl CacheBuilder {
         self
     }
 
-    /// Builds the new [`Cache`] instance using the configured options of the builder.
+    /// Builds the new [`Cache`](super::Cache) instance using the configured options of the builder.
     ///
     /// # Examples
     ///
@@ -124,8 +121,8 @@ impl CacheBuilder {
     ///     .unwrap();
     /// # }
     /// ```
-    pub async fn build(self) -> Result<Cache> {
-        Cache::create(self.opts).await
+    pub async fn build(self) -> Result<super::Cache> {
+        super::Cache::create(self.opts).await
     }
 }
 

src/builder.rs src/cache/builder.rssrc/builder.rs renamed to src/cache/builder.rs

@@ -107,11 +107,8 @@ impl error::Error for ForcepError {
 mod mem_cache;
 mod tmp;
 
-mod builder;
-pub use builder::CacheBuilder;
-
 mod cache;
-pub use cache::Cache;
+pub use cache::{Cache, CacheBuilder};
 
 mod metadata;
 pub(crate) use metadata::MetaDb;

src/lib.rs

@@ -27,7 +27,7 @@ pub type Md5Bytes = [u8; 16];
 /// let metadata = cache.read_metadata(&b"MY_KEY").unwrap();
 /// # }
 /// ```
-#[derive(Debug, serde::Deserialize, serde::Serialize)]
+#[derive(Debug)]
 pub struct Metadata {
     /// Size in bytes of the corresponding entry
     size: u64,
@@ -68,13 +68,75 @@ impl Metadata {
     }
 
     /// Serializes the metadata into bytes
-    pub(crate) fn serialize(&self) -> Result<Vec<u8>> {
-        bson::serialize_to_vec(self).map_err(ForcepError::MetaSer)
+    pub(crate) fn serialize(&self) -> Vec<u8> {
+        use bson::{
+            cstr,
+            raw::{RawBinaryRef, RawBson, RawDocumentBuf},
+        };
+
+        let mut doc = RawDocumentBuf::new();
+        doc.append(cstr!("size"), RawBson::Int64(self.size as i64));
+        doc.append(
+            cstr!("last_modified"),
+            RawBson::Int64(self.last_modified as i64),
+        );
+        doc.append(
+            cstr!("last_accessed"),
+            RawBson::Int64(self.last_accessed as i64),
+        );
+        doc.append(cstr!("hits"), RawBson::Int64(self.hits as i64));
+        doc.append(
+            cstr!("integrity"),
+            RawBinaryRef {
+                subtype: bson::spec::BinarySubtype::Md5,
+                bytes: &self.integrity,
+            },
+        );
+        doc.into_bytes()
     }
 
     /// Deserializes a slice of bytes into metadata
     pub(crate) fn deserialize(buf: &[u8]) -> Result<Self> {
-        bson::deserialize_from_slice(buf).map_err(ForcepError::MetaDe)
+        use bson::{error::Error as BsonError, raw::RawDocument, spec::BinarySubtype};
+
+        let doc = RawDocument::from_bytes(buf).map_err(ForcepError::MetaDe)?;
+
+        let make_error = |key: &str, msg: &str| -> ForcepError {
+            let io_err = std::io::Error::new(std::io::ErrorKind::InvalidData, msg.to_owned());
+            let mut err = BsonError::from(io_err);
+            err.key = Some(key.to_owned());
+            ForcepError::MetaDe(err)
+        };
+
+        let read_u64 = |key: &str| -> Result<u64> {
+            doc.get_i64(key)
+                .map(|v| v as u64)
+                .map_err(ForcepError::MetaDe)
+        };
+
+        let size = read_u64("size")?;
+        let last_modified = read_u64("last_modified")?;
+        let last_accessed = read_u64("last_accessed")?;
+        let hits = read_u64("hits")?;
+
+        let binary = doc.get_binary("integrity").map_err(ForcepError::MetaDe)?;
+        if binary.subtype != BinarySubtype::Md5 {
+            return Err(make_error("integrity", "expected MD5 binary subtype"));
+        }
+        const MD5_LEN: usize = 16;
+        if binary.bytes.len() != MD5_LEN {
+            return Err(make_error("integrity", "integrity must contain 16 bytes"));
+        }
+        let mut integrity = [0u8; MD5_LEN];
+        integrity.copy_from_slice(binary.bytes);
+
+        Ok(Self {
+            size,
+            last_modified,
+            last_accessed,
+            hits,
+            integrity,
+        })
     }
 
     /// The size in bytes of the corresponding cache entry.
@@ -174,7 +236,7 @@ impl MetaDb {
     /// If a previous entry exists, it is simply overwritten.
     pub fn insert_metadata_for(&self, key: &[u8], data: &[u8]) -> Result<Metadata> {
         let meta = Metadata::new(data);
-        let bytes = Metadata::serialize(&meta)?;
+        let bytes = Metadata::serialize(&meta);
         self.db
             .insert(key, &bytes[..])
             .map_err(ForcepError::MetaDb)?;
@@ -200,7 +262,7 @@ impl MetaDb {
         meta.last_accessed = now_since_epoch();
         meta.hits += 1;
         self.db
-            .insert(key, Metadata::serialize(&meta)?)
+            .insert(key, Metadata::serialize(&meta))
             .map_err(ForcepError::MetaDb)?;
         Ok(meta)
     }
@@ -264,7 +326,7 @@ mod test {
     fn metadata_ser_de() {
         let db = create_db().unwrap();
         let meta = db.insert_metadata_for(&DATA, &DATA).unwrap();
-        let ser_bytes = meta.serialize().unwrap();
+        let ser_bytes = meta.serialize();
         let de = Metadata::deserialize(&ser_bytes).unwrap();
         assert_eq!(meta.get_integrity(), de.get_integrity());
     }