From f9ac2f95ecd30b957665b732b2da9e8622d8310f Mon Sep 17 00:00:00 2001 From: Croxx Date: Tue, 19 Nov 2024 17:23:03 +0800 Subject: [PATCH] refactor: wrapper no/immutable/mutable op into Op (#788) Signed-off-by: MrCroxx --- foyer-memory/src/eviction/fifo.rs | 34 ++------ foyer-memory/src/eviction/lfu.rs | 109 ++++++++++-------------- foyer-memory/src/eviction/lru.rs | 98 ++++++++++----------- foyer-memory/src/eviction/mod.rs | 77 +++++++++-------- foyer-memory/src/eviction/s3fifo.rs | 38 +++------ foyer-memory/src/eviction/test_utils.rs | 42 ++++++++- foyer-memory/src/prelude.rs | 2 +- foyer-memory/src/raw.rs | 56 +++++++----- 8 files changed, 226 insertions(+), 230 deletions(-) diff --git a/foyer-memory/src/eviction/fifo.rs b/foyer-memory/src/eviction/fifo.rs index 680da0a9..bcc14188 100644 --- a/foyer-memory/src/eviction/fifo.rs +++ b/foyer-memory/src/eviction/fifo.rs @@ -18,7 +18,7 @@ use foyer_common::code::{Key, Value}; use intrusive_collections::{intrusive_adapter, LinkedList, LinkedListAtomicLink}; use serde::{Deserialize, Serialize}; -use super::{Eviction, Operator}; +use super::{Eviction, Op}; use crate::record::{CacheHint, Record}; /// Fifo eviction algorithm config. @@ -93,28 +93,12 @@ where record.set_in_eviction(false); } - fn acquire_operator() -> super::Operator { - Operator::Noop + fn acquire() -> Op { + Op::noop() } - fn acquire_immutable(&self, _record: &Arc>) { - unreachable!() - } - - fn acquire_mutable(&mut self, _record: &Arc>) { - unreachable!() - } - - fn release_operator() -> super::Operator { - Operator::Noop - } - - fn release_immutable(&self, _record: &Arc>) { - unreachable!() - } - - fn release_mutable(&mut self, _record: &Arc>) { - unreachable!() + fn release() -> Op { + Op::noop() } } @@ -125,17 +109,17 @@ pub mod tests { use super::*; use crate::{ - eviction::test_utils::{assert_ptr_eq, assert_ptr_vec_eq, TestEviction}, + eviction::test_utils::{assert_ptr_eq, assert_ptr_vec_eq, Dump}, record::Data, }; - impl TestEviction for Fifo + impl Dump for Fifo where K: Key + Clone, V: Value + Clone, { - type Dump = Vec>>; - fn dump(&self) -> Self::Dump { + type Output = Vec>>; + fn dump(&self) -> Self::Output { let mut res = vec![]; let mut cursor = self.queue.cursor(); loop { diff --git a/foyer-memory/src/eviction/lfu.rs b/foyer-memory/src/eviction/lfu.rs index 88e1e215..182f2f9f 100644 --- a/foyer-memory/src/eviction/lfu.rs +++ b/foyer-memory/src/eviction/lfu.rs @@ -22,7 +22,7 @@ use foyer_common::{ use intrusive_collections::{intrusive_adapter, LinkedList, LinkedListAtomicLink}; use serde::{Deserialize, Serialize}; -use super::{Eviction, Operator}; +use super::{Eviction, Op}; use crate::record::{CacheHint, Record}; /// w-TinyLFU eviction algorithm config. @@ -347,71 +347,56 @@ where } } - fn acquire_operator() -> super::Operator { - // TODO(MrCroxx): use a count-min-sketch with atomic u16 impl. - Operator::Mutable - } - - fn acquire_immutable(&self, _record: &Arc>) { - unreachable!() - } - - fn acquire_mutable(&mut self, record: &Arc>) { - // Update frequency by access. - self.update_frequencies(record.hash()); + fn acquire() -> Op { + Op::mutable(|this: &mut Self, record| { + // Update frequency by access. + this.update_frequencies(record.hash()); - if !record.is_in_eviction() { - return; - } + if !record.is_in_eviction() { + return; + } - let state = unsafe { &mut *record.state().get() }; + let state = unsafe { &mut *record.state().get() }; - strict_assert!(state.link.is_linked()); + strict_assert!(state.link.is_linked()); - match state.queue { - Queue::None => unreachable!(), - Queue::Window => { - // Move to MRU position of `window`. - let r = unsafe { self.window.remove_from_ptr(Arc::as_ptr(record)) }; - self.window.push_back(r); - } - Queue::Probation => { - // Promote to MRU position of `protected`. - let r = unsafe { self.probation.remove_from_ptr(Arc::as_ptr(record)) }; - self.decrease_queue_weight(Queue::Probation, record.weight()); - state.queue = Queue::Protected; - self.increase_queue_weight(Queue::Protected, record.weight()); - self.protected.push_back(r); - - // If `protected` weight exceeds the capacity, overflow entry from `protected` to `probation`. - while self.protected_weight > self.protected_weight_capacity { - strict_assert!(!self.protected.is_empty()); - let r = self.protected.pop_front().unwrap(); - let s = unsafe { &mut *r.state().get() }; - self.decrease_queue_weight(Queue::Protected, r.weight()); - s.queue = Queue::Probation; - self.increase_queue_weight(Queue::Probation, r.weight()); - self.probation.push_back(r); + match state.queue { + Queue::None => unreachable!(), + Queue::Window => { + // Move to MRU position of `window`. + let r = unsafe { this.window.remove_from_ptr(Arc::as_ptr(record)) }; + this.window.push_back(r); + } + Queue::Probation => { + // Promote to MRU position of `protected`. + let r = unsafe { this.probation.remove_from_ptr(Arc::as_ptr(record)) }; + this.decrease_queue_weight(Queue::Probation, record.weight()); + state.queue = Queue::Protected; + this.increase_queue_weight(Queue::Protected, record.weight()); + this.protected.push_back(r); + + // If `protected` weight exceeds the capacity, overflow entry from `protected` to `probation`. + while this.protected_weight > this.protected_weight_capacity { + strict_assert!(!this.protected.is_empty()); + let r = this.protected.pop_front().unwrap(); + let s = unsafe { &mut *r.state().get() }; + this.decrease_queue_weight(Queue::Protected, r.weight()); + s.queue = Queue::Probation; + this.increase_queue_weight(Queue::Probation, r.weight()); + this.probation.push_back(r); + } + } + Queue::Protected => { + // Move to MRU position of `protected`. + let r = unsafe { this.protected.remove_from_ptr(Arc::as_ptr(record)) }; + this.protected.push_back(r); } } - Queue::Protected => { - // Move to MRU position of `protected`. - let r = unsafe { self.protected.remove_from_ptr(Arc::as_ptr(record)) }; - self.protected.push_back(r); - } - } - } - - fn release_operator() -> Operator { - Operator::Noop - } - - fn release_immutable(&self, _record: &Arc>) { - unreachable!() + }) } - fn release_mutable(&mut self, _record: &Arc>) { - unreachable!() + fn release() -> Op { + Op::noop() } } @@ -422,17 +407,17 @@ mod tests { use super::*; use crate::{ - eviction::test_utils::{assert_ptr_eq, assert_ptr_vec_vec_eq, TestEviction}, + eviction::test_utils::{assert_ptr_eq, assert_ptr_vec_vec_eq, Dump, OpExt}, record::Data, }; - impl TestEviction for Lfu + impl Dump for Lfu where K: Key + Clone, V: Value + Clone, { - type Dump = Vec>>>; - fn dump(&self) -> Self::Dump { + type Output = Vec>>>; + fn dump(&self) -> Self::Output { let mut window = vec![]; let mut probation = vec![]; let mut protected = vec![]; diff --git a/foyer-memory/src/eviction/lru.rs b/foyer-memory/src/eviction/lru.rs index 74c77b11..d0a2fa80 100644 --- a/foyer-memory/src/eviction/lru.rs +++ b/foyer-memory/src/eviction/lru.rs @@ -21,7 +21,7 @@ use foyer_common::{ use intrusive_collections::{intrusive_adapter, LinkedList, LinkedListAtomicLink}; use serde::{Deserialize, Serialize}; -use super::{Eviction, Operator}; +use super::{Eviction, Op}; use crate::record::{CacheHint, Record}; /// Lru eviction algorithm config. @@ -248,70 +248,58 @@ where assert_eq!(self.high_priority_weight, 0); } - fn acquire_operator() -> super::Operator { - Operator::Mutable - } - - fn acquire_immutable(&self, _record: &Arc>) { - unreachable!() - } - - fn acquire_mutable(&mut self, record: &Arc>) { - if !record.is_in_eviction() { - return; - } - - let state = unsafe { &mut *record.state().get() }; - assert!(state.link.is_linked()); + fn acquire() -> Op { + Op::mutable(|this: &mut Self, record| { + if !record.is_in_eviction() { + return; + } - if state.is_pinned { - return; - } + let state = unsafe { &mut *record.state().get() }; + assert!(state.link.is_linked()); - // Pin the record by moving it to the pin list. + if state.is_pinned { + return; + } - let r = if state.in_high_priority_pool { - unsafe { self.high_priority_list.remove_from_ptr(Arc::as_ptr(record)) } - } else { - unsafe { self.list.remove_from_ptr(Arc::as_ptr(record)) } - }; + // Pin the record by moving it to the pin list. - self.pin_list.push_back(r); + let r = if state.in_high_priority_pool { + unsafe { this.high_priority_list.remove_from_ptr(Arc::as_ptr(record)) } + } else { + unsafe { this.list.remove_from_ptr(Arc::as_ptr(record)) } + }; - state.is_pinned = true; - } + this.pin_list.push_back(r); - fn release_operator() -> Operator { - Operator::Mutable + state.is_pinned = true; + }) } - fn release_immutable(&self, _record: &Arc>) { - unreachable!() - } - - fn release_mutable(&mut self, record: &Arc>) { - if !record.is_in_eviction() { - return; - } + fn release() -> Op { + Op::mutable(|this: &mut Self, record| { + if !record.is_in_eviction() { + return; + } - let state = unsafe { &mut *record.state().get() }; - assert!(state.link.is_linked()); + let state = unsafe { &mut *record.state().get() }; + assert!(state.link.is_linked()); - if !state.is_pinned { - return; - } + if !state.is_pinned { + return; + } - // Unpin the record by moving it from the pin list. + // Unpin the record by moving it from the pin list. - unsafe { self.pin_list.remove_from_ptr(Arc::as_ptr(record)) }; + unsafe { this.pin_list.remove_from_ptr(Arc::as_ptr(record)) }; - if state.in_high_priority_pool { - self.high_priority_list.push_back(record.clone()); - } else { - self.list.push_back(record.clone()); - } + if state.in_high_priority_pool { + this.high_priority_list.push_back(record.clone()); + } else { + this.list.push_back(record.clone()); + } - state.is_pinned = false; + state.is_pinned = false; + }) } } @@ -322,17 +310,17 @@ pub mod tests { use super::*; use crate::{ - eviction::test_utils::{assert_ptr_eq, assert_ptr_vec_vec_eq, TestEviction}, + eviction::test_utils::{assert_ptr_eq, assert_ptr_vec_vec_eq, Dump, OpExt}, record::Data, }; - impl TestEviction for Lru + impl Dump for Lru where K: Key + Clone, V: Value + Clone, { - type Dump = Vec>>>; - fn dump(&self) -> Self::Dump { + type Output = Vec>>>; + fn dump(&self) -> Self::Output { let mut low = vec![]; let mut high = vec![]; let mut pin = vec![]; diff --git a/foyer-memory/src/eviction/mod.rs b/foyer-memory/src/eviction/mod.rs index 612de1a6..564b5008 100644 --- a/foyer-memory/src/eviction/mod.rs +++ b/foyer-memory/src/eviction/mod.rs @@ -28,10 +28,48 @@ impl State for T where T: Send + Sync + 'static + Default {} pub trait Config: Send + Sync + 'static + Clone + Serialize + DeserializeOwned + Default {} impl Config for T where T: Send + Sync + 'static + Clone + Serialize + DeserializeOwned + Default {} -pub enum Operator { +/// Wrapper for one of the three kind of operations for the eviction container: +/// +/// 1. no operation +/// 2. immutable operation +/// 3. mutable operation +#[expect(clippy::type_complexity)] +pub enum Op +where + E: Eviction, +{ + /// no operation Noop, - Immutable, - Mutable, + /// immutable operation + Immutable(Box>) + Send + Sync + 'static>), + /// mutable operation + Mutable(Box>) + Send + Sync + 'static>), +} + +impl Op +where + E: Eviction, +{ + /// no operation + pub fn noop() -> Self { + Self::Noop + } + + /// immutable operation + pub fn immutable(f: F) -> Self + where + F: Fn(&E, &Arc>) + Send + Sync + 'static, + { + Self::Immutable(Box::new(f)) + } + + /// mutable operation + pub fn mutable(f: F) -> Self + where + F: FnMut(&mut E, &Arc>) + Send + Sync + 'static, + { + Self::Mutable(Box::new(f)) + } } /// Cache eviction algorithm abstraction. @@ -92,44 +130,15 @@ pub trait Eviction: Send + Sync + 'static + Sized { while self.pop().is_some() {} } - /// Determine if the immutable version or the mutable version to use for the `acquire` operation. - /// - /// Only the chosen version needs to be implemented. The other version is recommend to be left as `unreachable!()`. - fn acquire_operator() -> Operator; - - /// Immutable version of the `acquire` operation. - /// /// `acquire` is called when an external caller acquire a cache entry from the cache. /// /// The entry can be EITHER in the cache eviction algorithm instance or not. - fn acquire_immutable(&self, record: &Arc>); + fn acquire() -> Op; - /// Mutable version of the `acquire` operation. - /// - /// `acquire` is called when an external caller acquire a cache entry from the cache. - /// - /// The entry can be EITHER in the cache eviction algorithm instance or not. - fn acquire_mutable(&mut self, records: &Arc>); - - /// Determine if the immutable version or the mutable version to use for the `release` operation. - /// - /// Only the chosen version needs to be implemented. The other version is recommend to be left as `unreachable!()`. - fn release_operator() -> Operator; - - /// Immutable version of the `release` operation. - /// - /// - /// `release` is called when the last external caller drops the cache entry. - /// - /// The entry can be EITHER in the cache eviction algorithm instance or not. - fn release_immutable(&self, record: &Arc>); - - /// Mutable version of the `release` operation. - /// /// `release` is called when the last external caller drops the cache entry. /// /// The entry can be EITHER in the cache eviction algorithm instance or not. - fn release_mutable(&mut self, record: &Arc>); + fn release() -> Op; } pub mod fifo; diff --git a/foyer-memory/src/eviction/s3fifo.rs b/foyer-memory/src/eviction/s3fifo.rs index 0eb281aa..c32a1dc4 100644 --- a/foyer-memory/src/eviction/s3fifo.rs +++ b/foyer-memory/src/eviction/s3fifo.rs @@ -28,7 +28,7 @@ use foyer_common::{ use intrusive_collections::{intrusive_adapter, LinkedList, LinkedListAtomicLink}; use serde::{Deserialize, Serialize}; -use super::{Eviction, Operator}; +use super::{Eviction, Op}; use crate::record::{CacheHint, Record}; /// S3Fifo eviction algorithm config. @@ -293,29 +293,15 @@ where } } - fn acquire_operator() -> super::Operator { - Operator::Immutable - } - - fn acquire_immutable(&self, record: &Arc>) { - let state = unsafe { &mut *record.state().get() }; - state.inc_frequency(); - } - - fn acquire_mutable(&mut self, _record: &Arc>) { - unreachable!() - } - - fn release_operator() -> Operator { - Operator::Noop - } - - fn release_immutable(&self, _record: &Arc>) { - unreachable!() + fn acquire() -> Op { + Op::immutable(|_: &Self, record| { + let state = unsafe { &mut *record.state().get() }; + state.inc_frequency(); + }) } - fn release_mutable(&mut self, _record: &Arc>) { - unreachable!() + fn release() -> Op { + Op::noop() } } @@ -379,18 +365,18 @@ mod tests { use super::*; use crate::{ - eviction::test_utils::{assert_ptr_eq, assert_ptr_vec_vec_eq, TestEviction}, + eviction::test_utils::{assert_ptr_eq, assert_ptr_vec_vec_eq, Dump, OpExt}, record::Data, }; - impl TestEviction for S3Fifo + impl Dump for S3Fifo where K: Key + Clone, V: Value + Clone, { - type Dump = Vec>>>; + type Output = Vec>>>; - fn dump(&self) -> Self::Dump { + fn dump(&self) -> Self::Output { let mut small = vec![]; let mut main = vec![]; diff --git a/foyer-memory/src/eviction/test_utils.rs b/foyer-memory/src/eviction/test_utils.rs index a1d6f387..dfc18c4e 100644 --- a/foyer-memory/src/eviction/test_utils.rs +++ b/foyer-memory/src/eviction/test_utils.rs @@ -16,11 +16,45 @@ use std::sync::Arc; use itertools::Itertools; -use super::Eviction; +use super::{Eviction, Op}; +use crate::Record; -pub trait TestEviction: Eviction { - type Dump; - fn dump(&self) -> Self::Dump; +#[expect(dead_code)] +pub trait OpExt: Eviction { + fn acquire_immutable(&self, record: &Arc>) { + match Self::acquire() { + Op::Immutable(f) => f(self, record), + _ => unreachable!(), + } + } + + fn acquire_mutable(&mut self, record: &Arc>) { + match Self::acquire() { + Op::Mutable(mut f) => f(self, record), + _ => unreachable!(), + } + } + + fn release_immutable(&self, record: &Arc>) { + match Self::release() { + Op::Immutable(f) => f(self, record), + _ => unreachable!(), + } + } + + fn release_mutable(&mut self, record: &Arc>) { + match Self::release() { + Op::Mutable(mut f) => f(self, record), + _ => unreachable!(), + } + } +} + +impl OpExt for E where E: Eviction {} + +pub trait Dump: Eviction { + type Output; + fn dump(&self) -> Self::Output; } pub fn assert_ptr_eq(a: &Arc, b: &Arc) { diff --git a/foyer-memory/src/prelude.rs b/foyer-memory/src/prelude.rs index 3ce516b4..4e36ee44 100644 --- a/foyer-memory/src/prelude.rs +++ b/foyer-memory/src/prelude.rs @@ -16,7 +16,7 @@ pub use ahash::RandomState; pub use crate::{ cache::{Cache, CacheBuilder, CacheEntry, EvictionConfig, Fetch}, - eviction::{fifo::FifoConfig, lfu::LfuConfig, lru::LruConfig, s3fifo::S3FifoConfig}, + eviction::{fifo::FifoConfig, lfu::LfuConfig, lru::LruConfig, s3fifo::S3FifoConfig, Eviction, Op}, raw::{FetchMark, FetchState, Weighter}, record::{CacheHint, Record}, }; diff --git a/foyer-memory/src/raw.rs b/foyer-memory/src/raw.rs index fe9d2e5f..45ba0a40 100644 --- a/foyer-memory/src/raw.rs +++ b/foyer-memory/src/raw.rs @@ -43,7 +43,7 @@ use pin_project::pin_project; use tokio::{sync::oneshot, task::JoinHandle}; use crate::{ - eviction::{Eviction, Operator}, + eviction::{Eviction, Op}, indexer::{hash_table::HashTableIndexer, sentry::Sentry, Indexer}, record::{Data, Record}, }; @@ -253,22 +253,34 @@ where #[fastrace::trace(name = "foyer::memory::raw::shard::acquire_immutable")] fn acquire_immutable(&self, record: &Arc>) { - self.eviction.acquire_immutable(record); + match E::acquire() { + Op::Immutable(f) => f(&self.eviction, record), + _ => unreachable!(), + } } #[fastrace::trace(name = "foyer::memory::raw::shard::acquire_mutable")] fn acquire_mutable(&mut self, record: &Arc>) { - self.eviction.acquire_mutable(record); + match E::acquire() { + Op::Mutable(mut f) => f(&mut self.eviction, record), + _ => unreachable!(), + } } #[fastrace::trace(name = "foyer::memory::raw::shard::release_immutable")] fn release_immutable(&self, record: &Arc>) { - self.eviction.release_immutable(record); + match E::release() { + Op::Immutable(f) => f(&self.eviction, record), + _ => unreachable!(), + } } #[fastrace::trace(name = "foyer::memory::raw::shard::release_mutable")] fn release_mutable(&mut self, record: &Arc>) { - self.eviction.release_mutable(record); + match E::release() { + Op::Mutable(mut f) => f(&mut self.eviction, record), + _ => unreachable!(), + } } #[fastrace::trace(name = "foyer::memory::raw::shard::fetch_noop")] @@ -531,12 +543,12 @@ where { let hash = self.inner.hash_builder.hash_one(key); - let record = match E::acquire_operator() { - Operator::Noop => self.inner.shards[self.shard(hash)].read().get_noop(hash, key), - Operator::Immutable => self.inner.shards[self.shard(hash)] + let record = match E::acquire() { + Op::Noop => self.inner.shards[self.shard(hash)].read().get_noop(hash, key), + Op::Immutable(_) => self.inner.shards[self.shard(hash)] .read() .with(|shard| shard.get_immutable(hash, key)), - Operator::Mutable => self.inner.shards[self.shard(hash)] + Op::Mutable(_) => self.inner.shards[self.shard(hash)] .write() .with(|mut shard| shard.get_mutable(hash, key)), }?; @@ -566,14 +578,12 @@ where { let hash = self.inner.hash_builder.hash_one(key); - match E::acquire_operator() { - Operator::Noop => self.inner.shards[self.shard(hash)] - .read() - .with(|shard| shard.get_noop(hash, key)), - Operator::Immutable => self.inner.shards[self.shard(hash)] + match E::acquire() { + Op::Noop => self.inner.shards[self.shard(hash)].read().get_noop(hash, key), + Op::Immutable(_) => self.inner.shards[self.shard(hash)] .read() .with(|shard| shard.get_immutable(hash, key)), - Operator::Mutable => self.inner.shards[self.shard(hash)] + Op::Mutable(_) => self.inner.shards[self.shard(hash)] .write() .with(|mut shard| shard.get_mutable(hash, key)), } @@ -642,10 +652,10 @@ where let shard = &self.inner.shards[hash as usize % self.inner.shards.len()]; if self.record.dec_refs(1) == 0 { - match E::release_operator() { - Operator::Noop => {} - Operator::Immutable => shard.read().with(|shard| shard.release_immutable(&self.record)), - Operator::Mutable => shard.write().with(|mut shard| shard.release_mutable(&self.record)), + match E::release() { + Op::Noop => {} + Op::Immutable(_) => shard.read().with(|shard| shard.release_immutable(&self.record)), + Op::Mutable(_) => shard.write().with(|mut shard| shard.release_mutable(&self.record)), } if self.record.is_ephemeral() { @@ -870,10 +880,10 @@ where { let hash = self.inner.hash_builder.hash_one(&key); - let raw = match E::acquire_operator() { - Operator::Noop => self.inner.shards[self.shard(hash)].read().fetch_noop(hash, &key), - Operator::Immutable => self.inner.shards[self.shard(hash)].read().fetch_immutable(hash, &key), - Operator::Mutable => self.inner.shards[self.shard(hash)].write().fetch_mutable(hash, &key), + let raw = match E::acquire() { + Op::Noop => self.inner.shards[self.shard(hash)].read().fetch_noop(hash, &key), + Op::Immutable(_) => self.inner.shards[self.shard(hash)].read().fetch_immutable(hash, &key), + Op::Mutable(_) => self.inner.shards[self.shard(hash)].write().fetch_mutable(hash, &key), }; match raw {