12

Should a shared pointer be passed by reference or by value as a parameter to a class if it is going to be copied to a member variable?

The copying of the shared pointer will increment the refrence count and I don't want to make any unnecessary copies and thus ref count increments. Will passing the shared pointer as a refrence solve this? I assume it does but are there any other problems with this?

Passing by value:

class Boo {
public: 
    Boo() { }
};

class Foo {
public:
    Foo(std::shared_ptr<Boo> boo) 
        : m_Boo(boo) {}
private:
    std::shared_ptr<Boo> m_Boo;
};

int main() {
    std::shared_ptr<Boo> boo = std::make_shared<Boo>();

    Foo foo(boo);
}

Passing by refrence:

class Boo {
public: 
    Boo() { }
};

class Foo {
public:
    Foo(std::shared_ptr<Boo>& boo) 
        : m_Boo(boo) {}
private:
    std::shared_ptr<Boo> m_Boo;
};

int main() {
    std::shared_ptr<Boo> boo = std::make_shared<Boo>();

    Foo foo(boo);
}
O'Neil
  • 3,790
  • 4
  • 16
  • 30
Signekatt
  • 123
  • 1
  • 5
  • 3
    In most cases, you should not use smart pointers as parameters at all; there are some exceptions, though, and your example is one of these. I recommend having a look at Herb Sutter's [GotW 91](https://herbsutter.com/2013/05/30/gotw-91-smart-pointer-parameters/) for getting some general info about... – Aconcagua Apr 11 '18 at 13:35
  • @Aconcagua The reason I need to store a shared pointer to an object is to make sure that it's not destroyed before the objects that need it are. But thank you for the article. – Signekatt Apr 11 '18 at 14:01

2 Answers2

20

Pass it by value then move it into the member:

class Foo {
public:
    Foo(std::shared_ptr<Boo> boo) 
        : m_Boo(std::move(boo)) {}
private:
    std::shared_ptr<Boo> m_Boo;
};

This will be the most efficient in all cases - if the caller has a rvalue-reference then there wont be a single add-ref, if the caller has a value there'll be a single add-ref.

If you pass by const& you force an add-ref even in cases where its unnecessary. If you pass by value and then set without a std::move you may get 2 add-refs.

Edit: This is a good pattern to use if you've got a class where a move is significantly cheaper than a copy, and you have a function call which will always copy the instance passed in - as in this case. You force the copy to happen at the function call boundary by taking it by value, and then if the caller has a rvalue reference the copy need never happen at all - it will be moved instead.

Mike Vine
  • 9,468
  • 25
  • 44
  • 5
    Nevertheless, even a move is not free. For the simple function `std::shared_ptr foo(std::shared_ptr arg) { return std::move(arg); }`, `g++` generates six memory accesses: Two to read the argument, two to null the argument, and two to construct the result. That's rather heavy on the memory bus (the CPU will be twiddlings its thumbs in the mean time). Of course, the move (local cache operation) is cheaper than bumping an atomic ref count (which has inter-CPU overheads), so this use of moving is very ok. Nevertheless, there are cases where passing by `const &` is faster than moving. – cmaster - reinstate monica Apr 11 '18 at 14:02
-3

shared_ptr should be treated just like any other class when passing to functions. in this case you should pass to const reference.

why i wrote why i wrote Herb Sutter jump to min 21:50 https://www.youtube.com/watch?v=xnqTKD8uD64

Arkady Godlin
  • 588
  • 2
  • 9
  • If the shared_ptr is passed as a const reference, the constructor will be unable to participate with the shared ownership of the object, thus defeating the entire point. – Eljay Apr 11 '18 at 14:03
  • the constructor will be able to take ownership if and the ref count will be 2 in this case. but now i understand that when you always want to take ownership it is better to accept by value. – Arkady Godlin Apr 11 '18 at 14:05
  • 1
    @Eljay I don't know, what you are talking about: With a `const &` argument, the object is guaranteed to persist as long as the function is executing. And the constructor is free to copy-construct its own `shared_ptr<>` from it to take part in the ownership. – cmaster - reinstate monica Apr 11 '18 at 14:07
  • 1
    As a matter of fact, I do not understand the downvotes on this answer: Passing by `const &` is not exactly wrong, and it has been the best answer for a long time. And there are still cases where `const &` passing beats move constructing any time. I wouldn't upvote this answer because move constructing is usually better, but downvoting it seems a bit unfair to me. – cmaster - reinstate monica Apr 11 '18 at 14:10
  • @cmaster The question is how to pass a `shared_ptr` into a constructor where it'll be copied into a member variable. In that case `const&` is never better, sometimes worse. There are different cases where `const&` is better, but that is not this question. Its why I made an edit in my answer to be clear why its better in this case (and how to recognise this case). – Mike Vine Apr 11 '18 at 16:06
  • 1
    @MikeVine The case where `const &` is better is this: Some object is an owner of some instance of `Boo` via a `shared_ptr<>`, and is asked to generate a `Foo` that references the same instance. Passing by value and moving means a) constructing a `shared_ptr<>` to be passed to the constructor, b) moving that argument over to the member variable. Passing by `const &` means a) passing a pointer to the existing `shared_ptr<>` to the constructor, b) constructing the member variable directly from the original `shared_ptr<>`. Passing a single pointer is definitely faster than moving a `shared_ptr<>`. – cmaster - reinstate monica Apr 11 '18 at 16:16
  • @ArkadyGodlin • I was under the misimpression that the `const&` would apply to the shared_ptr (as an object itself - the smart pointer). That is not the case, I stand corrected, as per... https://stackoverflow.com/questions/44830481/is-use-count-on-const-stdshared-ptr-mutable – Eljay Apr 11 '18 at 22:25