Replace Page data Vec with boxed fixed array
#628
Conversation
✅ Deploy Preview for salsa-rs canceled.
|
| @@ -169,15 +171,11 @@ impl Table { | |||
| impl<T: Slot> Page<T> { | |||
| #[allow(clippy::uninit_vec)] | |||
| fn new(ingredient: IngredientIndex) -> Self { | |||
| let mut data = Vec::with_capacity(PAGE_LEN); | |||
| unsafe { | |||
| data.set_len(PAGE_LEN); | |||
There was a problem hiding this comment.
Afaik this considered UB as we now have a Vec containing uninitialized values (UnsafeCell does not make this sound either, MaybeUninit would I believe)
CodSpeed Performance ReportMerging #628 will not alter performanceComparing Summary
|
Veykril
force-pushed
the
veykril/push-sqqurotonstp
branch
from
0404bb5 to
950fb63
Compare
| @@ -62,7 +64,7 @@ pub(crate) struct Page<T: Slot> { | |||
|
|
|||
| /// Vector with data. This is always created with the capacity/length of `PAGE_LEN` | |||
| /// and uninitialized data. As we initialize new entries, we increment `allocated`. | |||
| data: Vec<UnsafeCell<T>>, | |||
| data: Box<[UnsafeCell<MaybeUninit<T>>; PAGE_LEN]>, | |||
There was a problem hiding this comment.
We could drop the Box here entirely if we wanted as a Page is currently always allocated as a Box<dyn> anyways, but we'd need to be careful in that we never put a Page on the stack when we construct it then which is rather tricky in today's Rust. Alternatively we could get rid of the Box<dyn Page> by type erasing manually without a trait object, as a Page always has the same size and alignment irrevelant to its contained T (as it is behind an indirection)
Veykril
force-pushed
the
veykril/push-sqqurotonstp
branch
from
950fb63 to
1eaf262
Compare
No need to use a
Vechere, we have a comptime length of the page so we unnecessarily store an extra length and capacity