-
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
Move always move the whole SmallVec
object.
#70
Comments
I consider this to be a rust bug: rust-lang/rust#42763 |
The workaround is "don't move stuff much", unfortunately :( Thanks a lot for noticing and for the report, but I don't think much can be done from this repo. Please reopen if you think otherwise though. |
I think that issue misses the problem I am talking about: when you move or
copy an enum, Rust memcpys the whole enum storage independently of which
variant is active. This is bad if one of the variants is much larger than
the others.
Passing something by move is intended to copy the objects storage. In the
case of SmallVec this mean copying the enum. My point was that if the
SmallVec is using the heap, this memcpy should not copy the whole enum, but
only the parts of it needed to track the heap allocation (which are
typically much smaller).
This is an orthogonal issue to "not copying anything on move when you don't
need to". If you are calling an extern function (e.g. From an external
crate that the optimizer cannot see) then values, in most cases, need to be
copied on move. If you can see the function and you move a value in and
then out then you don't need any moves at all. Or if you know that the
parent stackframe is alive during the function then you don't need moves
either.
So I think these are two different issues.
…On Thu 2. Nov 2017 at 06:30, Emilio Cobos Álvarez ***@***.***> wrote:
The workaround is "don't move stuff much", unfortunately :(
Thanks a lot for noticing and for the report, but I don't think much can
be done from this repo. Please reopen if you think otherwise though.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#70 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AA3NpgX54U-ePplN5YNF3z7_GWC4akjmks5syVNvgaJpZM4QOJpK>
.
|
Oh ok, I see what you mean, happy to reopen, though still not sure how can this be fixed on our side... Maybe it's better to fix a rust issue (too?). I don't know how this would be fixed without introducing branches on move, which I don't think it's something you can do. |
@emilio Custom move constructors in Rust are possible: Maybe we could have an optical feature to use this approach for SmallVec? |
Having to wrap all your smallvec declarations in a macro seems like a no-go tbh, specially to fix a relatively minor perf issue. |
Is this issue related to my problem? |
A
SmallVec<[f64; 100]>
with acapacity()
larger than 100 allocates the elements on the heap but moving theSmallVec
generates amemcpy
over the wholeSmallVec
object including the unused storage for the 100f64
elements and not just3*size
.This seems to be a general problem with
enum
s but I couldn't find a rustc bug to track this, so I don't know if this is intended or even fixable. Anyhow, maybeSmallVec
can workaround this behavior somehow?The text was updated successfully, but these errors were encountered: