kanidm / concread

Concurrently Readable Data Structures for Rust
Mozilla Public License 2.0
339 stars 15 forks source link

Updating lru to 0.10 #94

Closed plugwash closed 7 months ago

plugwash commented 1 year ago

Hi.

I have recently been looking into updating the rust-lru package in Debian. While we do have a mechanism for packaging multiple versions of a crate in paralell, it's something we try to avoid where possible. So part of the update process is looking at the reverse dependencies to see if they can use the new version.

The main change in the new version of lru seems to be that it no longer allows creating zero-sized LruCache objects. The LruCache::new function now takes a NonZeroUsize rather than a Usize.

concread uses an LruCache in ThreadLocal and passes the size through from ThreadLocal::new to LruCache::new. My searches in Debian have only found two users of , ThreadLocal::new from concread, both in the concread crate itself but it's possible I missed something and I have not searched stuff that is not in Debian.

Both of the uses in the concread crate seem to ensure a non-zero size. The one in src/threadcache/mod.rs uses a fixed size of 8. The one in benches/arccache.rs is dynamically calculated, but has explicit logic to ensure a size of at least one.

The patch I am considering uploading to Debian is at https://salsa.debian.org/rust-team/debcargo-conf/-/blob/master/src/concread/debian/patches/lru-0.10.patch any comments would be welcome.

Firstyear commented 1 year ago

I think the patch is fine for concread itself if you want to submit the PR.

Generally I am not a supporter of distros patching libraries like this because then valuable improvements get lost upstream. So I'd rather them be sent here.