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.

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

Fix a bug in sync caches where memory usage kept increasing when the eviction listener is set (v0.11.x) #295

Merged
merged 7 commits into from
Aug 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
}
}
Loading