From 81d6166d215b86fa3d349c5e73596dcbddd6d0ad Mon Sep 17 00:00:00 2001 From: kyren Date: Sat, 6 Jul 2019 09:06:04 -0400 Subject: [PATCH] Simplify LruCache, expose the entry API instead of a `produce` API There is a problem with the entry API which also exists with the `produce` API, and that is it is difficult to maintain the correct cache size when producing a new value. When implemented naively, after the mutable reference to newly produced value is obtained, the LruCache must be shrunk to fit the space requirements. However, this is not safe to do, as the returned reference must not be mutated or moved while it is alive, so we cannot (in general, safely) shrink the underlying map after obtaining this reference. Less naively, you could hash the provided key and use the raw entry API to look up the key, return an entry wrapping a raw entry if it is found, and otherwise shrink the map by 1 and use the raw api to look up an entry again (using the same computed hash). This way, you would not exceed the cache capacity and hits would be very fast, but a miss would incur two hash lookups (but not necessarily two key hashes due to the raw API allowing pre-computed hashes). The same basic issue exists if you try to do this with a `produce` style API. This is also problematic, because trying to do this safely runs into https://github.com/rust-lang/rust/issues/40307 The simplest thing to do is simply let the cache grow by 1 too many elements and document the behavior. --- src/linked_hash_map.rs | 4 + src/lru_cache.rs | 169 ++++++++++++----------------------------- 2 files changed, 51 insertions(+), 122 deletions(-) diff --git a/src/linked_hash_map.rs b/src/linked_hash_map.rs index fd89d52..5e7d672 100644 --- a/src/linked_hash_map.rs +++ b/src/linked_hash_map.rs @@ -197,6 +197,10 @@ impl LinkedHashMap { Some(((*back).key_ref(), (*back).value_ref())) } } + + pub fn hasher(&self) -> &S { + &self.hash_builder + } } impl LinkedHashMap diff --git a/src/lru_cache.rs b/src/lru_cache.rs index 5a62f1e..6233070 100644 --- a/src/lru_cache.rs +++ b/src/lru_cache.rs @@ -1,6 +1,5 @@ use std::{ - borrow::{Borrow, ToOwned}, - fmt, + borrow::Borrow, hash::{BuildHasher, Hash}, }; @@ -8,8 +7,7 @@ use hashbrown::hash_map; use crate::linked_hash_map::{self, LinkedHashMap}; -#[derive(Clone)] -pub struct LruCache { +pub struct LruCache { map: LinkedHashMap, max_size: usize, } @@ -24,7 +22,9 @@ impl LruCache { } } -impl LruCache { +impl LruCache + where S: BuildHasher +{ #[inline] pub fn with_hasher(capacity: usize, hash_builder: S) -> Self { LruCache { @@ -66,36 +66,29 @@ impl LruCache { } } + /// If the returned entry is vacant, it will always have room to insert a single value. By + /// using the entry API, you can exceed the configured capacity by 1. #[inline] - pub fn produce(&mut self, k: &Q, f: F) -> &mut V - where - K: Borrow, - Q: Hash + Eq + ToOwned + ?Sized, - F: FnOnce() -> V, - { - match self.map.raw_entry_mut().from_key(k) { - linked_hash_map::RawEntryMut::Occupied(mut occupied) => { - occupied.to_back(); - occupied.into_mut() - } - linked_hash_map::RawEntryMut::Vacant(vacant) => vacant.insert(k.to_owned(), f()).1, + pub fn entry(&mut self, key: K) -> Entry<'_, K, V, S> { + if self.len() > self.capacity() { + self.remove_lru(); } + self.map.entry(key) } #[inline] - pub fn produce_err(&mut self, k: &Q, f: F) -> Result<&mut V, E> - where - K: Borrow, - Q: Hash + Eq + ToOwned + ?Sized, - F: FnOnce() -> Result, - { - Ok(match self.map.raw_entry_mut().from_key(k) { - linked_hash_map::RawEntryMut::Occupied(mut occupied) => { - occupied.to_back(); - occupied.into_mut() - } - linked_hash_map::RawEntryMut::Vacant(vacant) => vacant.insert(k.to_owned(), f()?).1, - }) + pub fn raw_entry(&self) -> RawEntryBuilder<'_, K, V, S> { + self.map.raw_entry() + } + + /// If the returned entry is vacant, it will always have room to insert a single value. By + /// using the entry API, you can exceed the configured capacity by 1. + #[inline] + pub fn raw_entry_mut(&mut self) -> RawEntryBuilderMut<'_, K, V, S> { + if self.len() > self.capacity() { + self.remove_lru(); + } + self.map.raw_entry_mut() } #[inline] @@ -142,33 +135,36 @@ impl LruCache { #[inline] pub fn iter(&self) -> Iter { - Iter(self.map.iter()) + self.map.iter() } #[inline] pub fn iter_mut(&mut self) -> IterMut { - IterMut(self.map.iter_mut()) + self.map.iter_mut() } #[inline] pub fn drain(&mut self) -> Drain { - Drain(self.map.drain()) + self.map.drain() } } -impl Extend<(K, V)> for LruCache { +impl Clone for LruCache { #[inline] - fn extend>(&mut self, iter: I) { - for (k, v) in iter { - self.insert(k, v); + fn clone(&self) -> Self { + LruCache { + map: self.map.clone(), + max_size: self.max_size, } } } -impl fmt::Debug for LruCache { +impl Extend<(K, V)> for LruCache { #[inline] - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - f.debug_map().entries(self.iter().rev()).finish() + fn extend>(&mut self, iter: I) { + for (k, v) in iter { + self.insert(k, v); + } } } @@ -178,7 +174,7 @@ impl IntoIterator for LruCache { #[inline] fn into_iter(self) -> Drain { - Drain(self.map.into_iter()) + self.map.into_iter() } } @@ -202,84 +198,13 @@ impl<'a, K: Eq + Hash, V, S: BuildHasher> IntoIterator for &'a mut LruCache(linked_hash_map::Drain); - -impl Iterator for Drain { - type Item = (K, V); - - #[inline] - fn next(&mut self) -> Option<(K, V)> { - self.0.next() - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - self.0.size_hint() - } -} - -impl DoubleEndedIterator for Drain { - #[inline] - fn next_back(&mut self) -> Option<(K, V)> { - self.0.next_back() - } -} - -impl ExactSizeIterator for Drain {} - -pub struct Iter<'a, K: 'a, V: 'a>(linked_hash_map::Iter<'a, K, V>); - -impl<'a, K, V> Clone for Iter<'a, K, V> { - #[inline] - fn clone(&self) -> Iter<'a, K, V> { - Iter(self.0.clone()) - } -} - -impl<'a, K, V> Iterator for Iter<'a, K, V> { - type Item = (&'a K, &'a V); - - #[inline] - fn next(&mut self) -> Option<(&'a K, &'a V)> { - self.0.next() - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - self.0.size_hint() - } -} - -impl<'a, K, V> DoubleEndedIterator for Iter<'a, K, V> { - #[inline] - fn next_back(&mut self) -> Option<(&'a K, &'a V)> { - self.0.next_back() - } -} - -impl<'a, K, V> ExactSizeIterator for Iter<'a, K, V> {} - -pub struct IterMut<'a, K: 'a, V: 'a>(linked_hash_map::IterMut<'a, K, V>); - -impl<'a, K, V> Iterator for IterMut<'a, K, V> { - type Item = (&'a K, &'a mut V); - - #[inline] - fn next(&mut self) -> Option<(&'a K, &'a mut V)> { - self.0.next() - } - - #[inline] - fn size_hint(&self) -> (usize, Option) { - self.0.size_hint() - } -} - -impl<'a, K, V> DoubleEndedIterator for IterMut<'a, K, V> { - #[inline] - fn next_back(&mut self) -> Option<(&'a K, &'a mut V)> { - self.0.next_back() - } -} - -impl<'a, K, V> ExactSizeIterator for IterMut<'a, K, V> {} +pub type Drain = linked_hash_map::Drain; +pub type Iter<'a, K, V> = linked_hash_map::Iter<'a, K, V>; +pub type IterMut<'a, K, V> = linked_hash_map::IterMut<'a, K, V>; +pub type Entry<'a, K, V, S> = linked_hash_map::Entry<'a, K, V, S>; +pub type OccupiedEntry<'a, K, V> = linked_hash_map::OccupiedEntry<'a, K, V>; +pub type VacantEntry<'a, K, V, S> = linked_hash_map::VacantEntry<'a, K, V, S>; +pub type RawEntryBuilder<'a, K, V, S> = linked_hash_map::RawEntryBuilder<'a, K, V, S>; +pub type RawEntryBuilderMut<'a, K, V, S> = linked_hash_map::RawEntryBuilderMut<'a, K, V, S>; +pub type RawOccupiedEntryMut<'a, K, V> = linked_hash_map::RawOccupiedEntryMut<'a, K, V>; +pub type RawlVacantEntryMut<'a, K, V, S> = linked_hash_map::RawVacantEntryMut<'a, K, V, S>;