Hacker News Reader

Comment by corysama | Hacker News Reader

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()"

View on HN

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.

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.

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.

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.

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).

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?

I used to do the same thing, until I saw slides ( https://stlab.adobe.com/wiki/images/8/85/2008_06_26_classes_... ) for a talk ( 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.

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

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

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.

You can do const shared_ptr<const type>, it's like a const * const raw pointer.

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-poi...

If you're going to pass it by const_ref, why not just pass the actual stored object by const_ref instead?

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_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.

> 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?

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.

You are correct. I had a brain fart. Edited my comment.

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.

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.

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.

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.

Reply on news.ycombinator.com