1

I have a Visual Studio 2008 C++ application where a function Foo::CreateBar uses an internal function to populate a buffer with some data as below. I understand that VS2008 will use return value optimization (RVO) to ensure the Foo::GetData() call will not incur a copy, but will I incur copies for the Bar constructor? How about for returning the Bar object from Foo::CreateBar Is there a more efficient way to do this? Do I need to redefine my Buffer as boost::shared_ptr< std::vector< BYTE > >?

typedef std::vector< BYTE > Buffer;

class Bar
{
public:
    explicit Bar( Buffer buffer ) : buffer_( buffer ) { };

    // ...

private:
    Buffer buffer_;
};

class Foo
{
public:
    Bar CreateBar() const { return Bar( GetData() ); };

    // ...

private:

    static Buffer GetData()
    {
        Buffer buffer;
        // populate the buffer...
        return buffer;
    };
};

Thanks, PaulH

Improve this question
4
  • 1
    Have you proved by measurement that this is a real problem? Commented Apr 25, 2011 at 19:29
  • 1
    @unapersson - are redundant copies of data buffers or classes ever NOT a problem? This is a simple example, but the lesson applies more widely. Commented Apr 25, 2011 at 19:41
  • @Steve To know that we would have to have an encompassing definition of "redundant" - nothing in the OP's posted code indicated the copies are redundant. For example, is the copy returned by std::string's operator+() redundant? I would say not. Commented Apr 25, 2011 at 19:47
  • @unapersson - agreed, I've commented elsewhere that what is 'best' depends on the design goals. In general, I favour least possible number of copies - perhaps that's a clearer statement of my meaning? Commented Apr 25, 2011 at 19:49

6 Answers 6

Reset to default
3

You can answer this question for sure by examining the assembler code that's generated for this source - modify the compiler output options to produce a listing interlaced with the source code.

shared_ptr is not appropriate, in any case. That's a device for the programmer to manage designs requiring shared objects efficiently, not to fool the compiler into not constructing more objects than it should.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

+1… I'd go further, and say not to address the problem until looking at the assembly-interlaced-with-source listing in the profiler.
2

Using a const reference as parameter of the Bar constructor

explicit Bar( const Buffer & buffer ) : buffer_( buffer ) { };

will surely avoid you the copy caused by the pass-by-value in the constructor call, but you will have a copy in the construction of the buffer_ field.

Improve this answer

3 Comments

Not really, not necessarily. Some compilers (I don't know the details of VS2008) will elide that copy, basically using the same memory address for the variable inside GetData, the returned value from the function, and the argument value in the constructor, leaving a single copy. As a matter of fact, if you default construct and swap (for types like vector that implement an efficient swap you might avoid all copies.
@David: I had the same doubt about that, since, the const reference is a guarantee that the copy due to the constructor call won't be done, but it may interfere with the second potential optimization you say.
Literature in the last few years (were few is not that few, man, am I growing old) recomments passing by value if you are to make a copy yourself (generally implementing T& operatorX( T tmp, T const& rhs ) as { return tmp X= rhs; } In this particular case it is trickier, as the copy is not performed in the stack (the member might be dynamically allocated) so the compiler cannot really elide the internal copy (i.e. cannot place the argument and the member in the same location), but that last copy can be manually avoided with swap
1

shared_ptr would work. unique_ptr would work, because ownership is transferred.

You could also use out parameters to avoid the copy. Add a factory function...

class Bar
{
public:
    explicit Bar( void (*factory)(Buffer& buffer) ) { factory(buffer_); }

    // ...

private:
    Buffer buffer_;
};

class Foo
{
public:
    Bar CreateBar() const { return Bar( GetData ); }

    // ...

private:

    static void GetData(Buffer& buffer)
    {
        // populate the buffer...
    }
};
Improve this answer

3 Comments

That's clever. I hadn't considered doing that.
This is a clever approach if you always construct by calling a function with that exact signature. It could be made more general with std::function (or boost equivalent in VS08), but even then it will force only creating Bar objects from Buffer factories.
@David: of course, you can have both versions of the constructor, one more efficient than the other.
0

You should make a couple of different tests and see what your compiler does with it. In the code above there are as many as 4 copies of the buffer:

  1. Inside GetData
  2. Return value of GetData
  3. Argument of constructor
  4. Member

The compiler by doing RVO/NRVO can merge 1 and 2 into a single object. Moreover it can merge those two objects with 3 by placing the three objects in the same memory location. What it cannot do is place that object and 4 in the same memory address (in the general case).

On that particular copy, you might elide it by default initializing the internal member vector (usually no cost) and swapping the contents of the argument with the member:

explicit Bar( Buffer buffer )  // pass by value if your compiler optimizes it
   : buffer_()                 // default construct: no cost
{
   buffer_.swap( buffer );     // swap: no cost 
};

Also note that this is the whole reason for rvalue references in the upcoming standard, in C++0x (I know that is not available in VS08) the compiler will use move semantics to avoid the copies while maintaining the code readable (readable as in closest to the intentions than to tricks to avoid the cost).

Improve this answer

Comments

0

You will definitely incur a copy for the buffer.

Why do you want to use a shared pointer? Is it a shared data? If so - then go for it. If not - than you have to copy. We don't really know what is your design, do we?

Improve this answer

5 Comments

My design is that I want to do Bar b = foo.CreateBar(); while incurring as few copies as possible.
Yes, but do you want to have a copy of the data or not? You see, you can use a shared pointer, or you can copy. But if you don't know what each of them means - how can you choose?
@PaulH - sure, but you still need to understand exactly how the memory is used in your classes. Do you want a copy of your original buffer, or do you want a single buffer that's attached to the target Bar? Different design goals, different source code.
@Steve & littleadv - A single buffer attached to the target Bar is the goal.
@PaulH - then I would use boost::shared_array<BYTE>, for minimum inconvenience and copying - see boost.org/doc/libs/1_46_1/libs/smart_ptr/shared_array.htm If you need to change the data size then shared_ptr to vector works.
0

If you're sensitive to copy operations, you may want to force copy operations to be explicit or even prevent them altogether. If you make the copy constructor and assignment operator protected/private (or use boost::noncopyable) the compiler will produce an error whenever it tries to generate code that copies the object. Of course if you do want to make copies sometimes you'll need to provide a method for explicitly doing so, but that ensures that you never accidentally introduce a copy operation.

Improve this answer

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.