Skip to content

Commit

Permalink
Add is_empty to LenEntry
Browse files Browse the repository at this point in the history
Since this clippy warning requires touching several files we handle it
separately. Most remaining warnings should be self-contained enough that
we can handle them on a file-by-file basis.
  • Loading branch information
aaronmondal committed Jul 12, 2023
1 parent 5322c33 commit e643090
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 12 deletions.
4 changes: 4 additions & 0 deletions cas/store/filesystem_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,10 @@ impl LenEntry for FileEntryImpl {
self.file_size as usize
}

fn is_empty(&self) -> bool {
self.file_size == 0
}

#[inline]
async fn touch(&self) {
let result = self
Expand Down
5 changes: 5 additions & 0 deletions cas/store/memory_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ impl LenEntry for BytesWrapper {
fn len(&self) -> usize {
Bytes::len(&self.0)
}

#[inline]
fn is_empty(&self) -> bool {
Bytes::is_empty(&self.0)
}
}

pub struct MemoryStore {
Expand Down
4 changes: 4 additions & 0 deletions cas/store/tests/filesystem_store_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,10 @@ impl<Hooks: FileEntryHooks + 'static + Sync + Send> LenEntry for TestFileEntry<H
self.inner.as_ref().unwrap().len()
}

fn is_empty(&self) -> bool {
self.inner.as_ref().unwrap().is_empty()
}

async fn touch(&self) {
self.inner.as_ref().unwrap().touch().await
}
Expand Down
23 changes: 14 additions & 9 deletions util/evicting_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ pub trait LenEntry {
/// Length of referenced data.
fn len(&self) -> usize;

/// Returns `true` if `self` has zero length.
fn is_empty(&self) -> bool;

/// Called when an entry is touched.
#[inline]
async fn touch(&self) {}
Expand Down Expand Up @@ -91,14 +94,19 @@ impl<T: LenEntry + Send + Sync> LenEntry for Arc<T> {
T::len(self.as_ref())
}

#[inline]
fn is_empty(&self) -> bool {
T::is_empty(self.as_ref())
}

#[inline]
async fn touch(&self) {
self.as_ref().touch().await
self.as_ref().touch().await;
}

#[inline]
async fn unref(&self) {
self.as_ref().unref().await
self.as_ref().unref().await;
}
}

Expand Down Expand Up @@ -148,7 +156,7 @@ where
};
serialized_lru
.data
.push((serialized_digest, eviction_item.seconds_since_anchor as i32));
.push((serialized_digest, eviction_item.seconds_since_anchor));
}
serialized_lru
}
Expand Down Expand Up @@ -187,11 +195,8 @@ where
}

async fn evict_items(&self, state: &mut State<T>) {
let mut peek_entry = if let Some((_, entry)) = state.lru.peek_lru() {
entry
} else {
return;
};
let Some((_, mut peek_entry)) = state.lru.peek_lru() else { return; };

while self.should_evict(state.lru.len(), peek_entry, state.sum_store_size) {
let (key, eviction_item) = state.lru.pop_lru().expect("Tried to peek() then pop() but failed");
state.sum_store_size -= eviction_item.data.len() as u64;
Expand Down Expand Up @@ -246,7 +251,7 @@ where
};
let mut state = self.state.lock().await;

let maybe_old_item = if let Some(old_item) = state.lru.put(digest.into(), eviction_item) {
let maybe_old_item = if let Some(old_item) = state.lru.put(digest, eviction_item) {
state.sum_store_size -= old_item.data.len() as u64;
// Note: See comment in `unref()` requring global lock of insert/remove.
old_item.data.unref().await;
Expand Down
15 changes: 12 additions & 3 deletions util/tests/evicting_map_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ impl LenEntry for BytesWrapper {
fn len(&self) -> usize {
Bytes::len(&self.0)
}

#[inline]
fn is_empty(&self) -> bool {
Bytes::is_empty(&self.0)
}
}

impl From<Bytes> for BytesWrapper {
Expand Down Expand Up @@ -272,7 +277,11 @@ mod evicting_map_tests {
impl LenEntry for MockEntry {
fn len(&self) -> usize {
// Note: We are not testing this functionality.
return 0;
0
}

fn is_empty(&self) -> bool {
unreachable!("We are not testing this functionality");
}

async fn touch(&self) {
Expand Down Expand Up @@ -318,8 +327,8 @@ mod evicting_map_tests {
let existing_entry = evicting_map.get(&DigestInfo::try_new(HASH1, 0)?).await.unwrap();
assert_eq!(existing_entry.data, DATA2);

assert_eq!(entry1.unref_called.load(Ordering::Relaxed), true);
assert_eq!(entry2.unref_called.load(Ordering::Relaxed), false);
assert!(entry1.unref_called.load(Ordering::Relaxed));
assert!(!entry2.unref_called.load(Ordering::Relaxed));

Ok(())
}
Expand Down

0 comments on commit e643090

Please sign in to comment.