7

Consider the following vector:

std::vector<std::shared_ptr<X>> myVector;

and the following two functions that add a given element to the vector:

void foo1(std::shared_ptr<X> x)
{
    myVector.push_back(x);
}

void foo2(const std::shared_ptr<X>& x)
{
    myVector.push_back(x);
}

My understanding is that both functions push a shared_ptr to X into the vector and thus increment the ref count of X. The first function results in an additional increment and decrement of the reference count but this is needless.

Is my understanding correct? Is the second option therefore preferable?

ksl
  • 4,519
  • 11
  • 65
  • 106

3 Answers3

5

In foo1 you pass the parameter (i.e., shared pointer) by value. Thus, the copy constructor of std::shared_ptr<X> is going to be evoked (i.e., ref counter is going to be increased and then decreased when destructor of local copy is being called at } of foo1).

In foo2 you pass the parameter (i.e., shared pointer) by const reference. Thus, you pass a const qualified alias of the original object (i.e., ref counter is not going to be increased).

You can also see this in the following example:

struct X {};

void foo1(std::shared_ptr<X> x) { 
  std::cout << "count in foo1(): " << x.use_count() << std::endl;
}

void foo2(const std::shared_ptr<X>& x) {
  std::cout << "count in foo2(): " << x.use_count() << std::endl;
}

int main() {
  std::shared_ptr<X> x(new X);
  std::cout << "count in main(): " << x.use_count() << std::endl;
  foo1(x);
  foo2(x);
}

Output:

count in main(): 1
count in foo1(): 2
count in foo2(): 1

As you can see in foo1 the number of different shared_ptr instances is 2. That is the original shared_ptr defined in main and the copy in foo1. Whereas, in foo2 the ref counter remains 1.

Consequently, your reasoning is correct.

101010
  • 41,839
  • 11
  • 94
  • 168
  • When wouldn't you use option 2? – ksl Nov 06 '15 at 12:25
  • 1
    @ksl basically I would use option 2 most of the time. I would use option 1 only if I wanted at some point of the function the copied shared pointer to point to somewhere else. – 101010 Nov 06 '15 at 12:59
3

You should rewrite your functions as:

void foo1(std::shared_ptr<X> x)
{
    myVector.emplace_back(std::move(x));
}

void foo2(const std::shared_ptr<X>& x)
{
    myVector.emplace_back(x);
}

then both versions should be more efficient, which one would depend on situation and particular compiler, most probably difference would be very small if any.

Slava
  • 43,454
  • 1
  • 47
  • 90
1

Generally it is better to pass shared pointer by reference, as atomic increments tend to degrade performance. In your case, it will not be noticeable, since you copy it afterwards anyways.

SergeyA
  • 61,605
  • 5
  • 78
  • 137