I am working on a legacy application with a class named Point3D
as shown below...
template <class T>
class Point3D final
{
T values[3];
public:
Point3D();
Point3D(T x, T y, T z);
explicit Point3D(const T value);
Point3D(const Point3D& point);
explicit Point3D(const Point2D<T>& point);
~Point3D();
};
This class is getting used in many places as vector<Point3D<double>>
. The size of the vector is > 10^5
.
Also, the legacy code passes this vector as a value and returns by value. This is making the application very slow.
Also in many places, we have similar codes as shown below...
for(auto i: n){ // This loop runs for 10^4 times
Point3D<double> vpos;
vpos[X_COORDINATE] = /*Some calculation*/;
vpos[Y_COORDINATE] = /*Some calculation*/;
vpos[Z_COORDINATE] = /*Some calculation*/;
Positions.push_back(vpos);
}
To improve the performance, I plan to modify the Point3D
class to use move semantics
as shown below...
template <class T>
class Point3D final {
T* values;
public:
Point3D() {
values = new T[3];
}
Point3D(T x, T y, T z) {
values = new T[3];
}
explicit Point3D(const T value) {
values = new T[3];
}
Point3D(const Point3D& point) {
values = new T[3];
}
~Point3D() {
delete[] values;
}
T operator [] (qint64 coordinate) const { return values[coordinate]; }
T& operator [] (qint64 coordinate) { return values[coordinate]; }
Point3D& operator = (const Point3D& point) {
...
return *this;
}
Point3D(Point3D&& other) noexcept : values(other.values)
{
other.values = nullptr;
}
Point3D& operator=(Point3D&& other) noexcept
{
using std::swap;
swap(*this, other);
return *this;
}
};
I am new to move semantics
, please let me know any other ways to improve the performance.
Thanks
Also, the legacy code passes this vector as a value and returns by value. This is making the application very slow.
Your suggested modifications will make this worse in general. The cost of copying or moving your original Point3D
is very small if T
is a simple type like double
. It will just copy three double
values. If T
is a complex type, then as the other answer suggests, removing the declared destructor and copy constructor will cause move semantics to be implemented correctly automatically. (This is also known as the "rule-of-zero", letting all special member functions be implemented implicitly.)
If your problem are many copies of a vector of the type, move semantics on the type will however not help you at all. Copying the vector will still require copying all elements. Moving will not be sufficient.
You need to adjust your code at the use site of these vectors. For example, if you are passing a vector like this by-value:
void f(vector<Point3D<double>>);
void g() {
vector<Point3D<double>> v;
// fill v
f(v);
}
Then this will make an unnecessary copy of the vector. It is unnecessary because v
's data is not required anymore after the call to f
in g
. So it can be moved instead of copied into the function:
void g() {
vector<Point3D<double>> v;
// fill v
f(std::move(v));
}
This will move the vector, not the Point3D<double>
's. Moving a vector is a constant time operation, no matter whether or not move semantics are implemented for the element type. It won't copy or move any element.
If however v
's state is used in g
after the call, then you can't use move semantics directly, because move semantics will cause the state to be lost. In that case you need to rethink the design of the function and consider e.g. passing by const
-reference instead.
(Note however that you should not use std::move
if you return the vector. E.g. return std::move(v);
should be return v;
instead. Move is implicit on return statements that just return a local variable directly by name and doing the implicit std::move
call will inhibit copy elision which in certain situations will eliminate copy/moves completely.)