In this particular case, the most efficient solution is probably the C++98 solution:
void takeMessages(std::vector<std::tuple<HWND, UINT, WPARAM, LPARAM>> & windowMessages)
{
std::swap(windowMessages, m_windowMessages);
m_windowMessages.clear();
}
As long as the user doesn't throw away his vector every loop, memory will actually be reused every time instead of being reallocated. (which will happen no matter if you use RVO-move or regular move into another vector)
I have a similar follow up question about returning a std::tuple<ve0, vec1, vec2, vec3, ....>.
Suppose a game class have several member vectors:
class Game
{
public:
Game();
~Game();
std::tuple<
std::vector<std::string>, // vec0
std::vector<std::string>, // vec1
std::vector<std::string>, // vec2
std::vector<std::string> // vec3
> takeOutput();
private:
//Wrap members in m_output?
//std::tuple<
std::vector<std::string> m_vec0;
std::vector<std::string> m_vec1;
std::vector<std::string> m_vec2;
std::vector<std::string> m_vec3;
//> m_output
}
The game class has a method to take out these vectors called std::tuple<....> takeOutput():
std::tuple<
std::vector<std::string>, // vec0
std::vector<std::string>, // vec1
std::vector<std::string>, // vec2
std::vector<std::string> // vec3
> Game::takeOutput()
{
return m_output; // returns a copy? I want to move it out
//? return std::move(m_output);
//std::swap(localTuple, m_output) return localTuple; //swap method with a local, disables RVO?
//? return std::make_tuple(m_vec0, m_vec1, m_vec2, m_vec3);
//? return std::make_tuple(std::move(m_vec0), std::move(m_vec1), std::move(m_vec2), std::move(m_vec3));
}
I believe std::tuple has move semantics built in. But how would I go about wrapping the vectors in the tuple before returning from takeOutput()?
Should I have a member std::tuple<...> m_output and return it, possibly using std::swap or std::move?
Or should I return r-value using std::make_tuple<...>(vec0, vec1, vec2, vec3....)? What about std::move to move vectors into tuple?
I would say the similar problem requires you to answer similar questions. Do you need the vectors, and/or the tuples, to be in a usable state after you take the data? If so, then the std::swap approach seems to be the correct answer - anything else will just replicate the std::swap process manually, or do something even less efficient. You can create the tuple, swap the vectors in, then return the tuple. You don't need to 'move' it out, because (a) - that's what RVO is there for, and (b) since the tuple will be destroyed, more recent compilers can see that it's going out of scope and can generate the moves for you.
Personally this is starting to look like a pretty bad code smell though. If you're heavily reliant on pushing things into a structure, only to pull them out again en-masse, it sounds like you either want some sort of queuing system or a double-buffer system.
I watch videos from CPPCON15/16 and they keep saying, "return it by value" etc. Also the using refs or pointers to get output through arguments kinda seem to conflict with the idea of "returning". It seems to me that the "propper" use of the language is to use arguments as inputs, and the return as output... I don't want to create "bad code" at all, I just want good code, and I need to know how the language intends for me to return loads of data. It seems the move semantics are for this exact purpose. And to return anything more then one value I need to wrap it up in tuple or struct. And bonus STL containers has the move semantics built in so I don't have to worry about writing bad && operations.
There are so many parts to move semantics along the way that I feel I could get wrong, and if I mess up just one place I will be in deep copy hell and have a hard time figuring out where to apply the fix.
I could take a referenced tuple as a argument and std::swap with that, but... it just seems wrong.
As someone who is not yet up-to-date with modern C++ and move semantics, I understand your pain. But I think you've made things appear more complex than they actually are, due to your desire to implement queues/buffers via this sort of mechanism. Move semantics are for transferring ownership of an object. But when you move a vector, you move the whole vector, not just the contents. That's why it's the wrong solution for your buffer/queue situation. You need to keep a vector around in preparation for the next load of data - and that's where the old std::swap trick comes in - it basically does exactly the same as a std::move except it keeps your vector valid. And you can return the vector by value because the compiler will work out how to do that efficiently. (C++ compiler writers have been aware of this issue and did things to fix it well over a decade ago.)
Also, don't sweat these details until you know there's a problem. How many windows messages are you expecting per frame? 1? 2? Perhaps 10 if someone is particularly vigorous with the mouse? The amount of data being copied here is likely to be so small that copying it all might even be the cheapest approach. Allocating and deallocating vectors might be more expensive.
Move semantics are for transferring ownership of an object. But when you move a vector, you move the whole vector, not just the contents. That's why it's the wrong solution for your buffer/queue situation. You need to keep a vector around in preparation for the next load of data - and that's where the old std::swap trick comes in - it basically does exactly the same as a std::move except it keeps your vector valid. And you can return the vector by value because the compiler will work out how to do that efficiently.
Yes but is it the way the language intended to "transfer/move" data?
If returning a vector by value, what about this post above:
For the record; returning a local variable via move semantics is, per Effective Modern C++ (for C++11 & 14) by Scott Meyers, a bad idea and will more than likely disable RVO/NRVO and force the compiler in to a move instead of doing clever in place initialisation of the returned value.
Is that only because I was thinking about returning using std::move?
Would this be "proper" c++
std::tuple<
std::vector<std::string>, // vec0
std::vector<std::string>, // vec1
std::vector<std::string>, // vec2
std::vector<std::string> // vec3
> Game::takeOutput()
{
std::tuple<
std::vector<std::string>, // vec0
std::vector<std::string>, // vec1
std::vector<std::string>, // vec2
std::vector<std::string> // vec3
> localTuple;
std::swap(std::get<0>(localTuple), m_vec0);
std::swap(std::get<1>(localTuple), m_vec1);
std::swap(std::get<2>(localTuple), m_vec2);
std::swap(std::get<3>(localTuple), m_vec3);
return localTuple;
}
The C++ language doesn't have intent; it's a programming language, not a person. It provides tools for you to move data around in a method that suits you. Some of those methods are simple, some are fast, and some are in-between. The recent move/rvalue stuff is designed to give you an extra tool which strikes a better compromise between speed and complexity in some situations. (In the best case, you only get the extra speed, and in the worst case, you only get the extra complexity.)
std::move can get in the way of the compiler performing Named Value Optimisation, which, if you need to return something by value, is about as optimal as it gets, as it can avoid extra copies.
Regarding the question of whether your code is 'proper' C++; it looks to be legal and will perform your intended task.
Is it good C++? I'd say no; it looks ugly, and you've not convinced me that this is really something that you should be doing.
Is it optimal C++? Again I'd say no; even though you avoid copying strings, every call to takeOutput creates 4 new vectors, every subsequent object added goes in a completely new bit of memory (bad for the cache), and each vector (presumably) gets destroyed by whatever function called takeOutput once it is done with the contents.
Ultimately I would argue that it's not possible to give you the 'right' answer in the abstract here: it depends on how it will be used. That's why there are a bunch of different potantial approaches available. But it's probably no coincidence that existing libraries that wrap the OS message system (as in your original example), just yield up one message at a time - no copying of lists, no new data structures being allocated and deallocated, just writing the very simple message data in-place. (eg. https://wiki.libsdl.org/SDL_PollEvent, http://www.sfml-dev.org/documentation/2.0/classsf_1_1Window.php)
Yes but is it the way the language intended to "transfer/move" data? If returning a vector by value, what about this post above:
The C++ language assumes you understand what you're doing or not doing. It doesn't make choices for you, instead it mostly gives you primitives (options that you can use at your discretion), and very little policy or guidelines.
Even if it wanted to, it can't give guidelines in the general case, as "good use" is very different at different processor architectures, or even different machines.
Eg, a 16bit singleboard embedded system, versus an x64 desktop system, versus a Cray supercomputer.
In other words, you decide what is "good" for your case. Optimally, you do that based on knowledge from the system that you'll use for the software. In case of 'move' semantics, there is likely a break-even point somewhere, as I would guess returning a boolean is better done by value, and returning a 1GB vector is better done by move/swap.
If you don't know or don't care, just use the simplest solution until you find a counter-example where it makes a significant difference. At that time, adjust your view based on the experience.
Edit: Ninja'd by Kylotan
std::tuple<vec, vec, vec, vec> Game::takeOutput()
{
return { std::move(vec1), std::move(vec2), std::move(vec3), std::move(vec4) };
}
In this case you might be able to "get away with" doing the following;
Which indicates you don't care about the content of vec1...4 so move them in to the tuple, then return that by value which might allow RVO to kick in and directly move those vectors to the return value...std::tuple<vec, vec, vec, vec> Game::takeOutput() { return { std::move(vec1), std::move(vec2), std::move(vec3), std::move(vec4) }; }
maybe.
(but, then again I agree with the above that this feels like a bad code smell and probably not a sane way to go about doing things...)
Very interesting, but I think the issue begins when reusing the member vectors after they have moved.
Is it optimal C++? Again I'd say no; even though you avoid copying strings, every call to takeOutput creates 4 new vectors, every subsequent object added goes in a completely new bit of memory (bad for the cache), and each vector (presumably) gets destroyed by whatever function called takeOutput once it is done with the contents.
I could avoid creating 4 new vectors every time the method is called, but I would have to pass in the old ones and do the swaps. And they would have to be cleared before being reused. Is that not a "code smell"? Assuming a game refresh rate of 120 hz, are 480 vector creations per second "that bad" on a modern intel cpu? Maybe the compiler reuses behind the scenes?
Some of the std::strings in this example are placeholders for later when they will get swapped out by data such as "network data", "asset requests", "displaySettings" etc. I would also expand the output by adding more vectors to the tuple as the game gets more mature. I'm thinking the Game class should only keep track of game data and return only what the graphics class, window class, network class etc would need from it.
I'm not sure but from what I have learned I think this ain't all that bad:
class Game
{
public:
Game();
~Game();
void postInput(/*some vectors or a tuple maybe lol*/);
void update();
std::tuple<
std::vector<std::string>,
std::vector<std::string>,
std::vector<std::string>,
std::vector<std::string>
> takeOutput();
private:
std::tuple<
std::vector<std::string>, //m_vec0;
std::vector<std::string>, //m_vec1;
std::vector<std::string>, //m_vec2;
std::vector<std::string> //m_vec3;
> m_output
}
void Game::update()
{
// ...
// produce output
// ...
}
std::tuple<
std::vector<std::string>,
std::vector<std::string>,
std::vector<std::string>,
std::vector<std::string>
> Game::takeOutput()
{
std::tuple<
std::vector<std::string>,
std::vector<std::string>,
std::vector<std::string>,
std::vector<std::string>
> localTuple;
std::swap(localTuple, m_output);
return localTuple;
}
Or do I just not "get it"?
