assignment operator for a vector of unique_ptr's

Question

I am experimenting with smart pointers and try to exchange a few shared_ptr's with unique_ptr's in my project to see if I can gain little performance improvements.

I have the following code snippt:

// --- probably can be replaced with with std::copy ...
if (j != sentences.size()) {
    // also gives me an error if I initialize vector with a size.
    std::vector<unique_ptr<StemmedSentence> > newSentences(j);
    int base = 0;
    for (size_t i = 0; i < j; i++) {
        newSentences[base++] = std::move(sentences[i]); // std::move ??
    }
    sentences = newSentences; // std::move, i guess not !?
}

I copy a vector of pointers into a new vector (in this case a vector of unique_ptr's). Using shared_ptr, this is no problem at all. As I understand, shared pointer uses the implicit assignment, all is working fine. With unique_ptr though, I need to overload the assignment operator (to transfer ownership) and this gives me quiet a headache.

I created this assignment:

ArrayStemmedSnippet& operator=(const ArrayStemmedSnippet& other) {
    if (this != &other) {
        sentences.clear();
        std::vector<unique_ptr<StemmedSentence> >::const_iterator it;
        for (it = other.sentences.begin(); it != other.sentences.end(); ++it) {
            sentences.push_back(std::unique_ptr< StemmedSentence >(*it ? 
                *it : std::unique_ptr< StemmedSentence >())); // --- compiler error
        }
        return (*this);
    }
}

The pointers to copy can contain "Null" pointers. I try to check this in the for loop and copy either the pointer or an empty unique_ptr instance (a nullptr). That line marked with compiler error gives me a problem, I can not figure out the correct syntax. Also I think I have to create a new pointer (clone) if I copy a non-null object. Can somebody please give me a hint on that?

I found a similar post Handling smart pointers in stl container, the answer from Howard Hinnant gave me the inspiration to give it a try. But I can not use that syntax, I have gcc version 4.4.3 on my testmachine.

I have no clue if I will gain more performace, like I said, I experiment. As far as I know I would have no ownership problems in my code, so I thought making a test with unique_ptr and skip the shared_ptr (with all its overhead) might be a little gain in performance. I have about 5 objects where I use shared_ptr, so I would give it a try.

If all this makes no sense, let me know if it is worth playing arround with uniqe_ptr at all.

Thanks in advance!

Answer 1

The question makes little sense. The decision of using shared_ptr or unique_ptr should be taken based on whether ownership is shared or unique. The question is focusing on the details of forcing a change in the design that is probably not wanted.

In the original design, multiple vectors holding shared_ptr and sharing the ownership of some other objects, but in the modified approach, because unique_ptr has unique ownership you will be forced to copy the objects. The side effects of the change are multiple: now you need to copy the objects (higher cost). Because the type is a base, you need to implement a clone() approach so that the objects are not sliced, that will also have a (minimal) cost. After the copies are made the vectors will no longer refer to shared objects, so that a changed performed through one of them will not be visible through the rest, changing the semantics.

At this point, I can only start thinking whether you have profiled your code, and how much of the cost of the application is in the copying of the shared pointers, because that is not an expensive operation. My feeling is that you might be barking at the wrong tree, spending time unnecessarily and breaking your application in the process, with a much more obscure approach for nothing.

Sorry to sound negative, but I feel that you should really think this through. unique_ptr is not a replacement to shared_ptr, unless shared_ptr was the wrong solution from the start (objects should not really be shared), but that does not seem the case, since in your current program you are copying the pointers to different containers, and thus sharing.

---

If what you want is to compact the vector, you don't need copies at all, or even coding:

// [untested, but you can build on top of this]
typedef std::vector< std::unique_ptr< StemmedSentence > > vector_t;

vector_t v = ... // build the vector
// compact:
v.erase( std::remove( std::make_move_iterator( v.begin() ),
                      std::make_move_iterator( v.end() ),
                      std::unique_ptr<StemmedSentence>() 
                    ).base(),
         v.end() );

But this should not be implemented as operator= on the enclosing type taking a constant reference. unique_ptr cannot share ownership, so if the pointers are moved to the new container, then the old container is going to be emptied (this could fit a moving operator=, but not one that promises not to modify the original container.