Skip to content

Commit

Permalink
Merge pull request #295 from moka-rs/fix-eviction-listener-key-lock-v011
Browse files Browse the repository at this point in the history
Fix a bug in sync caches where memory usage kept increasing when the eviction listener is set (v0.11.x)
  • Loading branch information
tatsuya6502 authored Aug 4, 2023
2 parents 0032235 + 354a607 commit e9aa951
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 6 deletions.
10 changes: 10 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# Moka Cache — Change Log

## Version 0.11.3

### Fixed

- Fixed a bug in `sync::Cache` and `sync::SegmentedCache` where memory usage kept
increasing when the eviction listener was set with the `Immediate` delivery mode.
([#295][gh-pull-0295])


## Version 0.11.2

Bumped the minimum supported Rust version (MSRV) to 1.65 (Nov 3, 2022).
Expand Down Expand Up @@ -676,6 +685,7 @@ The minimum supported Rust version (MSRV) is now 1.51.0 (Mar 25, 2021).
[gh-issue-0034]: https://github.com/moka-rs/moka/issues/34/
[gh-issue-0031]: https://github.com/moka-rs/moka/issues/31/

[gh-pull-0295]: https://github.com/moka-rs/moka/pull/295/
[gh-pull-0277]: https://github.com/moka-rs/moka/pull/277/
[gh-pull-0275]: https://github.com/moka-rs/moka/pull/275/
[gh-pull-0272]: https://github.com/moka-rs/moka/pull/272/
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "moka"
version = "0.11.2"
version = "0.11.3"
edition = "2018"
# Rust 1.65 was released on Nov 3, 2022.
rust-version = "1.65"
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,15 @@ algorithm to determine which entries to evict when the capacity is exceeded.
[release-badge]: https://img.shields.io/crates/v/moka.svg
[docs-badge]: https://docs.rs/moka/badge.svg
[deps-rs-badge]: https://deps.rs/repo/github/moka-rs/moka/status.svg
[coveralls-badge]: https://coveralls.io/repos/github/moka-rs/moka/badge.svg?branch=master
[coveralls-badge]: https://coveralls.io/repos/github/moka-rs/moka/badge.svg?branch=main
[license-badge]: https://img.shields.io/crates/l/moka.svg
[fossa-badge]: https://app.fossa.com/api/projects/git%2Bgithub.com%2Fmoka-rs%2Fmoka.svg?type=shield

[gh-actions]: https://github.com/moka-rs/moka/actions?query=workflow%3ACI
[crate]: https://crates.io/crates/moka
[docs]: https://docs.rs/moka
[deps-rs]: https://deps.rs/repo/github/moka-rs/moka
[coveralls]: https://coveralls.io/github/moka-rs/moka?branch=master
[coveralls]: https://coveralls.io/github/moka-rs/moka?branch=main
[fossa]: https://app.fossa.com/projects/git%2Bgithub.com%2Fmoka-rs%2Fmoka?ref=badge_shield

[caffeine-git]: https://github.com/ben-manes/caffeine
Expand Down Expand Up @@ -112,7 +112,7 @@ routers. Here are some highlights:

## Change Log

- [CHANGELOG.md](https://github.com/moka-rs/moka/blob/master/CHANGELOG.md)
- [CHANGELOG.md](https://github.com/moka-rs/moka/blob/main/CHANGELOG.md)

The `unsync::Cache` and `dash::Cache` have been moved to a separate crate called
[Mini Moka][mini-moka-crate]:
Expand Down
6 changes: 6 additions & 0 deletions src/future/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2009,6 +2009,10 @@ where
fn set_expiration_clock(&self, clock: Option<crate::common::time::Clock>) {
self.base.set_expiration_clock(clock);
}

fn key_locks_map_is_empty(&self) -> bool {
self.base.key_locks_map_is_empty()
}
}

pub struct BlockingOp<'a, K, V, S>(&'a Cache<K, V, S>);
Expand Down Expand Up @@ -2164,6 +2168,7 @@ mod tests {
assert!(!cache.contains_key(&"d"));

verify_notification_vec(&cache, actual, &expected);
assert!(cache.key_locks_map_is_empty());
}

#[test]
Expand Down Expand Up @@ -2353,6 +2358,7 @@ mod tests {
assert_eq!(cache.weighted_size(), 25);

verify_notification_vec(&cache, actual, &expected);
assert!(cache.key_locks_map_is_empty());
}

#[tokio::test]
Expand Down
10 changes: 10 additions & 0 deletions src/sync/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2054,6 +2054,10 @@ where
pub(crate) fn set_expiration_clock(&self, clock: Option<crate::common::time::Clock>) {
self.base.set_expiration_clock(clock);
}

pub(crate) fn key_locks_map_is_empty(&self) -> bool {
self.base.key_locks_map_is_empty()
}
}

// To see the debug prints, run test as `cargo test -- --nocapture`
Expand Down Expand Up @@ -2186,6 +2190,7 @@ mod tests {
assert!(!cache.contains_key(&"d"));

verify_notification_vec(&cache, actual, &expected, delivery_mode);
assert_with_mode!(cache.key_locks_map_is_empty(), delivery_mode);
}
}

Expand Down Expand Up @@ -2316,6 +2321,7 @@ mod tests {
assert_eq_with_mode!(cache.weighted_size(), 25, delivery_mode);

verify_notification_vec(&cache, actual, &expected, delivery_mode);
assert_with_mode!(cache.key_locks_map_is_empty(), delivery_mode);
}
}

Expand Down Expand Up @@ -4339,6 +4345,8 @@ mod tests {
assert_eq!(a[0], (Arc::new("alice"), "a3", RemovalCause::Expired));
a.clear();
}

assert!(cache.key_locks_map_is_empty());
}

// This test ensures the key-level lock for the immediate notification
Expand Down Expand Up @@ -4469,6 +4477,8 @@ mod tests {
for (i, (actual, expected)) in actual.iter().zip(&expected).enumerate() {
assert_eq!(actual, expected, "expected[{}]", i);
}

assert!(cache.key_locks_map_is_empty());
}

// NOTE: To enable the panic logging, run the following command:
Expand Down
9 changes: 9 additions & 0 deletions src/sync/segment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -687,6 +687,13 @@ where

exp_clock
}

fn key_locks_map_is_empty(&self) -> bool {
self.inner
.segments
.iter()
.all(|seg| seg.key_locks_map_is_empty())
}
}

// For unit tests.
Expand Down Expand Up @@ -916,6 +923,7 @@ mod tests {
assert!(!cache.contains_key(&"d"));

verify_notification_vec(&cache, actual, &expected, delivery_mode);
assert_with_mode!(cache.key_locks_map_is_empty(), delivery_mode);
}
}

Expand Down Expand Up @@ -1065,6 +1073,7 @@ mod tests {
assert_eq_with_mode!(cache.weighted_size(), 25, delivery_mode);

verify_notification_vec(&cache, actual, &expected, delivery_mode);
assert_with_mode!(cache.key_locks_map_is_empty(), delivery_mode);
}
}

Expand Down
12 changes: 12 additions & 0 deletions src/sync_base/base_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -785,6 +785,10 @@ where
pub(crate) fn set_expiration_clock(&self, clock: Option<Clock>) {
self.inner.set_expiration_clock(clock);
}

pub(crate) fn key_locks_map_is_empty(&self) -> bool {
self.inner.key_locks_map_is_empty()
}
}

struct EvictionState<'a, K, V> {
Expand Down Expand Up @@ -2415,6 +2419,14 @@ where
*exp_clock = None;
}
}

fn key_locks_map_is_empty(&self) -> bool {
self.key_locks
.as_ref()
.map(|m| m.is_empty())
// If key_locks is None, consider it is empty.
.unwrap_or(true)
}
}

//
Expand Down
11 changes: 9 additions & 2 deletions src/sync_base/key_lock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,11 @@ where
S: BuildHasher,
{
fn drop(&mut self) {
if TrioArc::count(&self.lock) <= 1 {
if TrioArc::count(&self.lock) <= 2 {
self.map.remove_if(
self.hash,
|k| k == &self.key,
|_k, v| TrioArc::count(v) <= 1,
|_k, v| TrioArc::count(v) <= 2,
);
}
}
Expand Down Expand Up @@ -86,3 +86,10 @@ where
}
}
}

#[cfg(test)]
impl<K, S> KeyLockMap<K, S> {
pub(crate) fn is_empty(&self) -> bool {
self.locks.len() == 0
}
}

0 comments on commit e9aa951

Please sign in to comment.