Is it good practice to use std::vector as a simple buffer?
- 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
). - No, reserve is not necessary here; resize will automatically grow the capacity as necessary, in exactly the same way.
- 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.
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
.
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.
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 <array>
#include <chrono>
#include <ctime>
#include <iostream>
#include <memory>
#include <vector>
namespace {
std::unique_ptr<std::array<unsigned char, 4000000>> allocateWithPtr() {
return std::make_unique<std::array<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.
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.
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>
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 don't need dynamic resizing, or construction & destruction of the elements making up your buffer, don't use std::vector.
std::vector was MADE to be used in such cases. So, yes.
Yes, it is.
reserve
is unnecessary in your case.Yes, it will.