From 3aa689d69845ec52a4459463d9fc6daf3d910bf4 Mon Sep 17 00:00:00 2001 From: Croxx Date: Wed, 10 Apr 2024 15:09:49 +0800 Subject: [PATCH] refactor: remove unstable feature let_chains and lint_reasons (#315) Signed-off-by: MrCroxx --- foyer-common/src/code.rs | 6 ++++-- foyer-common/src/lib.rs | 1 - foyer-experimental/src/lib.rs | 1 - foyer-intrusive/src/lib.rs | 4 ++-- foyer-memory/src/eviction/s3fifo.rs | 12 ++++++------ foyer-memory/src/eviction/test_utils.rs | 3 ++- foyer-memory/src/generic.rs | 18 ++++++++++++------ foyer-memory/src/lib.rs | 3 --- foyer-storage-bench/src/main.rs | 23 +++++++++++------------ foyer-storage/src/admission/mod.rs | 3 +-- foyer-storage/src/buffer.rs | 3 ++- foyer-storage/src/catalog.rs | 22 ++++++++++++---------- foyer-storage/src/generic.rs | 6 ++++-- foyer-storage/src/lib.rs | 2 -- foyer-storage/src/region.rs | 9 ++++++--- foyer-storage/src/reinsertion/mod.rs | 3 +-- foyer-storage/src/test_utils.rs | 9 ++++++++- foyer-storage/tests/storage_test.rs | 4 ++-- 18 files changed, 73 insertions(+), 59 deletions(-) diff --git a/foyer-common/src/code.rs b/foyer-common/src/code.rs index 3e5c3ba3..dda8e00a 100644 --- a/foyer-common/src/code.rs +++ b/foyer-common/src/code.rs @@ -95,7 +95,8 @@ pub trait Cursor: Send + Sync + 'static + std::io::Read + std::fmt::Debug { /// [`Key`] is required to implement [`Clone`]. /// /// If cloning a [`Key`] is expensive, wrap it with [`Arc`]. -#[expect(unused_variables)] +// TODO(MrCroxx): use `expect` after `lint_reasons` is stable. +#[allow(unused_variables)] pub trait Key: Sized + Send + Sync + 'static + std::hash::Hash + Eq + PartialEq + Ord + PartialOrd + std::fmt::Debug + Clone { @@ -122,7 +123,8 @@ pub trait Key: /// [`Value`] is required to implement [`Clone`]. /// /// If cloning a [`Value`] is expensive, wrap it with [`Arc`]. -#[expect(unused_variables)] +// TODO(MrCroxx): use `expect` after `lint_reasons` is stable. +#[allow(unused_variables)] pub trait Value: Sized + Send + Sync + 'static + std::fmt::Debug + Clone { type Cursor: Cursor = UnimplementedCursor; diff --git a/foyer-common/src/lib.rs b/foyer-common/src/lib.rs index 1681a227..a9541ec9 100644 --- a/foyer-common/src/lib.rs +++ b/foyer-common/src/lib.rs @@ -13,7 +13,6 @@ // limitations under the License. #![feature(trait_alias)] -#![feature(lint_reasons)] #![feature(bound_map)] #![feature(associated_type_defaults)] #![feature(cfg_match)] diff --git a/foyer-experimental/src/lib.rs b/foyer-experimental/src/lib.rs index 1d8ca5e2..ec80631e 100644 --- a/foyer-experimental/src/lib.rs +++ b/foyer-experimental/src/lib.rs @@ -13,7 +13,6 @@ // limitations under the License. #![feature(cfg_match)] -#![feature(lint_reasons)] #![feature(error_generic_member_access)] #![feature(lazy_cell)] diff --git a/foyer-intrusive/src/lib.rs b/foyer-intrusive/src/lib.rs index 074ed6ba..6d6fbc28 100644 --- a/foyer-intrusive/src/lib.rs +++ b/foyer-intrusive/src/lib.rs @@ -14,9 +14,9 @@ #![feature(ptr_metadata)] #![feature(trait_alias)] -#![feature(lint_reasons)] #![feature(offset_of)] -#![expect(clippy::new_without_default)] +// TODO(MrCroxx): use `expect` after `lint_reasons` is stable. +#![allow(clippy::new_without_default)] pub use memoffset::offset_of; diff --git a/foyer-memory/src/eviction/s3fifo.rs b/foyer-memory/src/eviction/s3fifo.rs index c9f54e8a..e8911494 100644 --- a/foyer-memory/src/eviction/s3fifo.rs +++ b/foyer-memory/src/eviction/s3fifo.rs @@ -146,13 +146,13 @@ where V: Value, { unsafe fn evict(&mut self) -> Option as Eviction>::Handle>> { - if self.small_charges > self.small_capacity - && let Some(ptr) = self.evict_small() - { - Some(ptr) - } else { - self.evict_main() + // TODO(MrCroxx): Use `let_chains` here after it is stable. + if self.small_charges > self.small_capacity { + if let Some(ptr) = self.evict_small() { + return Some(ptr); + } } + self.evict_main() } unsafe fn evict_small(&mut self) -> Option as Eviction>::Handle>> { diff --git a/foyer-memory/src/eviction/test_utils.rs b/foyer-memory/src/eviction/test_utils.rs index 39b57347..adf478de 100644 --- a/foyer-memory/src/eviction/test_utils.rs +++ b/foyer-memory/src/eviction/test_utils.rs @@ -15,7 +15,8 @@ use super::Eviction; use crate::handle::Handle; -#[expect(clippy::type_complexity)] +// TODO(MrCroxx): use `expect` after `lint_reasons` is stable. +#[allow(clippy::type_complexity)] pub trait TestEviction: Eviction { fn dump(&self) -> Vec<(::Key, ::Value)>; } diff --git a/foyer-memory/src/generic.rs b/foyer-memory/src/generic.rs index 6352c368..5ac43267 100644 --- a/foyer-memory/src/generic.rs +++ b/foyer-memory/src/generic.rs @@ -43,7 +43,8 @@ struct CacheSharedState { listener: L, } -#[expect(clippy::type_complexity)] +// TODO(MrCroxx): use `expect` after `lint_reasons` is stable. +#[allow(clippy::type_complexity)] struct CacheShard where K: Key, @@ -198,9 +199,12 @@ where } unsafe fn evict(&mut self, charge: usize, last_reference_entries: &mut Vec<(K, V, H::Context, usize)>) { - while self.usage.load(Ordering::Relaxed) + charge > self.capacity - && let Some(evicted) = self.eviction.pop() - { + // TODO(MrCroxx): Use `let_chains` here after it is stable. + while self.usage.load(Ordering::Relaxed) + charge > self.capacity { + let evicted = match self.eviction.pop() { + Some(evicted) => evicted, + None => break, + }; self.state.metrics.evict.fetch_add(1, Ordering::Relaxed); let base = evicted.as_ref().base(); debug_assert!(base.is_in_indexer()); @@ -304,7 +308,8 @@ where pub event_listener: L, } -#[expect(clippy::type_complexity)] +// TODO(MrCroxx): use `expect` after `lint_reasons` is stable. +#[allow(clippy::type_complexity)] pub enum GenericEntry where K: Key, @@ -364,7 +369,8 @@ where } } -#[expect(clippy::type_complexity)] +// TODO(MrCroxx): use `expect` after `lint_reasons` is stable. +#[allow(clippy::type_complexity)] pub struct GenericCache where K: Key, diff --git a/foyer-memory/src/lib.rs b/foyer-memory/src/lib.rs index aa073eb3..3b92ad5a 100644 --- a/foyer-memory/src/lib.rs +++ b/foyer-memory/src/lib.rs @@ -12,9 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#![feature(let_chains)] -#![feature(lint_reasons)] - //! This crate provides a concurrent in-memory cache component that supports replaceable eviction algorithm. //! //! # Motivation diff --git a/foyer-storage-bench/src/main.rs b/foyer-storage-bench/src/main.rs index c2efeb34..acd59aab 100644 --- a/foyer-storage-bench/src/main.rs +++ b/foyer-storage-bench/src/main.rs @@ -12,9 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -#![feature(let_chains)] -#![feature(lint_reasons)] - mod analyze; mod export; mod rate; @@ -754,13 +751,14 @@ async fn write( } let idx = id + step * c; - // TODO(MrCroxx): Use random content? let entry_size = OsRng.gen_range(context.entry_size_range.clone()); let data = Arc::new(text(idx as usize, entry_size)); - if let Some(limiter) = &mut limiter - && let Some(wait) = limiter.consume(entry_size as f64) - { - tokio::time::sleep(wait).await; + + // TODO(MrCroxx): Use `let_chains` here after it is stable. + if let Some(limiter) = &mut limiter { + if let Some(wait) = limiter.consume(entry_size as f64) { + tokio::time::sleep(wait).await; + } } let time = Instant::now(); @@ -849,10 +847,11 @@ async fn read( } context.metrics.get_bytes.fetch_add(entry_size, Ordering::Relaxed); - if let Some(limiter) = &mut limiter - && let Some(wait) = limiter.consume(entry_size as f64) - { - tokio::time::sleep(wait).await; + // TODO(MrCroxx): Use `let_chains` here after it is stable. + if let Some(limiter) = &mut limiter { + if let Some(wait) = limiter.consume(entry_size as f64) { + tokio::time::sleep(wait).await; + } } } else { if let Err(e) = context.metrics.get_miss_lats.write().record(lat) { diff --git a/foyer-storage/src/admission/mod.rs b/foyer-storage/src/admission/mod.rs index dcff9e98..406ed732 100644 --- a/foyer-storage/src/admission/mod.rs +++ b/foyer-storage/src/admission/mod.rs @@ -41,12 +41,11 @@ where } } -#[expect(unused_variables)] pub trait AdmissionPolicy: Send + Sync + 'static + Debug { type Key: Key; type Value: Value; - fn init(&self, context: AdmissionContext) {} + fn init(&self, context: AdmissionContext); fn judge(&self, key: &Self::Key, weight: usize) -> bool; diff --git a/foyer-storage/src/buffer.rs b/foyer-storage/src/buffer.rs index dfc0c179..8090ef7e 100644 --- a/foyer-storage/src/buffer.rs +++ b/foyer-storage/src/buffer.rs @@ -179,7 +179,8 @@ where /// # Format /// /// | header | value (compressed) | key | | - #[expect(clippy::uninit_vec)] + // TODO(MrCroxx): use `expect` after `lint_reasons` is stable. + #[allow(clippy::uninit_vec)] pub async fn write( &mut self, Entry { diff --git a/foyer-storage/src/catalog.rs b/foyer-storage/src/catalog.rs index 3177995b..bbc9c749 100644 --- a/foyer-storage/src/catalog.rs +++ b/foyer-storage/src/catalog.rs @@ -130,12 +130,13 @@ where item.inserted = Some(Instant::now()); guard.insert(key.clone(), item) }; - if let Some(old) = old - && let Index::Inflight { .. } = old.index() - { - self.metrics - .inner_op_duration_entry_flush - .observe(old.inserted.unwrap().elapsed().as_secs_f64()); + // TODO(MrCroxx): Use `let_chains` here after it is stable. + if let Some(old) = old { + if let Index::Inflight { .. } = old.index() { + self.metrics + .inner_op_duration_entry_flush + .observe(old.inserted.unwrap().elapsed().as_secs_f64()); + } } } @@ -147,10 +148,11 @@ where pub fn remove(&self, key: &K) -> Option> { let shard = self.shard(key); let info: Option> = self.items[shard].write().remove(key); - if let Some(info) = &info - && let Index::Region { view } = &info.index - { - self.regions[*view.id() as usize].lock().remove(key); + // TODO(MrCroxx): Use `let_chains` here after it is stable. + if let Some(info) = &info { + if let Index::Region { view } = &info.index { + self.regions[*view.id() as usize].lock().remove(key); + } } info } diff --git a/foyer-storage/src/generic.rs b/foyer-storage/src/generic.rs index 490c3915..51522814 100644 --- a/foyer-storage/src/generic.rs +++ b/foyer-storage/src/generic.rs @@ -236,7 +236,8 @@ where let (flushers_stop_tx, _) = broadcast::channel(DEFAULT_BROADCAST_CAPACITY); let flusher_stop_rxs = (0..config.flushers).map(|_| flushers_stop_tx.subscribe()).collect_vec(); - #[expect(clippy::type_complexity)] + // TODO(MrCroxx): use `expect` after `lint_reasons` is stable. + #[allow(clippy::type_complexity)] let (flusher_entry_txs, flusher_entry_rxs): ( Vec>>, Vec>>, @@ -1081,7 +1082,8 @@ mod tests { type TestStoreConfig = GenericStoreConfig, FsDevice, Fifo>>; #[tokio::test] - #[expect(clippy::identity_op)] + // TODO(MrCroxx): use `expect` after `lint_reasons` is stable. + #[allow(clippy::identity_op)] async fn test_recovery() { const KB: usize = 1024; const MB: usize = 1024 * 1024; diff --git a/foyer-storage/src/lib.rs b/foyer-storage/src/lib.rs index 2799f01b..7e2cf9a1 100644 --- a/foyer-storage/src/lib.rs +++ b/foyer-storage/src/lib.rs @@ -16,10 +16,8 @@ #![feature(strict_provenance)] #![feature(trait_alias)] #![feature(get_mut_unchecked)] -#![feature(let_chains)] #![feature(error_generic_member_access)] #![feature(lazy_cell)] -#![feature(lint_reasons)] #![feature(box_into_inner)] #![feature(try_trait_v2)] #![feature(offset_of)] diff --git a/foyer-storage/src/region.rs b/foyer-storage/src/region.rs index 94a7dc37..4280d395 100644 --- a/foyer-storage/src/region.rs +++ b/foyer-storage/src/region.rs @@ -105,7 +105,8 @@ pub struct RegionInner where A: BufferAllocator, { - #[expect(clippy::type_complexity)] + // TODO(MrCroxx): use `expect` after `lint_reasons` is stable. + #[allow(clippy::type_complexity)] waits: BTreeMap<(usize, usize), Vec>>>>>, } @@ -152,7 +153,8 @@ where } /// Load region data by view from device. - #[expect(clippy::type_complexity)] + // TODO(MrCroxx): use `expect` after `lint_reasons` is stable. + #[allow(clippy::type_complexity)] #[tracing::instrument(skip(self, view))] pub async fn load(&self, view: RegionView) -> Result>>> { let res = self @@ -164,7 +166,8 @@ where } /// Load region data with given `range` from device. - #[expect(clippy::type_complexity)] + // TODO(MrCroxx): use `expect` after `lint_reasons` is stable. + #[allow(clippy::type_complexity)] #[tracing::instrument(skip(self, range), fields(start, end))] pub async fn load_range( &self, diff --git a/foyer-storage/src/reinsertion/mod.rs b/foyer-storage/src/reinsertion/mod.rs index 493c941f..7681863e 100644 --- a/foyer-storage/src/reinsertion/mod.rs +++ b/foyer-storage/src/reinsertion/mod.rs @@ -41,12 +41,11 @@ where } } -#[expect(unused_variables)] pub trait ReinsertionPolicy: Send + Sync + 'static + Debug { type Key: Key; type Value: Value; - fn init(&self, context: ReinsertionContext) {} + fn init(&self, context: ReinsertionContext); fn judge(&self, key: &Self::Key, weight: usize) -> bool; diff --git a/foyer-storage/src/test_utils.rs b/foyer-storage/src/test_utils.rs index 25fffc85..1c155ca0 100644 --- a/foyer-storage/src/test_utils.rs +++ b/foyer-storage/src/test_utils.rs @@ -17,7 +17,10 @@ use std::{collections::HashSet, marker::PhantomData}; use foyer_common::code::{Key, Value}; use parking_lot::Mutex; -use crate::{admission::AdmissionPolicy, reinsertion::ReinsertionPolicy}; +use crate::{ + admission::{AdmissionContext, AdmissionPolicy}, + reinsertion::{ReinsertionContext, ReinsertionPolicy}, +}; #[derive(Debug, Clone)] pub enum Record { @@ -83,6 +86,8 @@ where type Value = V; + fn init(&self, _: AdmissionContext) {} + fn judge(&self, key: &K, _weight: usize) -> bool { self.records.lock().push(Record::Admit(key.clone())); true @@ -102,6 +107,8 @@ where type Value = V; + fn init(&self, _: ReinsertionContext) {} + fn judge(&self, key: &K, _weight: usize) -> bool { self.records.lock().push(Record::Evict(key.clone())); false diff --git a/foyer-storage/tests/storage_test.rs b/foyer-storage/tests/storage_test.rs index 97bc7143..3910f881 100644 --- a/foyer-storage/tests/storage_test.rs +++ b/foyer-storage/tests/storage_test.rs @@ -12,8 +12,8 @@ // See the License for the specific language governing permissions and // limitations under the License. -#![feature(lint_reasons)] -#![expect(clippy::identity_op)] +// TODO(MrCroxx): use `expect` after `lint_reasons` is stable. +#![allow(clippy::identity_op)] use std::{path::PathBuf, sync::Arc, time::Duration};