Merge pull request #84 from moka-rs/remove-deq-region

Embed CacheRegion value into a TagNonNull pointer
moka-rs · Feb 3, 2022 · e54d991 · e54d991
2 parents 4e7d334 + 49853c4
commit e54d991
Show file tree

Hide file tree

Showing 12 changed files with 172 additions and 135 deletions.
diff --git a/.github/workflows/CI.yml b/.github/workflows/CI.yml
@@ -23,9 +23,7 @@ jobs:
         rust:
           - stable
           - beta
-          # - 1.46.0  # MSRV (future)
-          # - 1.45.2  # MSRV (no features)
-          - 1.49.0  # Temporary MSRV
+          - 1.51.0  # MSRV (for both no features and "future")
 
     steps:
       - name: Checkout Moka

diff --git a/.vscode/settings.json b/.vscode/settings.json
@@ -38,6 +38,7 @@
         "semver",
         "smallvec",
         "structs",
+        "tagptr",
         "Tatsuya",
         "thiserror",
         "toolchain",

diff --git a/Cargo.toml b/Cargo.toml
@@ -3,6 +3,7 @@ name = "moka"
 version = "0.7.2"
 authors = ["Tatsuya Kawano <tatsuya@hibaridb.org>"]
 edition = "2018"
+rust-version = "1.51"
 
 description = "A fast and concurrent cache library inspired by Caffeine (Java)"
 license = "MIT OR Apache-2.0"
@@ -42,6 +43,7 @@ parking_lot = "0.12"
 quanta = "0.9.3"
 scheduled-thread-pool = "0.2"
 smallvec = "1.8"
+tagptr = "0.2"
 thiserror = "1.0"
 triomphe = "0.1"
 uuid = { version = "0.8", features = ["v4"] }

diff --git a/README.md b/README.md
@@ -386,16 +386,17 @@ This crate's minimum supported Rust versions (MSRV) are the followings:
 
 | Feature    | Enabled by default? | MSRV        |
 |:-----------|:-------------------:|:-----------:|
-| no feature |                     | Rust 1.45.2 |
-| `atomic64` |       yes           | Rust 1.45.2 |
-| `future`   |                     | Rust 1.46.0 |
+| no feature |                     | Rust 1.51.0 |
+| `atomic64` |       yes           | Rust 1.51.0 |
+| `future`   |                     | Rust 1.51.0 |
 
 If only the default features are enabled, MSRV will be updated conservatively. When
 using other features, like `future`, MSRV might be updated more frequently, up to the
 latest stable. In both cases, increasing MSRV is _not_ considered a semver-breaking
 change.
 
 <!--
+- tagptr 0.2.0 requires 1.51.
 - socket2 0.4.0 requires 1.46.
 - quanta requires 1.45.
 - moka-cht requires 1.41.

diff --git a/src/common.rs b/src/common.rs
@@ -17,6 +17,41 @@ pub(crate) mod atomic_time;
 
 pub(crate) mod time;
 
+// Note: `CacheRegion` cannot have more than four enum variants. This is because
+// `crate::{sync,unsync}::DeqNodes` uses a `tagptr::TagNonNull<DeqNode<T>, 2>`
+// pointer, where the 2-bit tag is `CacheRegion`.
+#[derive(Clone, Copy, Debug, Eq)]
+pub(crate) enum CacheRegion {
+    Window = 0,
+    MainProbation = 1,
+    MainProtected = 2,
+    Other = 3,
+}
+
+impl From<usize> for CacheRegion {
+    fn from(n: usize) -> Self {
+        match n {
+            0 => Self::Window,
+            1 => Self::MainProbation,
+            2 => Self::MainProtected,
+            3 => Self::Other,
+            _ => panic!("No such CacheRegion variant for {}", n),
+        }
+    }
+}
+
+impl PartialEq<Self> for CacheRegion {
+    fn eq(&self, other: &Self) -> bool {
+        core::mem::discriminant(self) == core::mem::discriminant(other)
+    }
+}
+
+impl PartialEq<usize> for CacheRegion {
+    fn eq(&self, other: &usize) -> bool {
+        *self as usize == *other
+    }
+}
+
 // Ensures the value fits in a range of `128u32..=u32::MAX`.
 pub(crate) fn sketch_capacity(max_capacity: u64) -> u32 {
     max_capacity.try_into().unwrap_or(u32::MAX).max(128)

diff --git a/src/common/deque.rs b/src/common/deque.rs
@@ -14,17 +14,15 @@
 
 use std::{marker::PhantomData, ptr::NonNull};
 
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub(crate) enum CacheRegion {
-    Window,
-    MainProbation,
-    MainProtected,
-    WriteOrder,
-}
+use super::CacheRegion;
 
+// `crate::{sync,unsync}::DeqNodes` uses a `tagptr::TagNonNull<DeqNode<T>, 2>`
+// pointer. To reserve the space for the 2-bit tag, use 4 bytes as the *minimum*
+// alignment.
+// https://doc.rust-lang.org/reference/type-layout.html#the-alignment-modifiers
+#[repr(align(4))]
 #[derive(PartialEq, Eq)]
 pub(crate) struct DeqNode<T> {
-    pub(crate) region: CacheRegion,
     next: Option<NonNull<DeqNode<T>>>,
     prev: Option<NonNull<DeqNode<T>>>,
     pub(crate) element: T,
@@ -33,20 +31,18 @@ pub(crate) struct DeqNode<T> {
 impl<T> std::fmt::Debug for DeqNode<T> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.debug_struct("DeqNode")
-            .field("region", &self.region)
             .field("next", &self.next)
             .field("prev", &self.prev)
             .finish()
     }
 }
 
 impl<T> DeqNode<T> {
-    pub(crate) fn new(region: CacheRegion, element: T) -> Self {
+    pub(crate) fn new(element: T) -> Self {
         #[cfg(feature = "unstable-debug-counters")]
         crate::sync::debug_counters::InternalGlobalDebugCounters::deq_node_created();
 
         Self {
-            region,
             next: None,
             prev: None,
             element,
@@ -113,16 +109,16 @@ impl<T> Deque<T> {
         }
     }
 
-    pub(crate) fn region(&self) -> &CacheRegion {
-        &self.region
+    pub(crate) fn region(&self) -> CacheRegion {
+        self.region
     }
 
     pub(crate) fn len(&self) -> usize {
         self.len
     }
 
     pub(crate) fn contains(&self, node: &DeqNode<T>) -> bool {
-        self.region == node.region && (node.prev.is_some() || self.is_head(node))
+        node.prev.is_some() || self.is_head(node)
     }
 
     pub(crate) fn peek_front(&self) -> Option<&DeqNode<T>> {
@@ -229,8 +225,6 @@ impl<T> Deque<T> {
     ///
     /// Panics:
     pub(crate) unsafe fn unlink(&mut self, mut node: NonNull<DeqNode<T>>) {
-        assert_eq!(self.region, node.as_ref().region);
-
         if self.is_at_cursor(node.as_ref()) {
             self.advance_cursor();
         }
@@ -348,7 +342,7 @@ mod tests {
         assert!(deque.peek_back().is_none());
 
         // push_back(node1)
-        let node1 = DeqNode::new(MainProbation, "a".to_string());
+        let node1 = DeqNode::new("a".to_string());
         assert!(!deque.contains(&node1));
         let node1 = Box::new(node1);
         let node1_ptr = deque.push_back(node1);
@@ -384,7 +378,7 @@ mod tests {
         assert!(tail_a.next.is_none());
 
         // push_back(node2)
-        let node2 = DeqNode::new(MainProbation, "b".to_string());
+        let node2 = DeqNode::new("b".to_string());
         assert!(!deque.contains(&node2));
         let node2_ptr = deque.push_back(Box::new(node2));
         assert_eq!(deque.len(), 2);
@@ -459,7 +453,7 @@ mod tests {
         assert!(tail_c.next.is_none());
 
         // push_back(node3)
-        let node3 = DeqNode::new(MainProbation, "c".to_string());
+        let node3 = DeqNode::new("c".to_string());
         assert!(!deque.contains(&node3));
         let node3_ptr = deque.push_back(Box::new(node3));
         assert_eq!(deque.len(), 3);
@@ -601,11 +595,11 @@ mod tests {
         let mut deque: Deque<String> = Deque::new(MainProbation);
         assert!((&mut deque).next().is_none());
 
-        let node1 = DeqNode::new(MainProbation, "a".into());
+        let node1 = DeqNode::new("a".into());
         deque.push_back(Box::new(node1));
-        let node2 = DeqNode::new(MainProbation, "b".into());
+        let node2 = DeqNode::new("b".into());
         let node2_ptr = deque.push_back(Box::new(node2));
-        let node3 = DeqNode::new(MainProbation, "c".into());
+        let node3 = DeqNode::new("c".into());
         let node3_ptr = deque.push_back(Box::new(node3));
 
         // -------------------------------------------------------
@@ -653,7 +647,7 @@ mod tests {
 
         // -------------------------------------------------------
         // Try pop_front during iteration.
-        let node3 = DeqNode::new(MainProbation, "c".into());
+        let node3 = DeqNode::new("c".into());
         deque.push_back(Box::new(node3));
 
         assert_eq!((&mut deque).next(), Some(&"a".into()));
@@ -675,11 +669,11 @@ mod tests {
     fn next_node() {
         let mut deque: Deque<String> = Deque::new(MainProbation);
 
-        let node1 = DeqNode::new(MainProbation, "a".into());
+        let node1 = DeqNode::new("a".into());
         deque.push_back(Box::new(node1));
-        let node2 = DeqNode::new(MainProbation, "b".into());
+        let node2 = DeqNode::new("b".into());
         let node2_ptr = deque.push_back(Box::new(node2));
-        let node3 = DeqNode::new(MainProbation, "c".into());
+        let node3 = DeqNode::new("c".into());
         let node3_ptr = deque.push_back(Box::new(node3));
 
         // -------------------------------------------------------
@@ -731,10 +725,10 @@ mod tests {
         let mut deque: Deque<X> = Deque::new(MainProbation);
         let dropped = Rc::new(RefCell::new(Vec::default()));
 
-        let node1 = DeqNode::new(MainProbation, X(1, Rc::clone(&dropped)));
-        let node2 = DeqNode::new(MainProbation, X(2, Rc::clone(&dropped)));
-        let node3 = DeqNode::new(MainProbation, X(3, Rc::clone(&dropped)));
-        let node4 = DeqNode::new(MainProbation, X(4, Rc::clone(&dropped)));
+        let node1 = DeqNode::new(X(1, Rc::clone(&dropped)));
+        let node2 = DeqNode::new(X(2, Rc::clone(&dropped)));
+        let node3 = DeqNode::new(X(3, Rc::clone(&dropped)));
+        let node4 = DeqNode::new(X(4, Rc::clone(&dropped)));
         deque.push_back(Box::new(node1));
         deque.push_back(Box::new(node2));
         deque.push_back(Box::new(node3));

diff --git a/src/sync.rs b/src/sync.rs
@@ -4,6 +4,7 @@ use crate::common::{deque::DeqNode, time::Instant};
 
 use parking_lot::Mutex;
 use std::{ptr::NonNull, sync::Arc};
+use tagptr::TagNonNull;
 use triomphe::Arc as TrioArc;
 
 pub(crate) mod base_cache;
@@ -172,7 +173,7 @@ impl<K> AccessTime for DeqNode<KeyHashDate<K>> {
 }
 
 // DeqNode for an access order queue.
-type KeyDeqNodeAo<K> = NonNull<DeqNode<KeyHashDate<K>>>;
+type KeyDeqNodeAo<K> = TagNonNull<DeqNode<KeyHashDate<K>>, 2>;
 
 // DeqNode for the write order queue.
 type KeyDeqNodeWo<K> = NonNull<DeqNode<KeyDate<K>>>;

diff --git a/src/sync/base_cache.rs b/src/sync/base_cache.rs
@@ -14,9 +14,10 @@ use crate::{
     common::{
         self,
         atomic_time::AtomicInstant,
-        deque::{CacheRegion, DeqNode, Deque},
+        deque::{DeqNode, Deque},
         frequency_sketch::FrequencySketch,
         time::{CheckedTimeOps, Clock, Instant},
+        CacheRegion,
     },
     PredicateError,
 };
@@ -1097,10 +1098,12 @@ where
         if entry.is_admitted() {
             entry.set_is_admitted(false);
             counters.saturating_sub(1, entry.policy_weight());
+            // The following two unlink_* functions will unset the deq nodes.
             deqs.unlink_ao(&entry);
             Deques::unlink_wo(&mut deqs.write_order, &entry);
+        } else {
+            entry.unset_q_nodes();
         }
-        entry.unset_q_nodes();
     }
 
     fn handle_remove_with_deques(
@@ -1113,10 +1116,12 @@ where
         if entry.is_admitted() {
             entry.set_is_admitted(false);
             counters.saturating_sub(1, entry.policy_weight());
+            // The following two unlink_* functions will unset the deq nodes.
             Deques::unlink_ao_from_deque(ao_deq_name, ao_deq, &entry);
             Deques::unlink_wo(wo_deq, &entry);
+        } else {
+            entry.unset_q_nodes();
         }
-        entry.unset_q_nodes();
     }
 
     fn evict_expired(