Do not copy internal pointers on updating an entry (v0.9.x)

src/common/concurrent.rs

@@ -1,6 +1,5 @@
 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;
@@ -62,11 +61,11 @@ impl<K> Clone for KeyHash<K> {
 
 pub(crate) struct KeyDate<K> {
     key: Arc<K>,
-    entry_info: TrioArc<EntryInfo>,
+    entry_info: TrioArc<EntryInfo<K>>,
 }
 
 impl<K> KeyDate<K> {
-    pub(crate) fn new(key: Arc<K>, entry_info: &TrioArc<EntryInfo>) -> Self {
+    pub(crate) fn new(key: Arc<K>, entry_info: &TrioArc<EntryInfo<K>>) -> Self {
         Self {
             key,
             entry_info: TrioArc::clone(entry_info),
@@ -91,11 +90,11 @@ impl<K> KeyDate<K> {
 pub(crate) struct KeyHashDate<K> {
     key: Arc<K>,
     hash: u64,
-    entry_info: TrioArc<EntryInfo>,
+    entry_info: TrioArc<EntryInfo<K>>,
 }
 
 impl<K> KeyHashDate<K> {
-    pub(crate) fn new(kh: KeyHash<K>, entry_info: &TrioArc<EntryInfo>) -> Self {
+    pub(crate) fn new(kh: KeyHash<K>, entry_info: &TrioArc<EntryInfo<K>>) -> Self {
         Self {
             key: kh.key,
             hash: kh.hash,
@@ -111,7 +110,7 @@ impl<K> KeyHashDate<K> {
         self.hash
     }
 
-    pub(crate) fn entry_info(&self) -> &EntryInfo {
+    pub(crate) fn entry_info(&self) -> &EntryInfo<K> {
         &self.entry_info
     }
 }
@@ -172,59 +171,28 @@ impl<K> AccessTime for DeqNode<KeyHashDate<K>> {
 }
 
 // DeqNode for an access order queue.
-type KeyDeqNodeAo<K> = TagNonNull<DeqNode<KeyHashDate<K>>, 2>;
+pub(crate) type KeyDeqNodeAo<K> = TagNonNull<DeqNode<KeyHashDate<K>>, 2>;
 
 // DeqNode for the write order queue.
-type KeyDeqNodeWo<K> = NonNull<DeqNode<KeyDate<K>>>;
-
-pub(crate) struct DeqNodes<K> {
-    access_order_q_node: Option<KeyDeqNodeAo<K>>,
-    write_order_q_node: Option<KeyDeqNodeWo<K>>,
-}
-
-// We need this `unsafe impl` as DeqNodes have NonNull pointers.
-unsafe impl<K> Send for DeqNodes<K> {}
+pub(crate) type KeyDeqNodeWo<K> = NonNull<DeqNode<KeyDate<K>>>;
 
 pub(crate) struct ValueEntry<K, V> {
     pub(crate) value: V,
-    info: TrioArc<EntryInfo>,
-    nodes: Mutex<DeqNodes<K>>,
+    info: TrioArc<EntryInfo<K>>,
 }
 
 impl<K, V> ValueEntry<K, V> {
-    pub(crate) fn new(value: V, entry_info: TrioArc<EntryInfo>) -> Self {
-        #[cfg(feature = "unstable-debug-counters")]
-        self::debug_counters::InternalGlobalDebugCounters::value_entry_created();
-
-        Self {
-            value,
-            info: entry_info,
-            nodes: Mutex::new(DeqNodes {
-                access_order_q_node: None,
-                write_order_q_node: None,
-            }),
-        }
-    }
-
-    pub(crate) fn new_from(value: V, entry_info: TrioArc<EntryInfo>, other: &Self) -> Self {
+    pub(crate) fn new(value: V, entry_info: TrioArc<EntryInfo<K>>) -> Self {
         #[cfg(feature = "unstable-debug-counters")]
         self::debug_counters::InternalGlobalDebugCounters::value_entry_created();
 
-        let nodes = {
-            let other_nodes = other.nodes.lock();
-            DeqNodes {
-                access_order_q_node: other_nodes.access_order_q_node,
-                write_order_q_node: other_nodes.write_order_q_node,
-            }
-        };
         Self {
             value,
             info: entry_info,
-            nodes: Mutex::new(nodes),
         }
     }
 
-    pub(crate) fn entry_info(&self) -> &TrioArc<EntryInfo> {
+    pub(crate) fn entry_info(&self) -> &TrioArc<EntryInfo<K>> {
         &self.info
     }
 
@@ -250,33 +218,31 @@ impl<K, V> ValueEntry<K, V> {
     }
 
     pub(crate) fn access_order_q_node(&self) -> Option<KeyDeqNodeAo<K>> {
-        self.nodes.lock().access_order_q_node
+        self.info.access_order_q_node()
     }
 
     pub(crate) fn set_access_order_q_node(&self, node: Option<KeyDeqNodeAo<K>>) {
-        self.nodes.lock().access_order_q_node = node;
+        self.info.set_access_order_q_node(node);
     }
 
     pub(crate) fn take_access_order_q_node(&self) -> Option<KeyDeqNodeAo<K>> {
-        self.nodes.lock().access_order_q_node.take()
+        self.info.take_access_order_q_node()
     }
 
     pub(crate) fn write_order_q_node(&self) -> Option<KeyDeqNodeWo<K>> {
-        self.nodes.lock().write_order_q_node
+        self.info.write_order_q_node()
     }
 
     pub(crate) fn set_write_order_q_node(&self, node: Option<KeyDeqNodeWo<K>>) {
-        self.nodes.lock().write_order_q_node = node;
+        self.info.set_write_order_q_node(node);
     }
 
     pub(crate) fn take_write_order_q_node(&self) -> Option<KeyDeqNodeWo<K>> {
-        self.nodes.lock().write_order_q_node.take()
+        self.info.take_write_order_q_node()
     }
 
     pub(crate) fn unset_q_nodes(&self) {
-        let mut nodes = self.nodes.lock();
-        nodes.access_order_q_node = None;
-        nodes.write_order_q_node = None;
+        self.info.unset_q_nodes();
     }
 }
 

src/common/concurrent/entry_info.rs

@@ -1,9 +1,19 @@
 use std::sync::atomic::{AtomicBool, AtomicU32, Ordering};
 
-use super::AccessTime;
+use super::{AccessTime, KeyDeqNodeAo, KeyDeqNodeWo};
 use crate::common::{concurrent::atomic_time::AtomicInstant, time::Instant};
 
-pub(crate) struct EntryInfo {
+use parking_lot::Mutex;
+
+pub(crate) struct DeqNodes<K> {
+    access_order_q_node: Option<KeyDeqNodeAo<K>>,
+    write_order_q_node: Option<KeyDeqNodeWo<K>>,
+}
+
+// We need this `unsafe impl` as DeqNodes have NonNull pointers.
+unsafe impl<K> Send for DeqNodes<K> {}
+
+pub(crate) struct EntryInfo<K> {
     /// `is_admitted` indicates that the entry has been admitted to the
     /// cache. When `false`, it means the entry is _temporary_ admitted to
     /// the cache or evicted from the cache (so it should not have LRU nodes).
@@ -15,9 +25,10 @@ pub(crate) struct EntryInfo {
     last_accessed: AtomicInstant,
     last_modified: AtomicInstant,
     policy_weight: AtomicU32,
+    nodes: Mutex<DeqNodes<K>>,
 }
 
-impl EntryInfo {
+impl<K> EntryInfo<K> {
     #[inline]
     pub(crate) fn new(timestamp: Instant, policy_weight: u32) -> Self {
         #[cfg(feature = "unstable-debug-counters")]
@@ -29,6 +40,10 @@ impl EntryInfo {
             last_accessed: AtomicInstant::new(timestamp),
             last_modified: AtomicInstant::new(timestamp),
             policy_weight: AtomicU32::new(policy_weight),
+            nodes: Mutex::new(DeqNodes {
+                access_order_q_node: None,
+                write_order_q_node: None,
+            }),
         }
     }
 
@@ -60,16 +75,46 @@ impl EntryInfo {
     pub(crate) fn set_policy_weight(&self, size: u32) {
         self.policy_weight.store(size, Ordering::Release);
     }
+
+    pub(crate) fn access_order_q_node(&self) -> Option<KeyDeqNodeAo<K>> {
+        self.nodes.lock().access_order_q_node
+    }
+
+    pub(crate) fn set_access_order_q_node(&self, node: Option<KeyDeqNodeAo<K>>) {
+        self.nodes.lock().access_order_q_node = node;
+    }
+
+    pub(crate) fn take_access_order_q_node(&self) -> Option<KeyDeqNodeAo<K>> {
+        self.nodes.lock().access_order_q_node.take()
+    }
+
+    pub(crate) fn write_order_q_node(&self) -> Option<KeyDeqNodeWo<K>> {
+        self.nodes.lock().write_order_q_node
+    }
+
+    pub(crate) fn set_write_order_q_node(&self, node: Option<KeyDeqNodeWo<K>>) {
+        self.nodes.lock().write_order_q_node = node;
+    }
+
+    pub(crate) fn take_write_order_q_node(&self) -> Option<KeyDeqNodeWo<K>> {
+        self.nodes.lock().write_order_q_node.take()
+    }
+
+    pub(crate) fn unset_q_nodes(&self) {
+        let mut nodes = self.nodes.lock();
+        nodes.access_order_q_node = None;
+        nodes.write_order_q_node = None;
+    }
 }
 
 #[cfg(feature = "unstable-debug-counters")]
-impl Drop for EntryInfo {
+impl<K> Drop for EntryInfo<K> {
     fn drop(&mut self) {
         super::debug_counters::InternalGlobalDebugCounters::entry_info_dropped();
     }
 }
 
-impl AccessTime for EntryInfo {
+impl<K> AccessTime for EntryInfo<K> {
     #[inline]
     fn last_accessed(&self) -> Option<Instant> {
         self.last_accessed.instant()
@@ -135,12 +180,12 @@ mod test {
         };
 
         let expected_sizes = match (arch, is_quanta_enabled) {
-            (Linux64, true) => vec![("1.51", 24)],
-            (Linux32, true) => vec![("1.51", 24)],
-            (MacOS64, true) => vec![("1.62", 24)],
-            (Linux64, false) => vec![("1.66", 56), ("1.51", 72)],
-            (Linux32, false) => vec![("1.66", 56), ("1.62", 72), ("1.51", 40)],
-            (MacOS64, false) => vec![("1.62", 56)],
+            (Linux64, true) => vec![("1.60", 48)],
+            (Linux32, true) => vec![("1.60", 40)],
+            (MacOS64, true) => vec![("1.62", 48)],
+            (Linux64, false) => vec![("1.66", 80), ("1.60", 96)],
+            (Linux32, false) => vec![("1.66", 72)],
+            (MacOS64, false) => vec![("1.62", 80)],
         };
 
         let mut expected = None;
@@ -152,7 +197,7 @@ mod test {
         }
 
         if let Some(size) = expected {
-            assert_eq!(size_of::<EntryInfo>(), size);
+            assert_eq!(size_of::<EntryInfo<()>>(), size);
         } else {
             panic!("No expected size for {:?} with Rust version {}", arch, ver);
         }

src/dash/base_cache.rs

@@ -275,9 +275,10 @@ where
             // Update
             .and_modify(|entry| {
                 // NOTES on `new_value_entry_from` method:
-                // 1. The internal EntryInfo will be shared between the old and new ValueEntries.
-                // 2. This method will set the last_accessed and last_modified to the max value to
-                //    prevent this new ValueEntry from being evicted by an expiration policy.
+                // 1. The internal EntryInfo will be shared between the old and new
+                //    ValueEntries.
+                // 2. This method will set the dirty flag to prevent this new
+                //    ValueEntry from being evicted by an expiration policy.
                 // 3. This method will update the policy_weight with the new weight.
                 let old_weight = entry.policy_weight();
                 *entry = self.new_value_entry_from(value.clone(), ts, weight, entry);
@@ -333,7 +334,7 @@ where
         info.set_last_accessed(timestamp);
         info.set_last_modified(timestamp);
         info.set_policy_weight(policy_weight);
-        TrioArc::new(ValueEntry::new_from(value, info, other))
+        TrioArc::new(ValueEntry::new(value, info))
     }
 
     #[inline]
@@ -1158,7 +1159,7 @@ where
             if let Some((_k, entry)) = maybe_entry {
                 Self::handle_remove(deqs, entry, counters);
             } else if let Some(entry) = self.cache.get(key) {
-                if entry.last_modified().is_none() {
+                if entry.is_dirty() {
                     deqs.move_to_back_ao(&entry);
                     deqs.move_to_back_wo(&entry);
                 } else {

src/sync_base/base_cache.rs

@@ -475,9 +475,10 @@ where
             // on_modify
             |_k, old_entry| {
                 // NOTES on `new_value_entry_from` method:
-                // 1. The internal EntryInfo will be shared between the old and new ValueEntries.
-                // 2. This method will set the last_accessed and last_modified to the max value to
-                //    prevent this new ValueEntry from being evicted by an expiration policy.
+                // 1. The internal EntryInfo will be shared between the old and new
+                //    ValueEntries.
+                // 2. This method will set the dirty flag to prevent this new
+                //    ValueEntry from being evicted by an expiration policy.
                 // 3. This method will update the policy_weight with the new weight.
                 let old_weight = old_entry.policy_weight();
                 let old_timestamps = (old_entry.last_accessed(), old_entry.last_modified());
@@ -501,7 +502,6 @@ where
         match (op1, op2) {
             (Some((_cnt, ins_op)), None) => (ins_op, ts),
             (None, Some((_cnt, old_entry, (old_last_accessed, old_last_modified), upd_op))) => {
-                old_entry.unset_q_nodes();
                 if self.is_removal_notifier_enabled() {
                     self.inner
                         .notify_upsert(key, &old_entry, old_last_accessed, old_last_modified);
@@ -516,7 +516,6 @@ where
                 if cnt1 > cnt2 {
                     (ins_op, ts)
                 } else {
-                    old_entry.unset_q_nodes();
                     if self.is_removal_notifier_enabled() {
                         self.inner.notify_upsert(
                             key,
@@ -559,7 +558,7 @@ where
         info.set_last_accessed(timestamp);
         info.set_last_modified(timestamp);
         info.set_policy_weight(policy_weight);
-        TrioArc::new(ValueEntry::new_from(value, info, other))
+        TrioArc::new(ValueEntry::new(value, info))
     }
 
     #[inline]
@@ -1733,7 +1732,7 @@ where
                 }
                 Self::handle_remove(deqs, entry, &mut eviction_state.counters);
             } else if let Some(entry) = self.cache.get(hash, |k| k == key) {
-                if entry.last_modified().is_none() {
+                if entry.is_dirty() {
                     deqs.move_to_back_ao(&entry);
                     deqs.move_to_back_wo(&entry);
                 } else {