In the first sample code, there is also a copy into the SAV to begin with which isn’t necessary.
In produce(), in order to make a Future<T> from a T, the source object is copied even if it is an rvalue reference because Future only has a constructor for const T & and not T &&. I verified that this is what is happening by replacing the vector with a struct that prints out move/copy/construct activity.
Adding a T && constructor to Future avoids the copy in this situation - where a Future is constructed directly from an rvalue. I’m not sure how much copying this will save in practice, but unless it is somehow incorrect it can’t hurt to add.
The constructor is defined as
Future(T&& presentValue)
: sav(new SAV<T>(1, 0))
{
sav->send(std::forward<T>(presentValue));
}
In playing around with this, I ended up going down a very deep rabbit hole. I wanted to see what other situations would avoid copying into the SAV from an rvalue, so I wrote an actor with a bunch of return situations in which I would want to avoid copying into the SAV to see if they do. What I found is lots of extra copies I didn’t expect. I traced it to that the actor compiler generates a_callback_fire() as taking the result by value instead of const reference. So I changed the actor compiler, got the results I expected, and was happy right up until I pulled master and saw that this same fix was done 11 days ago. D’oh!
That’s okay though, because in the process I learned a lot about the actor compiler generated code. I also thought of this: In this pattern,
ACTOR static Future<B> produce() {
state B b;
// call actors, wait for them, add stuff to b, have a great time
return b;
}
b will be copied into the SAV. However, using std::move(b) instead will avoid the copy, and should always be done because it should always be valid, I think. When returning from an actor, the sequence is
- initialize the SAV
- destroy the actor state
- call callbacks
so at return time any actor state variable could be treated as an rvalue reference because it’s about to be destroyed anyway. Can anyone think of any reason this is not the case? If not, it would be neat for the actor compiler to automatically std::move() state variables when they are returned. At the very least it could recognize the syntax return <exactStateVarName>; because even if the state variable is masked by a same-named local var moving it shoudl still be valid.