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

Cache missing Debug impl #132

Closed
lilyball opened this issue May 20, 2022 · 3 comments · Fixed by #138
Closed

Cache missing Debug impl #132

lilyball opened this issue May 20, 2022 · 3 comments · Fixed by #138
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@lilyball
Copy link

The Cache types are all missing Debug impls, which is annoying. I'm not sure what offhand such an impl should show, but perhaps some basic stats about the cache if that's easy to gather.

@tatsuya6502 tatsuya6502 self-assigned this May 21, 2022
@tatsuya6502 tatsuya6502 added the enhancement New feature or request label May 21, 2022
@tatsuya6502 tatsuya6502 added this to the v0.8.5 milestone May 21, 2022
@tatsuya6502
Copy link
Member

tatsuya6502 commented May 23, 2022

Thank you for reporting this issue.

I'm not sure what offhand such an impl should show, but perhaps some basic stats about the cache if that's easy to gather.

I started to work on this as #134 and made the Debug impl to show some basic stats. But while I was writing a code sample for the doc, I realized this will confuse users as these stats are eventual consistent (will show stale numbers).

use moka::sync::Cache;let cache = Cache::new(10);// Insert entries.
cache.insert('n', "Netherland Dwarf");
cache.insert('l', "Lop Eared");
​cache.insert('d', "Dutch");

// Ensure an entry exists.
assert_eq!(cache.get(&'l'), Some("Lop Eared"));// Print a debug info. Note that this may print stale numbers.
// (e.g. `entry_count` can be `0` instead of `3`)
println!("{:?}", cache);
// -> Cache { max_capacity: Some(10), entry_count: 0, weighted_size: 0 }// You can mitigate this by performing a `sync` first.
// Bring `ConcurrentCacheExt` trait to the scope so we can use `sync` method.
use moka::sync::ConcurrentCacheExt;
// Call `sync` to run pending internal tasks.
cache.sync();

// Now it will print the actual numbers.
println!("{:?}", cache);
// -> Cache { max_capacity: Some(10), entry_count: 3, weighted_size: 3 }

These are kept eventual consistent to improve performance (doc).

So I think I will change it to show key-value entries like the Debug impl of std::collections::HashMap does. It will be something like:

use moka::sync::Cache;let cache = Cache::new(10);// Insert entries.
cache.insert('n', "Netherland Dwarf");
cache.insert('l', "Lop Eared");
​cache.insert('d', "Dutch");

// Print a debug info.
println!("{:?}", cache);
// -> {'l': "Lop Eared", 'd': "Dutch", 'n': "Netherland Dwarf"}
// Note that entries are returned in arbitrary order.

This will never show stale values, so it will not confuse users.

To show key-value entries, both K and V must implement Debug. I hope you are okay with this.

@lilyball
Copy link
Author

If it's easy enough to show those and won't be too expensive then that sounds fine to me. It just bothered me that my type that has a cache couldn't show anything at all about it.

It looks like #134 is adding methods to query for these stats, which I appreciate as it means I have the option of showing those after all if it suits my usage better.

@tatsuya6502
Copy link
Member

Hi @lilyball — I have published Moka v0.8.5 to crates.io containing this and some other changes. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants