63

I have an application that is performing some processing on some images.

Given that I know the width/height/format etc. (I do), and thinking just about defining a buffer to store the pixel data:

Then, rather than using new and delete [] on an unsigned char* and keeping a separate note of the buffer size, I'm thinking of simplifying things by using a std::vector.

So I would declare my class something like this:

#include <vector>

class MyClass
{
    // ... etc. ...

public:
    virtual void OnImageReceived(unsigned char *pPixels, 
        unsigned int uPixelCount);

private:
    std::vector<unsigned char> m_pImageBuffer;    // buffer for 8-bit pixels

    // ... etc. ...
};

Then, when I received a new image (of some variable size - but don't worry about those details here), I can just resize the vector (if necessary) and copy the pixels:

void MyClass::OnImageReceived(unsigned char *pPixels, unsigned int uPixelCount)
{
    // called when a new image is available
    if (m_pImageBuffer.size() != uPixelCount)
    {
        // resize image buffer
        m_pImageBuffer.reserve(uPixelCount);
        m_pImageBuffer.resize(uPixelCount, 0);
    }

    // copy frame to local buffer
    memcpy_s(&m_pImageBuffer[0], m_pImageBuffer.size(), pPixels, uPixelCount);

    // ... process image etc. ...
}

This seems fine to me, and I like that fact that I don't have to worry about the memory management, but it raises some questions:

  1. Is this a valid application of std::vector or is there a more suitable container?
  2. Am I doing the right thing performance-wise by calling reserve and resize?
  3. Will it always be the case that the underlying memory is consecutive so I can use memcpy_s as shown?

Any additional comment, criticism or advice would be very welcome.

8
  • 4
    Can I suggest using std::vector::assign instead of memcpy and (sometimes) resize? That will resize if necessary, and avoid the unnecessary initialization of the buffer.
    – mfontanini
    Oct 23, 2013 at 13:18
  • @mfontanini so I can assign a block of memory to the vector? Can you show me the syntax? Oct 23, 2013 at 13:22
  • 2
    I suggest using m_pImageBuffer.data() to access the raw memory block instead of the address of first element Oct 23, 2013 at 14:56
  • 1
    @PeteBecker, but what about supporting the code over the long run? Using memcpy_s will ensure that you don't change things around and introduce a buffer overrun later on, long after you've forgotten what was going through your head when you originally wrote it. Oct 23, 2013 at 18:53
  • 2
    @CharlieKilian - note also that the code that calls memcpy_s does not check the return value; if you forget why you resized the buffer and end up trying to write more data than the buffer will hold this call won't overwrite the buffer; all it will do is quietly produce corrupt data. There's no magic formula for writing robust code. You have to think. Oct 23, 2013 at 20:28

8 Answers 8

44
  1. Sure, this'll work fine. The one thing you need to worry about is ensuring that the buffer is correctly aligned, if your class relies on a particular alignment; in this case you may want to use a vector of the datatype itself (like float).
  2. No, reserve is not necessary here; resize will automatically grow the capacity as necessary, in exactly the same way.
  3. Before C++03, technically not (but in practice yes). Since C++03, yes.

Incidentally, though, memcpy_s isn't the idiomatic approach here. Use std::copy instead. Keep in mind that a pointer is an iterator.

Starting in C++17, std::byte is the idiomatic unit of opaquely typed storage such as you are using here. char will still work, of course, but allows unsafe usages (as char!) which byte does not.

9
  • 2
    Only by providing a particular allocator. If you want to align for __m128, just make a vector of __m128s.
    – Sneftel
    Oct 23, 2013 at 13:15
  • 1
    @JohnDibling -- data is being copied into the elements by memcpy_s; this will be effectless at best or crashy at worst if the vector is not first resized.
    – Sneftel
    Oct 23, 2013 at 13:16
  • 1
    @Ben: I was just pointing out that resize and reserve do not do the same thing. The end result here might be the same, but that is not because the functions do the same thing. Oct 23, 2013 at 13:30
  • 2
    3. is wrong, this was guaranteed in C++03 as well. You’re thinking of std::string. Oct 23, 2013 at 21:53
  • 2
    That's what I said -- guaranteed since C++03, but not before C++03. In contrast, it was not guaranteed in C++98.
    – Sneftel
    Oct 23, 2013 at 21:57
26

Besides what other answers mention, I would recommend you to use std::vector::assign rather than std::vector::resize and memcpy:

void MyClass::OnImageReceived(unsigned char *pPixels, unsigned int uPixelCount)
{
    m_pImageBuffer.assign(pPixels, pPixels + uPixelCount);
}

That will resize if necessary, and you would be avoiding the unnecessary 0 initialization of the buffer caused by std::vector::resize.

13
  • 2
    @Slava how does it not cause 0 initialization? If the new size is greater than the old one, the new elements will be value initialized(0 initialized for usnigned chars) or initialized with the second argument in OP's case.
    – mfontanini
    Oct 23, 2013 at 13:42
  • 1
    This is the best solution so far. Oct 23, 2013 at 13:43
  • 1
    @RogerRowland: I would consider this to be cannonical as well. In fact it's probably better than my copy method. Oct 23, 2013 at 13:46
  • 3
    @JohnDibling but if you're using RandomAccessIterators, then you can now how many elements there are in constant time. I mean, it might not be explicitly stated by the standard, but I'd be suprised if compilers didn't take this optimization into account.
    – mfontanini
    Oct 23, 2013 at 14:04
  • 3
    Of course, you can force the optimization by using reserve and then assign.
    – Ben Voigt
    Oct 31, 2013 at 17:13
16

Using a vector in this case is fine. In C++ the storage is guaranteed to be contigious.

I would not both resize and reserve, nor would I memcpy to copy the data in. Instead, all you need to do is reserve to make sure you don't have to reallocate many times, then clear out the vector using clear. If you resize, it will go through and set the values of every element to their defaults -- this is unnecesarry here because you're just going to overwrite it anyway.

When you're ready to copy the data in, don't use memcpy. Use copy in conjunction with back_inserter into an empty vector:

std::copy (pPixels, pPixels + uPixelCount, std::back_inserter(m_pImageBuffer));

I would consider this idiom to be much closer to canonical than the memcpy method you are employing. There might be faster or more efficient methods, but unless you can prove that this is a bottleneck in your code (which it likely won't be; you'll have much bigger fish to fry elsewhere) I would stick with idiomatic methods and leave the premature micro-optimizations to someone else.

11
  • Ahhh, good stuff, this is very helpful - thanks! I'm using VS2012 which is about as near to C++11 as MS will get, so I can assume contiguous data (fingers crossed). Is std::copy more efficient than memcpy_s or just better code (or both)? Oct 23, 2013 at 13:18
  • 1
    The use of back_inserter here is likely to make things less efficient than memcpy_s, because it works by repeatedly incrementing the same member, rather than setting the size all at once. In contrast, resizing a large vector, initializing it with all zeros, is generally implemented with special assembly routines and is very very fast. The same is true of copying a large amount of POD data from one array to another with a single (non-back-inserter-taking) std::copy.
    – Sneftel
    Oct 23, 2013 at 13:19
  • 1
    @RogerRowland: I would say better code because it is idiomatic. As Ben argues, memcpy might indeed be faster -- then again, might not -- however unless you have demonstrated through profiling that using back_inserter is an actual bottleneck in your code, I would categorize using memcpy as a premature micro-optimization. Simply because it diverges from the idiom, and ventures close to platform-specific behavior where there be dragons. Oct 23, 2013 at 13:28
  • 2
    Agreed. The brain-time you're spending on this could be better put to use refactoring that O(n^2) code block over there into O(n log n).
    – Sneftel
    Oct 23, 2013 at 13:31
  • 1
    You can avoid the reallocation but not the test to see if it's needed before each insert, which then most likely means the fast memmove turns into a slow character by character copy.
    – jcoder
    Oct 23, 2013 at 14:21
9

I would avoid std::vector as a container for storing an unstructured buffer, as std::vector is profoundly slow when used as a buffer

Consider this (C++14) example (for C++11, you can used shared instead of unique ptrs, but you'll notice slight performance hit in the array example that you don't get from the vectors when running at -O3 or -O2):

#include <chrono>
#include <ctime>
#include <iostream>
#include <memory>
#include <vector>

namespace {
std::unique_ptr<unsigned char[]> allocateWithPtr() {
  return std::unique_ptr<unsigned char[]>(new unsigned char[4000000]);
}

std::vector<unsigned char> allocateWithVector() {
  return std::vector<unsigned char>(4000000);
}
} // namespace

int main() {
  auto start = std::chrono::system_clock::now();

  for (long i = 0; i < 1000; i++) {
    auto myBuff = allocateWithPtr();
  }
  auto ptr_end = std::chrono::system_clock::now();

  for (long i = 0; i < 1000; i++) {
    auto myBuff = allocateWithVector();
  }
  auto vector_end = std::chrono::system_clock::now();

  std::cout << "std::unique_ptr = " << (ptr_end - start).count() / 1000.0
            << " ms." << std::endl;
  std::cout << "std::vector = " << (vector_end - ptr_end).count() / 1000.0
            << " ms." << std::endl;
}

Output:

bash % clang++ -O3 -std=gnu++14 test.cpp && ./a.out
std::unique_ptr = 0 ms.
std::vector = 0 ms

bash % clang++ -O2 -std=gnu++14 test.cpp && ./a.out
std::unique_ptr = 0 ms.
std::vector = 0 ms.

bash % clang++ -O1 -std=gnu++14 test.cpp && ./a.out
std::unique_ptr = 89.945 ms.
std::vector = 14135.3 ms.

bash % clang++ -O0 -std=gnu++14 test.cpp && ./a.out
std::unique_ptr = 80.945 ms.
std::vector = 67521.1 ms.

(Updated for Windows Visual Studio)
Debug:
std::unique_ptr = 8245 ms
std::vector = 9131.98 ms

Release:
std::unique_ptr = 79.46 ms
std::vector = 6830 ms

Even with no writes or reallocations, std::vector is over 800 times slower than just using a new with a unique_ptr at -O0 and 150 times slower at -O1. What's going on here?

As @MartinSchlott points out, it is not designed for this task. A vector is for holding a set object instances, not an unstructured (from an array standpoint) buffer. Objects have destructors and constructors. When the vector is destroyed, it calls the destructor for each element in it, even vector will call a destructor for each char in your vector, and it initializes the members on construction.

You can see how much time it takes just to "destroy" the unsigned chars in this vector with this example:

#include <chrono>
#include <ctime>
#include <iostream>
#include <memory>
#include <vector>

namespace {
std::vector<unsigned char> allocateWithVector() {
    return std::vector<unsigned char>(4000000); }
}

int main() {
    auto start = std::chrono::system_clock::now();

    for (long i = 0; i < 100; i++) {
        auto leakThis = new std::vector<unsigned char>(allocateWithVector());
    }
    auto leak_end = std::chrono::system_clock::now();

    for (long i = 0; i < 100; i++) {
        auto myBuff = allocateWithVector();
    }
    auto vector_end = std::chrono::system_clock::now();

    std::cout << "leaking vectors: = "
              << (leak_end - start).count() / 1000.0 << " ms." << std::endl;
    std::cout << "destroying vectors = "
              << (vector_end - leak_end).count() / 1000.0 << " ms." << std::endl;
}

Output:

leaking vectors: = 2058.2 ms.
destroying vectors = 3473.72 ms.

real    0m5.579s
user    0m5.427s
sys 0m0.135s

Even when removing the destruction of the vector, it's still taking 2 seconds to just construct 100 of these things.

If you're using the buffer to just hold incoming data and you don't require initialization, it's a waste of time to be initializing the bytes to 0 when you construct the vector. The Windows numbers above are so dramatic in Release mode because the OS is not even allocating memory yet. If you memset the data after the new, you'll see it's the same speed for constructing as the vector, but each additional memset adds 2 ms per buffer. So you're spending about 5 ms on the malloc, and 2 on the init. You can create a custom allocator for your vector to avoid initializing your bytes, but at this point, why are you working so hard to make a square peg fit in a round hole.

If you don't need dynamic resizing, or construction & destruction of the elements making up your buffer, don't use std::vector.

10
  • 1
    This is a meaningful answer, while the accepted one is not. For basic types using vector as a buffer can a lot of useless overhead for construction and destruction.
    – Pari
    Jul 10, 2019 at 14:07
  • Going forward, this is definitely the wrong answer. You should never explicitly use the new operator. If you do, you are probably misusing modern c++.If you're seeing vector allocation as slow, you're probably doing unnecessary initialization, like calling resize prior to assignment. If you're seeing any deallocation as slow, you probably are doing unnecessary destructor calls or have a debugger attached. modernescpp.com/index.php/no-new-new Jun 14, 2020 at 18:16
  • @RussellTrahan: In my example above, I'm not running in the debugger, and Im not using any unnecessary destructors - it's std::vector that's performing the unnecessary destructors. The distinction between using new inside a std::unique_ptr and using make_unique is a semantic one, but I've updated my example to be in line with more modern C++ conventions. As my example shows, compiling at the highest optimization levels eliminates the penalty for the bad choice of vector, but that is not always an option for production code, and greatly harms development work. Jun 15, 2020 at 16:06
  • Aren't you ignoring the fact that std::vector is initializing all of those unsigned chars to 0, but std::array isn't? Jun 16, 2020 at 4:40
  • 1
    @RussellTrahan: Is there another way to construct the vector to bypass the initialization? The original question is about using vectors as buffers to store image data, so presumably you don't want the initialization (and destruction) that vectors are giving you in the default/naive cases. The whole point of using an array (or new type[n] in the old days) is that it is NOT initializing memory for you. Jun 20, 2020 at 19:13
3

std::vector was MADE to be used in such cases. So, yes.

  1. Yes, it is.

  2. reserve is unnecessary in your case.

  3. Yes, it will.

2

In addition - to ensure a minimum of allocated memory:

void MyClass::OnImageReceived(unsigned char *pPixels, unsigned int uPixelCount)
{
    m_pImageBuffer.swap(std::vector<unsigned char>(
         pPixels, pPixels + uPixelCount));
    // ... process image etc. ...
}

vector::assign does not change the amount of memory allocated, if the capacity is bigger than the amount needed:

Effects: erase(begin(), end()); insert(begin(), first, last);

2
  • Swapping is relevant only if the input buffer may be altered! ... and it doesn't seem the OP can assume as much.
    – einpoklum
    Oct 23, 2013 at 18:26
  • @einpoklum. this swap do not affect the imput buffer.
    – qPCR4vir
    Oct 30, 2013 at 11:03
2

It depends. If you access the data only through iterators and the [] operator, than its okay to use a vector.

If you have to give a pointer to functions which expect a buffer of e.g. bytes. It is not in my opinion. In this case You should use something like

unique_ptr<unsigned char[]> buf(new unsigned char[size])

is it as save as a vector, but instead of a vector you have maximum control of the buffer. A vector may reallocate a buffer or during a method/function call you may unintentionally make a copy of your whole vector. A easily made mistake.

The rule (for me) is. If you have a vector, use it like a vector. If you need a memory buffer, use a memory buffer.

As in a comment pointed out, the vector has a data method. This is C++. The freedom of using a vector as a raw buffer does not mend that you should use it as a raw buffer. In my humble opinion, the intention of a vector was to have a type save buffer with type save access system. For compatibility you can use the internal buffer for calls. The intention was not to use the vector as a smart pointer buffer container. For that, I use the pointer templates, signaling other user of my code that I use this buffer in a raw way. If I use vectors, I use them in the way they are intended to, not the possible ways they offer.

AS I got some blame here for my opinion (not recommendation) I want to add some words to the actual problem the op described.

If he expect always the same picture size, he should, in my opinion, use a unique_ptr, because that's what he is doing with it in my opinion. Using

 m_pImageBuffer.resize(uPixelCount, 0);

zeros the buffer first before he copy the pPixel to it, a unnecessary time penalty.

If the pictures he is expecting of different size, he should, in my opinion, not use a vector during following reason. Especially in his code:

// called when a new image is available
if (m_pImageBuffer.size() != uPixelCount)
{
    // resize image buffer
    m_pImageBuffer.reserve(uPixelCount);
    m_pImageBuffer.resize(uPixelCount, 0);
}

he will resize the vector, which is in fact a malloc and copy as long as the images are getting bigger. A realloc in my experience always leads to malloc and copy.

That is the reason I, especially in this situation, recommand the use of a unique_ptr instead of a vector.

11
  • Yes, I do need to pass a pointer to the buffer as bytes - why is this a problem? Are you saying that the memory may not be consecutive or is it just that I need to make sure I don't resize while something else is using it? (I can understand that one). Oct 23, 2013 at 13:12
  • what's the problem? for these cases they've added data() method to vector.
    – zoska
    Oct 23, 2013 at 13:16
  • The memory will not resize and it will be consecutive. But in your example you only need the raw buffer. a unique_ptr will be better if you work in a team, where teammates expect that a vector is used as vector, not as raw buffer. For the compiler or your program, it makes no difference. Oct 23, 2013 at 13:26
  • 1
    @MartinSchlott you do know, at least you able to know when std::vector will grow. It does not happen suddenly and it is documented. Yes if you prefer you do not have to use std::vector as a buffer, but you should not recommend that to others. That's my opinion.
    – Slava
    Oct 23, 2013 at 14:07
  • 1
    I added a better explanation for the reason I recommend (in my opinion) pointer_ptr. As he ask for advise so I thought it was legit to point out my view of things. Oct 23, 2013 at 17:07
2

Please, consider this:

void MyClass::OnImageReceived(unsigned char *pPixels, unsigned int uPixelCount)
{
    // called when a new image is available
    if (m_pImageBuffer.size() != uPixelCount) // maybe just <  ??
    {
        std::vector<unsigned char> temp;
        temp.reserve(uPixelCount);        // no initialize
        m_pImageBuffer.swap(temp) ;       // no copy old data
    }

    m_pImageBuffer.assign(pPixels, pPixels + uPixelCount);  // no reallocate

    // ... process image etc. ...
}

My point is that if you have a big picture and need a litter bigger pic, your old pic will get copy during the reserve and/or resize into the new allocated memmory, the excess of memmory initialized, and then rewrited with the new pic. You colud directly assing, but then you will no be able to use the info you have about the new size to avoid posible reallocations (maybe the implementation of assign is allready optimize for this simple case ????).

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.