toeplitz
toeplitz

Reputation: 787

C++11, move semantics of returning vectors functional style

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

Answers (3)

cooky451
cooky451

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

max66
max66

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

Andy
Andy

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

Related Questions