From 9fd7947aa5665ce1d5458fb7e43f0a2ad2b5e2c6 Mon Sep 17 00:00:00 2001 From: ByteAlex Date: Sat, 22 Jan 2022 16:02:19 +0100 Subject: [PATCH 1/5] Improve TTL backing --- src/backing.rs | 118 +++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 99 insertions(+), 19 deletions(-) diff --git a/src/backing.rs b/src/backing.rs index d5719fe..78c229a 100644 --- a/src/backing.rs +++ b/src/backing.rs @@ -6,6 +6,7 @@ use lru::LruCache; use std::collections::VecDeque; #[cfg(feature = "ttl-cache")] use std::ops::Add; +use std::process::id; #[cfg(feature = "ttl-cache")] use tokio::time::{Instant, Duration}; @@ -23,7 +24,7 @@ pub trait CacheBacking #[cfg(feature = "lru-cache")] pub struct LruCacheBacking { - lru: LruCache + lru: LruCache, } #[cfg(feature = "lru-cache")] @@ -93,10 +94,26 @@ impl< #[cfg(feature = "ttl-cache")] pub struct TtlCacheBacking { ttl: Duration, - expiry_queue: VecDeque<(K, Instant)>, + expiry_queue: VecDeque>, map: HashMap, } +#[cfg(feature = "ttl-cache")] +struct TTlEntry { + key: K, + expiry: Instant, + +} + +impl From<(K, Instant)> for TTlEntry { + fn from(tuple: (K, Instant)) -> Self { + Self { + key: tuple.0, + expiry: tuple.1, + } + } +} + #[cfg(feature = "ttl-cache")] impl< K: Eq + Hash + Sized + Clone + Send, @@ -117,21 +134,21 @@ impl< fn set(&mut self, key: K, value: V) -> Option { self.remove_old(); let expiry = Instant::now().add(self.ttl); - let option = self.map.insert(key.clone(), (value, expiry)); - if option.is_some() { - self.expiry_queue.retain(|(vec_key, _)| vec_key.ne(&key)); + let result = self.replace(key.clone(), value, expiry); + match self.expiry_queue.binary_search_by_key(&expiry, |entry| entry.expiry) { + Ok(found) => { + self.expiry_queue.insert(found + 1, (key, expiry).into()); + } + Err(idx) => { + self.expiry_queue.insert(idx, (key, expiry).into()); + } } - self.expiry_queue.push_back((key, expiry)); - option.map(|(value, _)| value) + result } fn remove(&mut self, key: &K) -> Option { self.remove_old(); - let option = self.map.remove(key); - if option.is_some() { - self.expiry_queue.retain(|(vec_key, _)| vec_key.ne(&key)); - } - option.map(|(value, _)| value) + self.remove_key(key) } fn contains_key(&self, key: &K) -> bool { @@ -154,7 +171,8 @@ impl< .collect::>(); for key in keys.into_iter() { self.map.remove(&key); - self.expiry_queue.retain(|(expiry_key, _)| expiry_key.ne(&key)) + // optimize looping through expiry_queue multiple times? + self.expiry_queue.retain(|entry| entry.key.ne(&key)) } } @@ -165,7 +183,7 @@ impl< } #[cfg(feature = "ttl-cache")] -impl TtlCacheBacking { +impl TtlCacheBacking { pub fn new(ttl: Duration) -> TtlCacheBacking { TtlCacheBacking { ttl, @@ -176,18 +194,80 @@ impl TtlCacheBacking { fn remove_old(&mut self) { let now = Instant::now(); - while let Some((key, expiry)) = self.expiry_queue.pop_front() { - if now.lt(&expiry) { - self.expiry_queue.push_front((key, expiry)); + while let Some(entry) = self.expiry_queue.pop_front() { + if now.lt(&entry.expiry) { + self.expiry_queue.push_front(entry); break; } - self.map.remove(&key); + self.map.remove(&entry.key); + } + } + + fn replace(&mut self, key: K, value: V, expiry: Instant) -> Option { + let entry = self.map.insert(key.clone(), (value, expiry)); + self.cleanup_expiry(entry, &key) + } + + fn remove_key(&mut self, key: &K) -> Option { + let entry = self.map.remove(key); + self.cleanup_expiry(entry, key) + } + + fn cleanup_expiry(&mut self, entry: Option<(V, Instant)>, key: &K) -> Option { + if let Some((value, old_expiry)) = entry { + match self.expiry_queue.binary_search_by_key(&old_expiry, |entry| entry.expiry) { + Ok(found) => { + let index = self.expiry_index_on_key_eq(found, &old_expiry, key); + if let Some(index) = index { + self.expiry_queue.remove(index); + } else { + // expiry not found (key)??? + } + } + Err(_) => { + // expiry not found??? + } + } + Some(value) + } else { + None + } + } + + fn expiry_index_on_key_eq(&self, idx: usize, expiry: &Instant, key: &K) -> Option { + let entry = self.expiry_queue.get(idx).unwrap(); + if entry.key.eq(key) { + return Some(idx); + } + + let mut offset = 0; + while idx - offset > 0 { + offset += 1; + let entry = self.expiry_queue.get(idx - offset).unwrap(); + if !entry.expiry.eq(expiry) { + break; + } + if entry.key.eq(key) { + return Some(idx - offset); + } + } + offset = 0; + while idx + offset < self.expiry_queue.len() { + offset += 1; + let entry = self.expiry_queue.get(idx + offset).unwrap(); + if !entry.expiry.eq(expiry) { + break; + } + if entry.key.eq(key) { + return Some(idx + offset); + } } + None } } pub struct HashMapBacking { - map: HashMap + map: HashMap, } impl< From 82c576628812ab4b4cf57ed17015ab63d97569ff Mon Sep 17 00:00:00 2001 From: ByteAlex Date: Sat, 22 Jan 2022 16:03:05 +0100 Subject: [PATCH 2/5] How did that import slip in.. --- src/backing.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/backing.rs b/src/backing.rs index 78c229a..8ee60a8 100644 --- a/src/backing.rs +++ b/src/backing.rs @@ -6,7 +6,6 @@ use lru::LruCache; use std::collections::VecDeque; #[cfg(feature = "ttl-cache")] use std::ops::Add; -use std::process::id; #[cfg(feature = "ttl-cache")] use tokio::time::{Instant, Duration}; From af25999c424a858b16a4dd861d203b90c4eba004 Mon Sep 17 00:00:00 2001 From: ByteAlex Date: Sat, 22 Jan 2022 16:08:42 +0100 Subject: [PATCH 3/5] Fix features --- src/backing.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/backing.rs b/src/backing.rs index 8ee60a8..7b89823 100644 --- a/src/backing.rs +++ b/src/backing.rs @@ -104,6 +104,7 @@ struct TTlEntry { } +#[cfg(feature = "ttl-cache")] impl From<(K, Instant)> for TTlEntry { fn from(tuple: (K, Instant)) -> Self { Self { From 3017896cf3fe5823d17c1a6d7b3e6aa98ac316e0 Mon Sep 17 00:00:00 2001 From: ByteAlex Date: Sat, 22 Jan 2022 16:14:42 +0100 Subject: [PATCH 4/5] Improve test matrix --- .github/workflows/rust.yml | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index fa15bc0..cf0b80f 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -23,5 +23,11 @@ jobs: - uses: actions/checkout@v2 - name: Build run: cargo build - - name: Run tests - run: cargo test --features "ttl-cache","lru-cache" + - name: Run tests no-features + run: cargo test + - name: Run tests lru-feature + run: cargo test --features lru-cache + - name: Run tests ttl-feature + run: cargo test --features ttl-cache + - name: Run tests all-features + run: cargo test --features lru-cache,ttl-cache \ No newline at end of file From 0065b6d66738d57bcc8c667cfd1249f0235be03d Mon Sep 17 00:00:00 2001 From: ByteAlex Date: Sun, 23 Jan 2022 16:17:36 +0100 Subject: [PATCH 5/5] Improve tests (test all features) --- Cargo.toml | 3 + cache-loader-async-macros/Cargo.toml | 14 ++++ cache-loader-async-macros/src/lib.rs | 113 +++++++++++++++++++++++++++ src/test.rs | 113 +++++++++++---------------- 4 files changed, 175 insertions(+), 68 deletions(-) create mode 100644 cache-loader-async-macros/Cargo.toml create mode 100644 cache-loader-async-macros/src/lib.rs diff --git a/Cargo.toml b/Cargo.toml index 1f10e3b..524ed88 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,6 +19,9 @@ thiserror = "1.0" # Optional feature based dependencies lru = { version = "0.6.5", optional = true } +[dev-dependencies] +cache_loader_async_macros = { path = "./cache-loader-async-macros" } + [features] default = [] lru-cache = ["lru"] diff --git a/cache-loader-async-macros/Cargo.toml b/cache-loader-async-macros/Cargo.toml new file mode 100644 index 0000000..e3a6cfc --- /dev/null +++ b/cache-loader-async-macros/Cargo.toml @@ -0,0 +1,14 @@ +[package] +name = "cache_loader_async_macros" +version = "0.1.0" +edition = "2021" + +# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html + +[lib] +proc-macro = true + +[dependencies] +syn = { version = "1.0", features = ["full"] } +quote = "1.0" +proc-macro2 = "1.0" \ No newline at end of file diff --git a/cache-loader-async-macros/src/lib.rs b/cache-loader-async-macros/src/lib.rs new file mode 100644 index 0000000..fd60c64 --- /dev/null +++ b/cache-loader-async-macros/src/lib.rs @@ -0,0 +1,113 @@ +extern crate proc_macro; + +use quote::quote; +use proc_macro2::{Ident, Span, TokenStream, TokenTree}; +use proc_macro2::token_stream::IntoIter; + +fn collect_diamond_idents(stream: &mut IntoIter) -> Vec { + let mut idents = Vec::new(); + if let TokenTree::Punct(punct) = stream.next().expect("Missing next element") { + if !punct.as_char().eq(&'<') { + panic!("Invalid diamond start"); + } + } else { + panic!("Invalid diamond start"); + } + let mut expect_ident = true; + loop { + match stream.next().expect("Missing next element") { + TokenTree::Ident(ident) => { + if expect_ident { + expect_ident = false; + idents.push(ident); + } else { + panic!("Invalid diamond format! (Didn't expect ident)"); + } + }, + TokenTree::Punct(punct) => { + if !expect_ident { + if punct.as_char().eq(&',') { + expect_ident = true; + } else if punct.as_char().eq(&'>') { + break; + } else { + panic!("Invalid diamond format! (Invalid punct)"); + } + } else { + panic!("Invalid diamond format! (Didn't expect punct)"); + } + } + _ => panic!("Invalid type"), + } + } + idents +} + +#[proc_macro] +pub fn test_with_features(item: proc_macro::TokenStream) -> proc_macro::TokenStream { + let mut stream = TokenStream::from(item).into_iter(); + let fn_ident = if let TokenTree::Ident(ident) = stream.next().expect("First token mandatory") { + ident + } else { + panic!("First token must be an ident!"); + }; + let ident = if let TokenTree::Ident(ident) = stream.next().expect("Second token mandatory") { + ident + } else { + panic!("Second token must be an ident!"); + }; + let types = collect_diamond_idents(&mut stream); + let loader = if let TokenTree::Group(group) = stream.next().expect("Missing group token") { + group + } else { + panic!("Group token not present"); + }; + + let mut fn_body = quote! {}; + while let Some(token) = stream.next() { + fn_body = quote! { + #fn_body #token + } + } + + let key_type = types.get(0).unwrap(); + let value_type = types.get(1).unwrap(); + let error_type = types.get(2).unwrap(); + + let fn_ident_default = syn::Ident::new(&format!("test_default_{}", fn_ident), Span::call_site()); + let fn_ident_lru = syn::Ident::new(&format!("test_lru_{}", fn_ident), Span::call_site()); + let fn_ident_ttl = syn::Ident::new(&format!("test_ttl_{}", fn_ident), Span::call_site()); + + let result = quote! { + #[tokio::test] + async fn #fn_ident_default() { + let #ident: LoadingCache<#key_type, #value_type, #error_type> = LoadingCache::new(move |key: #key_type| { + async move #loader + }); + + #fn_body + } + + #[cfg(feature = "lru-cache")] + #[tokio::test] + async fn #fn_ident_lru() { + let #ident: LoadingCache<#key_type, #value_type, #error_type> = LoadingCache::with_backing(LruCacheBacking::new(100), move |key: #key_type| { + async move #loader + }); + + #fn_body + } + + #[cfg(feature = "ttl-cache")] + #[tokio::test] + async fn #fn_ident_ttl() { + let #ident: LoadingCache<#key_type, #value_type, #error_type> = LoadingCache::with_backing(TtlCacheBacking::new(Duration::from_secs(3)), move |key: #key_type| { + async move #loader + }); + + #fn_body + } + }; + + return result.into(); +} \ No newline at end of file diff --git a/src/test.rs b/src/test.rs index 6ce8e47..f27e7ad 100644 --- a/src/test.rs +++ b/src/test.rs @@ -1,6 +1,7 @@ use std::collections::HashMap; use crate::cache_api::{LoadingCache, CacheLoadingError}; use tokio::time::Duration; +use cache_loader_async_macros::test_with_features; #[cfg(feature = "lru-cache")] use crate::backing::LruCacheBacking; #[cfg(feature = "ttl-cache")] @@ -12,6 +13,7 @@ pub struct ThingOne(u8); #[derive(Debug, Clone)] pub struct ThingTwo(String); + #[tokio::test] async fn test_load() { let thing_one_static_db: HashMap = @@ -51,13 +53,10 @@ async fn test_load() { assert_eq!(result_two, "buzz".to_owned()); } -#[tokio::test] -async fn test_write() { - let cache: LoadingCache = LoadingCache::new(move |key: String| { - async move { - Ok(key.to_lowercase()) - } - }); +test_with_features! { + test_write cache { + Ok(key.to_lowercase()) + } let lowercase_result = cache.get("LOL".to_owned()).await.unwrap(); println!("Result of lowercase loader: {}", lowercase_result); @@ -70,13 +69,10 @@ async fn test_write() { assert_eq!(result, "BIG".to_owned()); } -#[tokio::test] -async fn test_get_if_present() { - let cache: LoadingCache = LoadingCache::new(move |key: String| { - async move { - Ok(key.to_lowercase()) - } - }); +test_with_features! { + get_if_present cache { + Ok(key.to_lowercase()) + } let option = cache.get_if_present("test".to_owned()).await.unwrap(); assert!(option.is_none()); @@ -87,13 +83,10 @@ async fn test_get_if_present() { assert!(option.is_some()); } -#[tokio::test] -async fn test_exists() { - let cache: LoadingCache = LoadingCache::new(move |key: String| { - async move { - Ok(key.to_lowercase()) - } - }); +test_with_features! { + exists cache { + Ok(key.to_lowercase()) + } let exists = cache.exists("test".to_owned()).await.unwrap(); assert!(!exists); @@ -158,14 +151,11 @@ async fn test_update() { assert_eq!(result, "race_condition".to_owned()); } -#[tokio::test] -async fn test_update_if_exists() { - let cache: LoadingCache = LoadingCache::new(move |key: String| { - async move { - tokio::time::sleep(Duration::from_millis(500)).await; - Ok(key.to_lowercase()) - } - }); +test_with_features! { + update_if_exists cache { + tokio::time::sleep(Duration::from_millis(500)).await; + Ok(key.to_lowercase()) + } cache.set("test".to_owned(), "test".to_owned()).await.ok(); let no_value = cache.update_if_exists("test2".to_owned(), |val| { @@ -234,14 +224,11 @@ async fn test_update_mut() { assert_eq!(result, "race_condition".to_owned()); } -#[tokio::test] -async fn test_update_mut_if_exists() { - let cache: LoadingCache = LoadingCache::new(move |key: String| { - async move { - tokio::time::sleep(Duration::from_millis(500)).await; - Ok(key.to_lowercase()) - } - }); +test_with_features! { + update_mut_if_exists cache { + tokio::time::sleep(Duration::from_millis(500)).await; + Ok(key.to_lowercase()) + } cache.set("test".to_owned(), "test".to_owned()).await.ok(); let no_value = cache.update_mut_if_exists("test2".to_owned(), |val| { @@ -258,14 +245,11 @@ async fn test_update_mut_if_exists() { assert_eq!(two_test.unwrap(), "testtest"); } -#[tokio::test] -async fn test_remove() { - let cache: LoadingCache = LoadingCache::new(move |key: String| { - async move { - tokio::time::sleep(Duration::from_millis(500)).await; - Ok(key.to_lowercase()) - } - }); +test_with_features! { + remove cache { + tokio::time::sleep(Duration::from_millis(500)).await; + Ok(key.to_lowercase()) + } cache.set("test".to_owned(), "lol".to_owned()).await.ok(); @@ -276,14 +260,12 @@ async fn test_remove() { assert_eq!(cache.get("test".to_owned()).await.unwrap(), "test".to_owned()); } -#[tokio::test] -async fn test_remove_if() { - let cache: LoadingCache = LoadingCache::new(move |key: u64| { - async move { - tokio::time::sleep(Duration::from_millis(500)).await; - Ok(key * 2) - } - }); + +test_with_features! { + remove_if cache { + tokio::time::sleep(Duration::from_millis(500)).await; + Ok(key * 2) + } cache.set(1, 2).await.ok(); cache.set(2, 4).await.ok(); @@ -307,13 +289,11 @@ async fn test_remove_if() { assert!(cache.get_if_present(5).await.unwrap().is_none()); } -#[tokio::test] -async fn test_load_error() { - let cache: LoadingCache = LoadingCache::new(move |_key: String| { - async move { - Err(5) - } - }); + +test_with_features! { + load_error cache { + Err(5) + } let cache_loading_error = cache.get("test".to_owned()).await.expect_err("Didn't error, what?"); if let CacheLoadingError::LoadingError(val) = cache_loading_error { @@ -323,14 +303,11 @@ async fn test_load_error() { } } -#[tokio::test] -async fn test_meta() { - let cache: LoadingCache = LoadingCache::new(move |key: String| { - async move { - tokio::time::sleep(Duration::from_millis(500)).await; - Ok(key.to_lowercase()) - } - }); +test_with_features! { + meta cache { + tokio::time::sleep(Duration::from_millis(500)).await; + Ok(key.to_lowercase()) + } let meta = cache.get_with_meta("key".to_owned()).await.unwrap(); assert!(!meta.cached);