heapless
heapless copied to clipboard
Implement a generic length parameter for Vec<T, N>
Currently, Vec<T, N> always uses usize for the length field, which leads to silly situations like Vec<u8, 4> being 8 bytes large on 32 bit platforms. This PR introduces a generic to provide this length field as well as a default for many common length values to avoid much user code changing.
I am okay to revert sections of this PR if wanted, such as
- ~~The change of
usizetoLenTin many arguments and return types, if they are considered too breaking.~~ Done,usizeis still used for indexing and as thelenreturn type. - Adding the length generic to
VecView, this can be changed to only storeusizeat the cost of onlyVec<T, N, usize>being able to unsize (or requiring aVec::cast_len_typecall before unsizing, at least).
This lays the ground work for followup PRs to implement a similar length generic on the other containers, but I'm starting with just Vec to test the waters.
May I ask what your motivation is for this change? Do you have any real-world use cases for it?
This seems like a premature optimization to me, and it might confuse users who are new to Rust, especially when they need to specify the full type of Vec.
If this change is necessary, a better approach might be to add LenT only to VecInner and let Vec use it with LenT=usize. This would keep the API simple while still allowing advanced users to leverage VecInner when needed.
I have been interested in performing this kind of optimization to a stack array for a while, and in-fact have a PR open to arrayvec to implement this, but the case that pushed me over the edge was embedded-tls's usage of many 1 byte large Ts in nested Vec's in enums leading to 160 bytes of data to be take up 328 bytes of storage.
Users who are new to Rust will most likely not hit this, as they will not be generic over the length type and therefore will get the exact same experience due to the default. If they do stray into being generic or using large N values, it is explained in documentation and in the error message what to do (pick the smallest integer type for your N, or usize).
Sorted out all of your review comments, @reitermarkus!
Interestingly, I made a similar MR in #204 2 years ago, but it was rejected for complexity.
It doesn't look like it was rejected for complexity, although that PR is significantly more complex due to typenum bounds, it was just closed because of the migration to const generics.
@GnomedDev, sorry for the long wait. Can you have a look at rebasing this?
I'll take a look at rebasing it in the next week, if I don't get around to it after that time feel free to close or take over this PR.
It looks like this PR is going to have to be practically rewritten due to the VecStorage change... but I'll see if I can do that.
@reitermarkus Done, please note that I just did this from scratch instead of rebasing as it was conflicting so much, so keep that in mind while reviewing.
@GnomedDev, needs another rebase, but should be good to go after that.
Adding the length generic to
VecView, this can be changed to only storeusizeat the cost of onlyVec<T, N, usize>being able to unsize (or requiring aVec::cast_len_typecall before unsizing, at least).
I'm quite split on this: If VecView only supports usize, it's easier to use, but then Vec will have to use usize by default or it gets confusing, so the space-saving becomes opt-in. I sightly prefer the space-saving by default, since it applies to the most common sizes with N < 256, and keeping Vec and VecView more consistent.
@reitermarkus I've rebased, happy for this to finally get merged in.
Thanks!