-
Notifications
You must be signed in to change notification settings - Fork 146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
SmallVec<[T; N]> is often larger than it needs to be #219
Comments
This would make sense if |
That is somewhat true with the current implementation, but it's certainly not true in general -- you'd just have to move from using a (e.g.) |
The main difference here I guess is with SmallVec<[u8; N]> where N is low. Other cases don't matter quite as much (although might be slightly larger than they would be otherwise), but IME this is a somewhat common case for people who use SmallVec<[u8; N]> as a kind of SmallString. |
I still don't quite see how that would work. |
You wouldn't be able to use capacity for that. That said I don't think this would cause really any additional branching or anything -- you already have to branch on representation in each method. |
But how would you disambiguate instead? Using an enum Storage {
Stack(u8, MaybeUninit<[u8; 4]>),
Heap(*mut u8, usize, usize),
} So you could only gain something if the array backing storage is larger than |
Though I don't know if this can be implemented, we can disambiguate by most significant bit of the pointer, because pointers don't overflow |
Could we do something like what https://github.com/maciejhirsz/beef did? |
Hmm, on further investigation a lot of this is actually fixed (in part, anyway) by the Or, it's still larger than it needs to be in some cases, but doesn't get better by doing what I suggested (I suspect if/when rustc gets more aggressive niche-filling optimizations that might change, but obviously this could be revisited if that ever happens and it makes a difference). That said, I'm still not sure why this uses the size to differentiate than storing everything in the enum, which I think would eliminate the extra size overhead on stable (or am I mistaken again...), but I guess the preference (mentioned in #183) is to wait for unions to stabilize. |
Regarding There might be more too -- it mentions it only being 64-bit compatible and there are a lot of tricks you can do by taking advantage of the somewhat peculiar way that the address space is laid out on x86_64 and aarch64 ... I'm not sure this sort of thing is really worth the hassle for smallvec though. My gut would be to say "no not really". Anyway I'm just going to close it since I filed it due to a misunderstanding. I still wish SmallVec was, err, smaller though >_> |
Currently a
usize
is used to store the size when inline, but ideally if N < 256 a u8 would be used for the size, and u16 for N < 65536, etc.The text was updated successfully, but these errors were encountered: