Use a custom container for Cache's cache #689
Conversation
The new Lru type, as a single-purpose "ArrayVec", - removes a level of indirection - omits a needless Vec allocation entirely - internalizes invariants we attempted to maintain in open-coded form It costs a little `unsafe` to handle Drop, which we unfortunately need as addr2line uses Arc inside Context.
|
Code size changes for a hello-world Rust program linked with libstd with backtrace: On platform
|
| fn default() -> Self { | ||
| Lru { | ||
| len: 0, | ||
| arr: [const { MaybeUninit::uninit() }; N], |
There was a problem hiding this comment.
this requires an MSRV bump, or to use the infamously sus-looking-but-actually-okay-for-this-ONE-instance:
| arr: [const { MaybeUninit::uninit() }; N], | |
| arr: unsafe { MaybeUninit::uninit().assume_init() }, |
There was a problem hiding this comment.
On the one hand I'm not against bumping the MSRV for this crate but on the other hand it is only one line. But on the third hand it is a very sussy line that would at least need a comment saying "no really, this is ok!"
What would the new MSRV be?
There was a problem hiding this comment.
if 318dd91 passes then the answer is 1.79
There was a problem hiding this comment.
Ah 1.73 to 1.79? That seems fine to me. We should probably also update the rust-version in Cargo.toml as that seems to have gone out of the sink.
|
Code size changes for a hello-world Rust program linked with libstd with backtrace: On platform
|
workingjubilee
force-pushed
the
formalize-lru
branch
from
318dd91 to
3afccb1
Compare
|
Code size changes for a hello-world Rust program linked with libstd with backtrace: On platform
|
| let len = self.len; | ||
| self.len = 0; | ||
| // SAFETY: we can't touch these values again due to setting self.len = 0 | ||
| unsafe { ptr::drop_in_place(ptr::addr_of_mut!(self.arr[0..len]) as *mut [T]) } |
There was a problem hiding this comment.
cast still requires sized, shame.
| let len_to_init = self.len + 1; | ||
| let mut last = MaybeUninit::new(value); | ||
| for elem in self.arr[0..len_to_init].iter_mut() { | ||
| last = mem::replace(elem, last); |
There was a problem hiding this comment.
what we do is simultaneously assign the last to an index, and take out the old value at that index. eventually everything gets moved over one index by this process. it isn't really mem::swap because we're instead moving everything a step over, and that isn't really expressible that way (and there's no windows_mut, besides).
There was a problem hiding this comment.
Maybe I'm having a brain fart but I mean literally last = mem::replace(elem, last); looks suspiciously like mem::swap(elem, &mut last); no?
There was a problem hiding this comment.
Hm, that had a big effect on code size???
|
Code size changes for a hello-world Rust program linked with libstd with backtrace: On platform
|
|
... |
|
Yeah ok, no idea what that's about but revering back to |
This reverts commit b7fbd04.
|
Code size changes for a hello-world Rust program linked with libstd with backtrace: On platform
|
The new Lru type, as a single-purpose "ArrayVec",
It costs a little
unsafeto handle Drop, which we unfortunately need as addr2line uses Arc inside Context.