Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure that a pinned entry is accepted by the cache #47

Merged
merged 2 commits into from
Aug 24, 2024
Merged

Ensure that a pinned entry is accepted by the cache #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4c58727
Ensure that a pinned entry is accepted by the cache
arthurprs Aug 24, 2024
106ddcf
slightly improve hit rate
arthurprs Aug 24, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion fuzz/Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 4 additions & 2 deletions fuzz/fuzz_targets/fuzz_sync_cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,9 +178,11 @@ fn check_evicted(key: u16, get: Option<Value>, evicted: Vec<(u16, Value)>) {
let mut evicted_hm = HashSet::default();
evicted_hm.reserve(evicted.len());
for (ek, ev) in evicted {
// we can't evict a 0 weight item, unless it was for the same key
// we can't evict a 0 weight item, unless it was replaced
assert!(ev.current != 0 || ek == key);
// we can't evict something twice, except if the insert displaced an old old value but the new value also got evicted
// we can't evict a pinned item, unless it was replaced
assert!(!ev.pinned || ek == key);
// we can't evict something twice, except if the insert displaced an old value but the new value also got evicted
assert!(evicted_hm.insert(ek) || (ek == key && get.is_none()));
}
}
17 changes: 9 additions & 8 deletions fuzz/fuzz_targets/fuzz_unsync_cache.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#![no_main]
use std::time::Duration;

use ahash::HashSet;
use arbitrary::Arbitrary;
use libfuzzer_sys::fuzz_target;
Expand Down Expand Up @@ -94,7 +92,7 @@ fn run(input: Input) {
.shards(1)
.build()
.unwrap();
let mut did_any_update_increase_weight = false;
let mut did_any_update_increase_weight_or_unpin = false;
let mut cache = Cache::with_options(options, MyWeighter, hasher, MyLifecycle);
for op in operations {
match op {
Expand All @@ -115,7 +113,8 @@ fn run(input: Input) {
}
Op::Update(k, v, pinned) => {
if let Some(mut ref_mut) = cache.get_mut(&k) {
did_any_update_increase_weight |= v > ref_mut.current;
did_any_update_increase_weight_or_unpin |=
ref_mut.current < v || (ref_mut.pinned && !pinned);
*ref_mut = Value {
original: v,
current: v,
Expand Down Expand Up @@ -171,9 +170,9 @@ fn run(input: Input) {
assert!(cache.peek(&k).is_none());
}
}
cache.validate(did_any_update_increase_weight_or_unpin);
}
cache.validate(did_any_update_increase_weight);
if did_any_update_increase_weight {
if did_any_update_increase_weight_or_unpin {
cache.insert(
0,
Value {
Expand All @@ -190,9 +189,11 @@ fn check_evicted(key: u16, get: Option<Value>, evicted: Vec<(u16, Value)>) {
let mut evicted_hm = HashSet::default();
evicted_hm.reserve(evicted.len());
for (ek, ev) in evicted {
// we can't evict a 0 weight item, unless it was for the same key
// we can't evict a 0 weight item, unless it was replaced
assert!(ev.current != 0 || ek == key);
// we can't evict something twice, except if the insert displaced an old old value but the new value also got evicted
// we can't evict a pinned item, unless it was replaced
assert!(!ev.pinned || ek == key);
// we can't evict something twice, except if the insert displaced an old value but the new value also got evicted
assert!(evicted_hm.insert(ek) || (ek == key && get.is_none()));
}
}
36 changes: 11 additions & 25 deletions src/shard.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -542,9 +542,6 @@ impl<
/// Panics if the cache is empty.
#[must_use]
fn advance_cold(&mut self, lcs: &mut L::RequestState) -> bool {
debug_assert_ne!(self.num_cold + self.num_hot, 0);
debug_assert_ne!(self.weight_cold + self.weight_hot, 0);

let mut pinned = 0usize;
loop {
let idx = if let Some(idx) = self.cold_head {
Expand All @@ -558,6 +555,7 @@ impl<
};
debug_assert_eq!(resident.state, ResidentState::Cold);
if *resident.referenced.get_mut() != 0 {
*resident.referenced.get_mut() -= 1;
resident.state = ResidentState::Hot;
let weight = self.weighter.weight(&resident.key, &resident.value);
self.weight_hot += weight;
Expand Down Expand Up @@ -632,7 +630,7 @@ impl<
}
if self.lifecycle.is_pinned(&resident.key, &resident.value) {
pinned += 1;
if pinned >= self.num_hot * MAX_F as usize {
if pinned > self.num_hot * MAX_F as usize {
return false;
}
self.hot_head = Some(next);
Expand Down Expand Up @@ -681,8 +679,7 @@ impl<
weight: u64,
strategy: InsertStrategy,
) -> Result<(), (Key, Val)> {
// caller must have already handled overweight items
debug_assert!(weight <= self.weight_target_hot);
// caller already handled overweight items, but it could have been pinned
let (entry, _) = self.entries.get_mut(idx).unwrap();
let referenced;
let enter_state;
Expand Down Expand Up @@ -830,7 +827,7 @@ impl<
};
let mut weight = self.weighter.weight(&key, &value);
// don't admit if it won't fit within the budget
if weight > self.weight_target_hot {
if weight > self.weight_target_hot && !self.lifecycle.is_pinned(&key, &value) {
self.lifecycle.before_evict(lcs, &key, &mut value);
weight = self.weighter.weight(&key, &value);
if weight > self.weight_target_hot {
Expand Down Expand Up @@ -892,7 +889,7 @@ impl<
) -> Result<(), (Key, Val)> {
let mut weight = self.weighter.weight(&key, &value);
// don't admit if it won't fit within the budget
if weight > self.weight_target_hot {
if weight > self.weight_target_hot && !self.lifecycle.is_pinned(&key, &value) {
self.lifecycle.before_evict(lcs, &key, &mut value);
weight = self.weighter.weight(&key, &value);
if weight > self.weight_target_hot {
Expand All @@ -906,24 +903,13 @@ impl<
return Err((key, value));
}

let enter_hot = if self.weight_hot + self.weight_cold + weight > self.weight_capacity {
// evict until we have enough space for this entry
loop {
let evicted = self.advance_cold(lcs);
let overweight = self.weight_hot + self.weight_cold + weight > self.weight_capacity;
if !overweight {
break;
}
if !evicted {
return self.handle_insert_overweight(lcs, hash, key, value, strategy);
}
}
false
} else {
// cache if filling, admit as hot if possible
self.weight_hot + weight <= self.weight_target_hot
};
// pre-evict instead of post-evict, this gives sightly more priority to the new item
while self.weight_hot + self.weight_cold + weight > self.weight_capacity
&& self.advance_cold(lcs)
{}

// cache is filling up, admit as hot if possible
let enter_hot = self.weight_hot + weight <= self.weight_target_hot;
let (state, list_head) = if enter_hot {
self.num_hot += 1;
self.weight_hot += weight;
Expand Down
Loading