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 size 0 / Disable Cache #165

Open
PSeitz opened this issue Jan 11, 2023 · 3 comments
Open

Cache size 0 / Disable Cache #165

PSeitz opened this issue Jan 11, 2023 · 3 comments

Comments

@PSeitz
Copy link

PSeitz commented Jan 11, 2023

v0.7x had better ergonomics if you want to optionally disable the cache. That's not possible anymore with NonZeroUsize, now you would need extra code for that use case.

@behzadnouri
Copy link

+1 to this.

@jeromefroe Can you please consider reverting #150 which uses NonZeroUsize for capacity.

There are certain cases which capacity zero is pretty useful and ergonomic in the code as in for example to disable the cache mentioned above. It is a similar thing to pass an empty vector or buffer around.
Or you may be dynamically allocating capacity to each of many caches and some might get zero.

NonZeroUsize is also making the api more divergent from standard hash-map type (or other common types). Having to use NonZeroUsize also forces to either use the unsafe new_unchecked function or use unwrap after new, and both are ugly.

@Yosi-Hezi
Copy link
Contributor

If you wish to support a cache with zero capacity, you will need to return an Option whenever you call get_or_insert or get_mut_or_insert. This means handling the case where the capacity is zero each time instead of just during initialization. Is this justified? I think that in most use cases the desired change will add much more branching (handling Options).

Comparing it to a hash-map type may not be suitable because hash-maps typically have dynamic capacity and don't fail on insertions due to capacity limitations, as it increases on demand.

Still, it should be possible to create a wrapper around the cache to handle the zero-capacity case and expose the desired APIs.
WDYT?

behzadnouri added a commit to behzadnouri/lru-rs that referenced this issue Aug 3, 2023
NonZeroUsize for capacity is very nonergonomic and prevents some
use-cases. For example a usize allows to disable caching by using
capacity zero. see: jeromefroe#165

This reverts commit 0e415ef.
@behzadnouri
Copy link

If you wish to support a cache with zero capacity, you will need to return an Option whenever you call get_or_insert or get_mut_or_insert

Not necessarily.
You can implement:

enum ValueWrapper<'a, V> {
    Borrowed(&'a V),
    Owned(V),
}

So that get_or_insert will return ValueWrapper instead, and the ValueWrapper can implement Borrow:

impl<'a, V> Borrow<V> for ValueWrapper<'a, V> {
    fn borrow(&self) -> &V {
        match self {
            Self::Borrowed(v) => v,
            Self::Owned(v) => v,
        }
    }
}

so that effectively it is equivalent to &V.

I have implemented this idea in #177

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants