6 ms·

Not mentioned is passing shared_ptrs as parameters by value. People do it all the time because "passing pointer parameters is as cheap as it gets, right?" But

Not mentioned is passing shared_ptrs as parameters by value. People do it all the time because "passing pointer parameters is as cheap as it gets, right?" But, shared_ptr's thread safety means that each copy involves a full trip past all cache levels to actual DRAM. 100+ cycles each. They are not pointers. They are small yet heavy objects and should be passed by reference. Copying implies assuming shared ownership and should not be done lightly. Also, "if(!p->expired()) p->lock()->foo()" is not thread safe. The last shared_ptr could die on another thread between the test and the lock. Instead, what you want is "if (auto q = p->lock()) q->foo()"

sneha87 10y ago
Interesting point about shared_ptr. Do you think shared_ptrs should be passed by const reference or just reference ?
- corysama 10y ago
  I'm a "const all the things!" coder in general. If you see something that's not const, that is documentation that you should expect your object to be modified, not that I was too lazy to type 'const' ;P
  - millstone 10y ago
    Unfortunately a const shared_ptr is like a const raw pointer: you can't change the pointer itself, but can still modify what it points to.
    - Too 10y ago
      You can do const shared_ptr<const type>, it's like a const * const raw pointer.
- sharth 10y ago
  If you're going to pass it by const_ref, why not just pass the actual stored object by const_ref instead?
  - corysama 10y ago
    Edit: Ignore this comment. I shouldn't talk so much about something I use so little. Somehow I skimmed through http://en.cppreference.com/w/cpp/memory/shared_ptr/shared_ptr http://en.cppreference.com/w/cpp/memory/shared_ptr/shared_pt... and thought that shared_ptr( const shared_ptr& r ); was missing. >> That's a good point. If the function will eventually lead to some object retaining a reference, then you should use a non-const shared_ptr ref. The assignment into the object will do the copy constructor and that needs a non-const ref. But, you don't need to be making temp object copies along the way. If the function will not lead to something retaining a reference, then you shouldn't be passing the reference retention object to it. Just pass a direct ref to the target object.
    - tomjakubowski 10y ago
      > The assignment into the object will do the copy constructor and that needs a non-const ref. Hum? What assignment into the object? Where I'd typically see taking a `const std::shared_ptr<> &` to signal "retaining ownership" would be something like this: class Foo { public: void AppendChild(const std::shared_ptr<Foo> &x) { children_.emplace_back(x); } private: std::vector<std::shared_ptr<Foo>> children_; }; Why should Foo:;AppendChild's signature be changed to a non-const ref?
      - corysama 10y ago
        You are correct. I had a brain fart. Edited my comment.
      - millstone 10y ago
        It should be changed to pass by value, and then use std::move to move it into the children array. The reason is that if the argument is a temporary, you are making an unnecessary copy of that temporary, when you could just transfer ownership. This also gives callers the flexibility to std::move into your argument, transferring ownership. A good rule of thumb is, if you are going to unconditionally take ownership of an object, accept it by value.
- optforfon 10y ago
  Typically you should pass raw pointers (constness depends on your use case...). I can't really think of a scenario where the method/function needs to be coupled with the pointer type.
  - MereInterest 10y ago
    I will often have functions that accept a unique_ptr as a parameter. This way, I can clearly encode that the object belongs to the callee now.
  - zanny 10y ago
    And if you are wanting to pass the raw pointer, you should probably pass by & instead as well. The only real "features" of raw pointers over references are the ability to reassign or delete them, and if you are passing something by reference, you do not want anyone doing any such thing to it 99.9% of the time.
- Kristine1975 10y ago
  Unless they are modified: const reference. Otherwise you cannot pass a temporary object as in: void foo(const std::shared_ptr<int>& p); ... foo(std::make_shared<int>(23)); This will not compile with void foo(std::shared_ptr<int>& p); since a temporary object cannot be implicitly converted to a non-const reference, only to a const one.
- pveierland 10y ago
  Herb Sutter's GotW #91 talks about smart pointer parameters in detail and when to use const and reference: https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/ https://herbsutter.com/2013/06/05/gotw-91-solution-smart-poi...
jstarks 10y ago
Atomic memory accesses do not require touching DRAM in the general case. The cost of an atomic operation on a modern Intel CPU is closer to 20 cycles than 100. You will see higher costs if you have two processors trying to access the same shared object concurrently, of course. Your point is still valid, though -- a shared_ptr should be copied only when you actually want to increase the reference count. In general, I try to avoid shared_ptr unless I really need reference counting. unique_ptr is much easier to reason about and is much more efficient.
- hellofunk 10y ago
  I agree, and I frequently use unique_ptr only, and then pass naked pointers to non-owning objects who need a reference. In this way, the code is clear: if it is a regular pointer, it is non-owning always. Some might argue that using shared pointers is better for this application, but then you must also deal with potential reference cycles and add weak pointers to the mix eventually, and in addition to these concerns, handle any runtime hits for the reference counting.
  - barrkel 10y ago
    Life is easy when there's an unambiguous single owner. Shared pointers are really for the harder situation where ownership is distributed. I prefer to use arenas if possible, when objects live in a complicated graph.
  - zanny 10y ago
    It is definitely better to use T& instead of shared_ptr<T>&. Passing references to either unique or shared pointers is just adding a layer of indirection you are never going to use, unless you intend to copy-construct the pointer object later, which sounds more like you need to rethink your object design if you want to do something that messy.
    - hellofunk 10y ago
      To be clear, I was not referring to references at all but rather to naked pointers you get by calling get() on a unique_ptr, and passing those to non-owning classes/functions that need them (including third-party APIs).
sytelus 10y ago
I generally prefer to pass by const ref. If function needs to make copy, it can always invoke copy constructor. If it doesn't then all is well and performant. I know there are optimizations that can happen if you pass by value and function intends to make a copy. But those are micro optimizations and I've always felt its not worth the confusion it creates to the caller as well as callee. Does anyone else follow this general rule of thumb?
- maxlybbert 10y ago
  I used to do the same thing, until I saw slides ( https://stlab.adobe.com/wiki/images/8/85/2008_06_26_classes_that_work.pdf https://stlab.adobe.com/wiki/images/8/85/2008_06_26_classes_... ) for a talk ( https://my.adobeconnect.com/p53888531/?launcher=false&fcsContent=true&pbMode=normal https://my.adobeconnect.com/p53888531/?launcher=false&fcsCon... ) that pointed out that explicitly making the copy makes it much harder for the compiler to do copy elision.

sneha87 10y ago

Interesting point about shared_ptr. Do you think shared_ptrs should be passed by const reference or just reference ?

corysama 10y ago
I'm a "const all the things!" coder in general. If you see something that's not const, that is documentation that you should expect your object to be modified, not that I was too lazy to type 'const' ;P
- millstone 10y ago
  Unfortunately a const shared_ptr is like a const raw pointer: you can't change the pointer itself, but can still modify what it points to.
  - Too 10y ago
    You can do const shared_ptr<const type>, it's like a const * const raw pointer.
sharth 10y ago
If you're going to pass it by const_ref, why not just pass the actual stored object by const_ref instead?
- corysama 10y ago
  Edit: Ignore this comment. I shouldn't talk so much about something I use so little. Somehow I skimmed through http://en.cppreference.com/w/cpp/memory/shared_ptr/shared_ptr http://en.cppreference.com/w/cpp/memory/shared_ptr/shared_pt... and thought that shared_ptr( const shared_ptr& r ); was missing. >> That's a good point. If the function will eventually lead to some object retaining a reference, then you should use a non-const shared_ptr ref. The assignment into the object will do the copy constructor and that needs a non-const ref. But, you don't need to be making temp object copies along the way. If the function will not lead to something retaining a reference, then you shouldn't be passing the reference retention object to it. Just pass a direct ref to the target object.
  - tomjakubowski 10y ago
    > The assignment into the object will do the copy constructor and that needs a non-const ref. Hum? What assignment into the object? Where I'd typically see taking a `const std::shared_ptr<> &` to signal "retaining ownership" would be something like this: class Foo { public: void AppendChild(const std::shared_ptr<Foo> &x) { children_.emplace_back(x); } private: std::vector<std::shared_ptr<Foo>> children_; }; Why should Foo:;AppendChild's signature be changed to a non-const ref?
    - corysama 10y ago
      You are correct. I had a brain fart. Edited my comment.
    - millstone 10y ago
      It should be changed to pass by value, and then use std::move to move it into the children array. The reason is that if the argument is a temporary, you are making an unnecessary copy of that temporary, when you could just transfer ownership. This also gives callers the flexibility to std::move into your argument, transferring ownership. A good rule of thumb is, if you are going to unconditionally take ownership of an object, accept it by value.
optforfon 10y ago
Typically you should pass raw pointers (constness depends on your use case...). I can't really think of a scenario where the method/function needs to be coupled with the pointer type.
- MereInterest 10y ago
  I will often have functions that accept a unique_ptr as a parameter. This way, I can clearly encode that the object belongs to the callee now.
- zanny 10y ago
  And if you are wanting to pass the raw pointer, you should probably pass by & instead as well. The only real "features" of raw pointers over references are the ability to reassign or delete them, and if you are passing something by reference, you do not want anyone doing any such thing to it 99.9% of the time.
Kristine1975 10y ago
Unless they are modified: const reference. Otherwise you cannot pass a temporary object as in: void foo(const std::shared_ptr<int>& p); ... foo(std::make_shared<int>(23)); This will not compile with void foo(std::shared_ptr<int>& p); since a temporary object cannot be implicitly converted to a non-const reference, only to a const one.
pveierland 10y ago
Herb Sutter's GotW #91 talks about smart pointer parameters in detail and when to use const and reference: https://herbsutter.com/2013/06/05/gotw-91-solution-smart-pointer-parameters/ https://herbsutter.com/2013/06/05/gotw-91-solution-smart-poi...

jstarks 10y ago

Atomic memory accesses do not require touching DRAM in the general case. The cost of an atomic operation on a modern Intel CPU is closer to 20 cycles than 100. You will see higher costs if you have two processors trying to access the same shared object concurrently, of course. Your point is still valid, though -- a shared_ptr should be copied only when you actually want to increase the reference count. In general, I try to avoid shared_ptr unless I really need reference counting. unique_ptr is much easier to reason about and is much more efficient.

hellofunk 10y ago
I agree, and I frequently use unique_ptr only, and then pass naked pointers to non-owning objects who need a reference. In this way, the code is clear: if it is a regular pointer, it is non-owning always. Some might argue that using shared pointers is better for this application, but then you must also deal with potential reference cycles and add weak pointers to the mix eventually, and in addition to these concerns, handle any runtime hits for the reference counting.
- barrkel 10y ago
  Life is easy when there's an unambiguous single owner. Shared pointers are really for the harder situation where ownership is distributed. I prefer to use arenas if possible, when objects live in a complicated graph.
- zanny 10y ago
  It is definitely better to use T& instead of shared_ptr<T>&. Passing references to either unique or shared pointers is just adding a layer of indirection you are never going to use, unless you intend to copy-construct the pointer object later, which sounds more like you need to rethink your object design if you want to do something that messy.
  - hellofunk 10y ago
    To be clear, I was not referring to references at all but rather to naked pointers you get by calling get() on a unique_ptr, and passing those to non-owning classes/functions that need them (including third-party APIs).

sytelus 10y ago

I generally prefer to pass by const ref. If function needs to make copy, it can always invoke copy constructor. If it doesn't then all is well and performant. I know there are optimizations that can happen if you pass by value and function intends to make a copy. But those are micro optimizations and I've always felt its not worth the confusion it creates to the caller as well as callee. Does anyone else follow this general rule of thumb?

maxlybbert 10y ago
I used to do the same thing, until I saw slides ( https://stlab.adobe.com/wiki/images/8/85/2008_06_26_classes_that_work.pdf https://stlab.adobe.com/wiki/images/8/85/2008_06_26_classes_... ) for a talk ( https://my.adobeconnect.com/p53888531/?launcher=false&fcsContent=true&pbMode=normal https://my.adobeconnect.com/p53888531/?launcher=false&fcsCon... ) that pointed out that explicitly making the copy makes it much harder for the compiler to do copy elision.