I'm trying to figure out the optimal way to do this assignment.
Right now I'm just doing a simple assignment, but was wondering if using std::swap() would be better or if maybe there is just a better way to write this code so that I can replace my initial list with what I build up during the validation pass.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19
std::vector<int> tets_to_use;
//... add entries to tets_to_use
//Validate
while (true) {
std::vector<int> tets_validated;
for (int tet_idx: tets_to_use) {
if (is_valid(tet_idx))
tets_validated.push_back(tet_idx);
}
if (tets_validated.size() == tets_to_use.size())
break;
//Something was not valid - try again with reduced list
tets_to_use = tets_validated;
}
Also consider remove_if() on tets_to_use. You're not trying to set a std::vector to another std::vector - you're removing elements that don't satisfy a criteria.
If you want to assign one vector to another, and if the "source" vector is not needed anymore after this assignment, then I think a move-assignment would be the best (most efficient) choice here:
vec1 = std::move(vec2);
Normal assignment leaves the "source" vector unchanged, so the data actually needs to be duplicated into the "destination" vector. That is inefficient, if you don't need the "source" vector anymore after the assignment (as is the case here). Meanwhile, move-assignment allows the "destination" vector to steal the data from the "source" vector, leaving the "source" vector in a valid but unspecified state.
(However, I'm not sure whether compiler optimizations would already turn the regular assignment into a move-assignment automatically for you in this situation 🤷)
Furthermore, I think an std::swap() pretty much does the same thing as a move-assignment, only that it happens in both directions at the same time: Vector a steals the data from vector b, while vector b steals the data from vector a. That is probably okay, performance-wise, but I think an explicit move-assignment is more clear, because it more clearly expresses what your intention is.
The outermost loop is what bothers me most. Assuming that there is at least one invalid input, it runs us through the whole vector of previously validated data one extra time for no reason. That is easily the most inefficient part of the original code.
(Please delete line 6, 14, 15, and line 19)
As far as I can tell, the assignment operator for vectors (vec1 = vec2) is highly optimized, when I actually run tests on 100,000,000 data points I could not beat it using Seeplus's method, though Kigar's vec1 = std::move(vec2) function did tie it.
On my system the program using Kigar's method took 4.98 seconds while Seeplus's took 5.45. Keep in mind that setting up the random values takes about 3.54 seconds, so subtract that to get a more accurate picture.
I suspect Seeplus's version might run better in real-world data where invalid numbers are likely to be grouped near each other (such as in audio processing).
I'm using the G++ compiler on Linux, on other operating systems the system libraries might run differently. I assume Windows and Mac compilers would have similar optimizations though.
One surprise to me was that when I tested using lambdas the program took 2 tenths of a second longer to finish than using the equivalent function. Perhaps lambdas are more useful in larger programs where the functions may be loaded further away from the executing code in memory, and this code is so small that it's all loading within the same "page".
TLDR: You are doing fine, just use the equal sign.