Reputation: 9546
So I need to manually handle the memory allocated by an std::vector for efficiency purposes. And I noticed that my program was slower than expected, so I added this idiom everywhere in my code base:
const uint new_edges_capacity = mesh.edges.size() + 6;
if(new_edges_capacity > mesh.edges.capacity())
mesh.edges.reserve(new_edges_capacity * 2);
Originally I was simply doing:
const uint new_edges_capacity = mesh.edges.size() + 6;
mesh.edges.reserve(new_edges_capacity * 2);
Well, that first snippet is orders of magnitude faster than the second.
I don't understand, the official documentation seems to suggest that reserve should already be doing the same check I am. However, perf
definitely marks std::reserve as the most expensive operation in my code, and indeed the modification shows that not calling reserve and relying on it to check for allocation is faster.
An example of a function where I am using this pattern:
template<typename V>
void HMesh<V>::SplitFace(uint face_id, HMesh<V>& mesh)
{
mesh.vertex_data.push_back({});
mesh.verts.push_back({&mesh});
HMesh<V>::MVert& c = mesh.verts.back();
const uint new_edges_capacity = mesh.edges.size() + 6;
if(new_edges_capacity > mesh.edges.capacity())
mesh.edges.reserve(new_edges_capacity * 2);
mesh.edges.push_back({&mesh});
HMesh<V>::MEdge& n00 = mesh.edges.back();
mesh.edges.push_back({&mesh});
HMesh<V>::MEdge& n01 = mesh.edges.back();
mesh.edges.push_back({&mesh});
HMesh<V>::MEdge& n10 = mesh.edges.back();
mesh.edges.push_back({&mesh});
HMesh<V>::MEdge& n11 = mesh.edges.back();
mesh.edges.push_back({&mesh});
HMesh<V>::MEdge& n20 = mesh.edges.back();
mesh.edges.push_back({&mesh});
HMesh<V>::MEdge& n21 = mesh.edges.back();
const uint new_faces_capacity = mesh.faces.size() + 2;
if(new_faces_capacity > mesh.faces.capacity())
mesh.faces.reserve(new_faces_capacity * 2);
mesh.faces.push_back({&mesh});
HMesh<V>::MFace& f1 = mesh.faces.back();
mesh.faces.push_back({&mesh});
HMesh<V>::MFace& f2 = mesh.faces.back();
auto& face = mesh.faces[face_id];
HMesh<V>::MFace& f0 = face;
HMesh<V>::MEdge& e0 = face.EdgeD();
HMesh<V>::MEdge& e1 = face.EdgeD().NextD();
HMesh<V>::MEdge& e2 = face.EdgeD().PrevD();
HMesh<V>::MVert& v0 = e0.VertD();
HMesh<V>::MVert& v1 = e1.VertD();
HMesh<V>::MVert& v2 = e2.VertD();
ConnectFace(f0, n00, e0, n10);
ConnectFace(f1, n11, e1, n21);
ConnectFace(f2, n20, e2, n01);
Pair(n00, n01);
Pair(n10, n11);
Pair(n20, n21);
AttachVertices(n00, c, v0);
AttachVertices(n11, c, v1);
AttachVertices(n20, c, v2);
c.Data({
(v0.Data().position + v1.Data().position + v2.Data().position) / 3.0,
(v0.Data().uv + v1.Data().uv + v2.Data().uv) / 3.0,
{0,0,1}
});
}
Upvotes: 1
Views: 279
Reputation: 62704
You don't need to reserve at all. Just use indices.
template<typename V>
void HMesh<V>::SplitFace(uint face_id, HMesh<V>& mesh)
{
mesh.vertex_data.push_back({});
mesh.verts.push_back({&mesh});
HMesh<V>::MVert& c = mesh.verts.back();
const uint n = mesh.edges.size();
mesh.edges.push_back({&mesh});
mesh.edges.push_back({&mesh});
mesh.edges.push_back({&mesh});
mesh.edges.push_back({&mesh});
mesh.edges.push_back({&mesh});
mesh.edges.push_back({&mesh});
auto & n00 = mesh.edges[n + 0];
auto & n01 = mesh.edges[n + 1];
auto & n10 = mesh.edges[n + 2];
auto & n11 = mesh.edges[n + 3];
auto & n20 = mesh.edges[n + 4];
auto & n21 = mesh.edges[n + 5];
const uint f = mesh.faces.size();
mesh.faces.push_back({&mesh});
mesh.faces.push_back({&mesh});
auto& f0 = mesh.faces[face_id];
auto& f1 = mesh.faces[f + 0];
auto& f2 = mesh.faces[f + 1];
auto& e0 = f0.EdgeD();
auto& e1 = f0.EdgeD().NextD();
auto& e2 = f0.EdgeD().PrevD();
auto& v0 = e0.VertD();
auto& v1 = e1.VertD();
auto& v2 = e2.VertD();
ConnectFace(f0, n00, e0, n10);
ConnectFace(f1, n11, e1, n21);
ConnectFace(f2, n20, e2, n01);
Pair(n00, n01);
Pair(n10, n11);
Pair(n20, n21);
AttachVertices(n00, c, v0);
AttachVertices(n11, c, v1);
AttachVertices(n20, c, v2);
c.Data({
(v0.Data().position + v1.Data().position + v2.Data().position) / 3.0,
(v0.Data().uv + v1.Data().uv + v2.Data().uv) / 3.0,
{0,0,1}
});
}
Upvotes: 1
Reputation: 155418
You're not reimplementing reserve
, you're reimplementing the resize operation when capacity is exhausted. Problem is, if you do this every time you insert an item, you're resizing every time, making every operation O(n)
(as it has to move all the items from the original backing storage to the new, larger, storage). If you ran this every time, building from an empty vector
, you'd see a pattern of:
Size Capacity Reallocation needed?
1 14 Yes
2 16 Yes (16 > 14)
3 18 Yes (18 > 16)
4 20 Yes (20 > 18)
... reallocations continue forever ...
and each of those resizes would cause a new allocation, a move of all existing elements, and clean up of the old allocation (with destructors of the moved-from objects firing). You haven't saved anything, because you force new allocations every time, in an effort to avoid unnecessary allocations (which is clearly unhelpful).
In the case with the if
test, since you test against a value half of what you reserve
if the test fails, it reallocates much less frequently:
Size Capacity Reallocation needed?
1 14 Yes
2 14 No (2 + 6 <= 14)
3 14 No (3 + 6 <= 14)
4 14 No (4 + 6 <= 14)
5 14 No (5 + 6 <= 14)
6 14 No (6 + 6 <= 14)
7 14 No (7 + 6 <= 14)
8 14 No (8 + 6 <= 14)
9 30 Yes (9 + 6 > 14)
... very rare reallocations ...
As you can see, your test means far less resizing activity.
If you're just trying to guess at how much space you need, don't; let vector
resize automatically on demand (doubling every time it's necessary might save you a little time over the default amortized growth schedule, but it also wastes a lot of memory). reserve
is for when you know how much space you need; using it over and over is expensive.
Upvotes: 4