Reputation: 787
I'm creating an example to try to understand how the move semantics is optimized when using c++11.
First, I have the following two functions that take some input and return a modified version of the input.
std::vector<float> abs(const std::vector<float> &v)
{
std::vector<float> abs_vec = v;
for (auto &val: abs_vec) {
auto i = &val - &abs_vec[0];
abs_vec[i] = std::abs(abs_vec[i]);
}
return abs_vec;
}
std::vector<float> normalize(const std::vector<float> &v)
{
std::vector<float> norm_vec = v;
auto max_val = std::max_element(std::begin(norm_vec), std::end(norm_vec));
auto min_val = std::min_element(std::begin(norm_vec), std::end(norm_vec));
auto norm_factor = *max_val;
auto min_check = std::abs(*min_val);
if (min_check > std::abs(norm_factor)) {
norm_factor = min_check;
}
for (auto &val: norm_vec) {
auto i = &val - &norm_vec[0];
norm_vec[i] = norm_vec[i] / norm_factor;
}
return norm_vec;
}
Then I test the function with the following code:
int main()
{
std::vector<float> tmp{-1,2,-3,4,5};
printf("pointer before: %p\n", &tmp[0]);
tmp = normalize(tmp);
printf("pointer after normalize: %p\n", &tmp[0]);
tmp = abs(tmp);
printf("pointer after abs: %p\n", &tmp[0]);
};
From this I get the output:
pointer before: 0x156dc20
pointer after normalize: 0x156e050
pointer after abs: 0x156dc20
Could anyone please explain what is going on and why it seems like I get a copy after normalization, but then the same memory location as the original after the abs function call?
Is there a better way to write functions in a functional style like this that would lead to more optimized code?
Upvotes: 2
Views: 285
Reputation: 3510
When you make a new copy from a const-reference that can't be a move, obviously. If you want to write these functions in a way where the vector gets moved in and out of them, you need to transfer the vectors by value (should be preferred here) or rvalue-reference, and move the vector into the function.
#include <cstdio>
#include <vector>
std::vector<float> plus_five(std::vector<float> v)
{
for (auto& e : v)
e += 5;
return v;
}
int main()
{
std::vector<float> v{1, 2, 3, 4};
std::printf("%p\n", v.data());
v = plus_five(std::move(v)); // move
std::printf("%p\n", v.data());
v = plus_five(v); // copy
std::printf("%p\n", v.data());
}
Upvotes: 3
Reputation: 66200
The value you printf
is the address of the underlying data member of the vector.
So I suppose that, in your case
1) the creation of tmp
create (allocate) a data member in position 0x156dc20
2) the normalize()
operation substitute the data member with the data member of the vector created in normalize()
(norm_vec
), with address 0x156e050, and free the memory of the previous data member
3) the abs()
operation substitute the data member with the data member of the vector
created in abs()
(abs_vec
), that is again 0x156dc20, recycling the previously freed memory
Briefly: I suppose that abs_vect
recycle the memory previously used by tmp
En passant, your for-each loop are over-complicated.
The loop
for (auto &val: norm_vec) {
auto i = &val - &norm_vec[0];
norm_vec[i] = norm_vec[i] / norm_factor;
}
can be simplified (you're using a reference (&
)) as
for (auto &val: norm_vec)
val /= norm_factor;
and the loop
for (auto &val: abs_vec) {
auto i = &val - &abs_vec[0];
abs_vec[i] = std::abs(abs_vec[i]);
}
can be simplified (same reason) as
for (auto &val: abs_vec)
val = std::abs(val);
The use of std::transform
, as suggested by Andy, is (IMHO) even better.
Regarding the finding of norm_factor
, i think that
auto pairIt = std::minmax_element(norm_vec.begin(), norm_vec.end());
auto norm_factor = std::max(std::abs(*(pairIt.first)), *(pairIt.second));
is simpler (and more optimized, I suppose) than
auto max_val = std::max_element(std::begin(norm_vec), std::end(norm_vec));
auto min_val = std::min_element(std::begin(norm_vec), std::end(norm_vec));
auto norm_factor = *max_val;
auto min_check = std::abs(*min_val);
if (min_check > std::abs(norm_factor)) {
norm_factor = min_check;
}
p.s.: sorry for my bad English
Upvotes: 2
Reputation: 30418
I'm not sure why the memory address of the first element is the same after calling abs()
, but there is a better way to write it. The code would be simpler if you use std::transform()
, available in the algorithm
header:
std::vector<float> abs(const std::vector<float> &v)
{
std::vector<float> abs_vec;
std::transform(v.begin(), v.end(), std::back_inserter(abs_vec),
[] (float val) {
return std::abs(val);
});
return abs_vec;
}
This will iterate over all of the elements in v
, and for each one will call the lambda function. The return value of that function will be inserted onto the end of abs_vec
.
The loop at the end of normalize()
could also be rewritten to use std::transform()
, as well. In this case, the norm_factor
variable can be captured so it can be used within the lambda:
std::transform(v.begin(), v.end(), std::back_inserter(norm_vec),
[norm_factor] (float val) {
return val / norm_factor;
});
Upvotes: 3