How to allocate memory for std::vector and then call constructor for some elements later?

Viewed 283

I'm "modernizing" a (pretty old) C++ project, and stumbling on this part:

The old code allocates memory for a dynamic array, then call constructor later for element if needed. I guess calling constructor for all elements is expensive, so the author chose to do this way (performance is critical for this project). The old code looks like this (simplified):

struct my_struct {
    my_struct(int x, int y, int z) { /* expensive ctor */ }
};

struct other_class {
    my_struct* arr;
    other_class(int n) {        
        arr = (my_struct*) malloc(n * sizeof(arr[0]);
    }

    void foo(int idx, int a, int b, int c) {
        new (&arr[idx]) my_struct(a, b, c);
    }
};

I changed arr to std::vector<my_struct>, and use std::reserve to "reserve" the memory. The code works okay, pass all current tests, but I know it's not okay, since std::reserve doesn't increase the size of that vector, so calling arr.size() will still return 0. Here's my code:

struct other_class {
    std::vector<my_struct> arr;
    other_class(int n) {        
        arr.reserve(n);
    }

    void foo(int idx, int a, int b, int c) {
        new (&arr[idx]) my_struct(a, b, c);
    }
};

How to make this code fast and safe (assuming that I can't add a default ctor to my_struct)? Thank you.


Edit: here's the sample code. It is compiled and run as expected, without any warnings: http://cpp.sh/8ytwf

2 Answers

but I know it's not okay, since std::reserve doesn't increase the size of that vector

std::reserve does increase the vectors capacity and when you only want to allocate memory and only later push elements, then that is exactly what you want.

The size of the underlying array that the vector manages is not equal to the vectors size(). size() returns the number of elements in the container and as long as there aren't any size() == 0.

When you later push elements you need not use placement new, but you should use push_back or emplace_back. More specifially, this is wrong:

 new (&arr[idx]) my_struct(a, b, c);

Because you are accessing the vector out-of-bounds (remember: size is number of elements, capacity can be bigger but you cannot access indices bigger than the vectors size). Instead:

 arr.emplace_back(a,b,c);

Although I like the emplace_back solution better, you could use std::optional in this case. This would allow you to use the idx to determine which element gets populated with a valid object.

struct other_class 
{
    std::vector<std::optional<my_struct>> arr;
    other_class( int n ) : arr( n, std::nullopt )
    { }

    void foo( int idx, int a, int b, int c ) 
    {
        arr.at( idx ).emplace( a, b, c );
    }
};
Related