rust-embedded / heapless

Heapless, `static` friendly data structures
Apache License 2.0
1.52k stars 181 forks source link

Implement a generic length parameter for Vec<T, N> #504

Open GnomedDev opened 2 months ago

GnomedDev commented 2 months ago

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

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.

t-moe commented 2 months ago

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.

GnomedDev commented 2 months ago

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).

GnomedDev commented 2 months ago

Sorted out all of your review comments, @reitermarkus!

YuhanLiin commented 2 months ago

Interestingly, I made a similar MR in #204 2 years ago, but it was rejected for complexity.

GnomedDev commented 2 months ago

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.