2
\$\begingroup\$

I have a class Vector3d with 3 double components and want to store several of them in a Field. For reasons of efficiency when sending a Field over a network via MPI, I would like to allocate contiguous memory in the Field class and let the Vector3ds store their components in that chunk of memory. In addition, Vector3ds should also be able to live without a Field; in this case, they have to care about their own memory allocation.

#include <cstddef>
#include <iostream>
#include <vector>

class Field;

class Vector3d
{
    friend class Field;

    public:
        Vector3d() : components_(new double[3]) {}
        ~Vector3d() { delete[] components_; }
        Vector3d(const Vector3d &v) : components_(new double[3])
        {
            components_[0] = v.components_[0]; 
            components_[1] = v.components_[1]; 
            components_[2] = v.components_[2];
        }
        double & operator()(const std::size_t index) { return components_[index]; }

    private:
        Vector3d(double *ptr) : components_(ptr) {};
        double *components_;
};

class Field
{
    public:    
        Field(std::size_t size);
        ~Field();
        Vector3d & operator()(const std::size_t index) { return *(vectors_[index]); }

    private:
        std::vector<double> components_;
        std::vector<Vector3d *> vectors_;
};

Field::Field(std::size_t size) : components_(std::vector<double>(3 * size)), vectors_(std::vector<Vector3d *>(size, new Vector3d(0)))
{
    double *ptr = &components_[0];
    for (auto it = vectors_.begin(); it != vectors_.end(); ++it)
    {
        (*it)->components_ = ptr;
        ptr += 3;
    }
} 

Field::~Field()
{
    for (auto it = vectors_.begin(); it != vectors_.end(); ++it)
        (*it)->components_ = 0;
}

int main(int argc, char *argv[])
{
    Field field(2);
    Vector3d &v = field(1);
    v(2) = 5;

    Vector3d v2 = v;
    v2(2) = 2;

    std::cout << field(1)(2) << std::endl;

    return 0;
}

I would like to hear what you think about this piece of code. Can this be considered a good solution or what would you change?

\$\endgroup\$
  • \$\begingroup\$ Lets say you have a 2d vector std::vector<std::vector<double>> data which is basically what you have. If you wanted flatten it out you could use for( auto& e : data) flat_vector.insert(flat_vector.end(), e.begin(), e.end()) \$\endgroup\$ – NathanOliver Mar 10 '15 at 19:35
  • 1
    \$\begingroup\$ Why not distinguish between vector3d_view and vector3d, where a _view is non-owning? As a bonus, a vector3d could store a double data[3];, and a std::vector<vector3d> would be contiguous memory already all by itself. Making your primitive types "not care about ownership" is a recipe for inefficiency and bugs in my experience. While casting a std::vector<vector3d> to a raw array of double is not permitted by the standard, it may work on your particular platform if vector3d contains nothing but the double data[3]; (is standard layout) \$\endgroup\$ – Yakk Mar 10 '15 at 19:50
  • \$\begingroup\$ @DrewDormann: Ok, my example might be a bit too long but I just wanted to make myself clear. To boil it down: What I need is a vector of vectors but stored in contiguous memory. Since this is not possible using std::vector<std::vector<double>>, I thought illustrating my alternative with a piece of code would help people to understand my question better. \$\endgroup\$ – Marcel Mar 11 '15 at 5:07
  • \$\begingroup\$ @NathanOliver: Thanks for that suggestion but I think your solution involves a copy operation each time I want to access the aggregate vector. Since I have to do this very often in my application, I don't think this would be the right way to go. \$\endgroup\$ – Marcel Mar 11 '15 at 5:09
  • \$\begingroup\$ @Yakk: I also came up with that solution but then I would have to treat Vector3d_view and Vector3d differently which is not what I want. Inheriting from a common base class also seems to be a bit overkill. Your suggestion for casting std::vector<Vector3d> to a raw array of double is not an option because I'm planning to add some functionality to the Vector3d class which would destroy contiguous memory layout. \$\endgroup\$ – Marcel Mar 11 '15 at 5:15
2
\$\begingroup\$

When it comes to multidimnesional vectors I usually suggest two approaches.

1. vector/array (C++11 and later)

This only works, when the inner dimensions are fixed and only the outer is runtime dynamic. In the end it comes down to something like this:

std::vector<std::array<T, N>> field;

In your case a Vector3d would either derive from std::array or contain one.

2. simple vector

This approach simply creates a one dimensional vector with X * Y size. The element (x, y) can then be addressed with vec[y * X + x]. This works also with the older C++ standard. Although I think this solution might not work in your setup.

Bonus

I just want to highlight @Yakk's comment. Your Vector3d could look like this:

class Vector3d
{
public:
    // stuff
    // ...
private:
    double components_[3];
};

This way the memory would also be contigous.

\$\endgroup\$
  • \$\begingroup\$ Thanks for your suggestion. Regarding your first solution, you suggest that the Field class holds a std::vector<Vector3d>, which, due to the fact that the Vector3d only holds double components_[3] as a sole member could be casted to a raw double array? Can this really considered to be good style? If sometime I decide to add members this solution won't work anymore... Actually, your second solution is what I had already but it would be better to access the individual entities of a field individually. What about my proposed solution? Do you see any memory issues? \$\endgroup\$ – Marcel Mar 12 '15 at 15:23
  • \$\begingroup\$ I had to say something more after inspecting your code, so I commented it on ideone. Also here is a version that I would call "clean", though that does not have to mean much. \$\endgroup\$ – Felix Bytow Mar 12 '15 at 21:25
  • \$\begingroup\$ Thanks for your comments. The idea was to create a Vector3d class which can also live on its own (and might have members in addition to components_). I know that new double[3] creates memory somewhere on the heap and that is what the default behavior should be. I don't see the memory leak (point me to it if I'm wrong!) because the Field class cares for memory allocation and deallocation. The Field class needs to store std::vector<Vector3d *> because I don't want the Vector3d's copy constructor to be invoked (which would allocate memory a second time). \$\endgroup\$ – Marcel Mar 13 '15 at 7:18
  • \$\begingroup\$ In the initializer list of the Field constructor, the private constructor of Vector3d is called with a 0 pointer (only used by the Field class!). In the loop, the pointer is assigned a new location. Further, I don't see any issues using operator() instead of operator[], I think this is just personal taste. The same for const std::size_t, instead of std::size_t - it doesn't harm. I was just following the rule "put const wherever you can". I didn't implement the const version of the access operators because I didn't want to make the examples longer than necessary. \$\endgroup\$ – Marcel Mar 13 '15 at 7:19
  • \$\begingroup\$ Regarding your solution, you essentially use a typedef to std::array<double, 3>, i.e., the a vector does not have any operators or the like. Of course, you could embed it in a class and let it have just a single member (its components) but as soon as you add just one other member, memory contiguity would be destroyed. I would be happy if you could elaborate this issue. \$\endgroup\$ – Marcel Mar 13 '15 at 7:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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