Preserve the allocator in From<SmallVec> for Box<[T], A> - #732
HarshRajSinghania wants to merge 1 commit into
Conversation
The previous conversion rebuilt the box through Vec, which always uses the global allocator. With the allocator API enabled, convert into Box<[T], A> and release spilled storage with the same allocator. Fixes servo#714 Signed-off-by: Harsh Raj Singhania <harshrajsinghania@proton.me>
|
|
||
| let mut boxed = Box::<[MaybeUninit<T>], A>::new_uninit_slice_in(length, allocator); | ||
| unsafe { | ||
| copy_nonoverlapping(source, boxed.as_mut_ptr().cast(), length); |
There was a problem hiding this comment.
This copy/realloc can be avoided if on_heap && length == capacity. In that case, this whole function could just be Vec::from(this).into_boxed_slice() again (once I finish implementing #702).
There was a problem hiding this comment.
but we can remove the intermediate Vec conversion
why would we want it there
I'm not sure but probably our desired behavior is to always have it on the heap when converting it from a box
There was a problem hiding this comment.
but we can remove the intermediate
Vecconversion
Is there any overhead to that conversion? It seems like that would do the exact same things that a manual implementation would do.
I'm not sure but probably our desired behavior is to always have it on the heap when converting it from a box
This is converting to a box, not from.
There was a problem hiding this comment.
Is there any overhead to that conversion? It seems like that would do the exact same things that a manual implementation would do.
yeah it's basically the same now that I think about it
after optimizations it probably compiles to the same thing
|
why is this 100 LOC it shouldn't be that big |
alejandro-vaz
left a comment
alejandro-vaz
left a comment
There was a problem hiding this comment.
yeah... but this looks overengineered at best
|
oh the issue is that #702 is pending I see |
3 participants
Summary
From<SmallVec<T, N, A>> for Box<[T]>rebuilt the box throughVec, which always uses the global allocator. With the allocator API enabled, the conversion is nowFrom<SmallVec<T, N, A>> for Box<[T], A>and releases spilled storage with the same allocator.Motivation
Requested in #714. The previous impl changed the allocator before boxing, and used an intermediary
Vec.Implementation
allocator-api,Boxis stillBox<[T]>and the existingVecpath remains.allocator-api/allocator-api2, elements are copied intoBox::new_uninit_slice_inowned byA. If theSmallVechad spilled, the old heap buffer is deallocated withA.into_boxed_slicestill returns a globalBox<[T]>viaVec, so the deprecated helper keeps its signature.Testing
cargo +nightly test --tests— 74 passedcargo +nightly test --features allocator-api --test main into_box_from_inline_and_spilled— passedcargo +nightly test --features allocator-api2 --test main into_box_from_inline_and_spilled— passedcargo +nightly test --features allocator-api2 --tests— 74 passedFixes #714