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 Vector3d
s store their components in that chunk of memory. In addition, Vector3d
s 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?
1 Answer 1
When it comes to multidimensional 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<double, 3> 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.
-
\$\begingroup\$ Thanks for your suggestion. Regarding your first solution, you suggest that the
Field
class holds astd::vector<Vector3d>
, which, due to the fact that theVector3d
only holdsdouble 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– Marcel2015年03月12日 15:23:39 +00:00Commented Mar 12, 2015 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– Felix Bytow2015年03月12日 21:25:25 +00:00Commented Mar 12, 2015 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 tocomponents_
). I know thatnew 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 theField
class cares for memory allocation and deallocation. TheField
class needs to storestd::vector<Vector3d *>
because I don't want theVector3d
's copy constructor to be invoked (which would allocate memory a second time). \$\endgroup\$Marcel– Marcel2015年03月13日 07:18:23 +00:00Commented Mar 13, 2015 at 7:18 -
\$\begingroup\$ In the initializer list of the
Field
constructor, the private constructor ofVector3d
is called with a0
pointer (only used by theField
class!). In the loop, the pointer is assigned a new location. Further, I don't see any issues usingoperator()
instead ofoperator[]
, I think this is just personal taste. The same forconst std::size_t
, instead ofstd::size_t
- it doesn't harm. I was just following the rule "putconst
wherever you can". I didn't implement theconst
version of the access operators because I didn't want to make the examples longer than necessary. \$\endgroup\$Marcel– Marcel2015年03月13日 07:19:43 +00:00Commented Mar 13, 2015 at 7:19 -
\$\begingroup\$ Regarding your solution, you essentially use a
typedef
tostd::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– Marcel2015年03月13日 07:20:04 +00:00Commented Mar 13, 2015 at 7:20
std::vector<std::vector<double>> data
which is basically what you have. If you wanted flatten it out you could usefor( auto& e : data) flat_vector.insert(flat_vector.end(), e.begin(), e.end())
\$\endgroup\$vector3d_view
andvector3d
, where a_view
is non-owning? As a bonus, avector3d
could store adouble data[3];
, and astd::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 astd::vector<vector3d>
to a raw array ofdouble
is not permitted by the standard, it may work on your particular platform ifvector3d
contains nothing but thedouble data[3];
(is standard layout) \$\endgroup\$std::vector<std::vector<double>>
, I thought illustrating my alternative with a piece of code would help people to understand my question better. \$\endgroup\$Vector3d_view
andVector3d
differently which is not what I want. Inheriting from a common base class also seems to be a bit overkill. Your suggestion for castingstd::vector<Vector3d>
to a raw array ofdouble
is not an option because I'm planning to add some functionality to theVector3d
class which would destroy contiguous memory layout. \$\endgroup\$