Makogan
Makogan

Reputation: 9546

std::vector reserve being more costly than expected

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

Answers (2)

Caleth
Caleth

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

ShadowRanger
ShadowRanger

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

Related Questions