CODEWITHSUNDEEP
c++performancec++11
samol
samol

Reputation: 20570

Is there a way to make this code faster?

I have a C++ struct that I need to convert to a list so that I can load into GPU

struct point_cloud_tensor
{
    std::vector<float>               timestamp;
    std::vector<std::vector<double>>  position;
    // more fields
};


point_cloud_tensor sweep_to_array(const point_sweep &point_sweep)
{
    const auto         num_points = point_sweep.points.size();
    point_cloud_tensor tensor;

    point_cloud_tensor.timestamp.reserve(num_points);
    point_cloud_tensor.point.reserve(num_points);

    for (int i = 0; i < point_sweep.points.size(); i++)
    {
        const auto point = point_sweep.points.at(i);
        tensor.timestamp.push_back(point.timestamp);

        std::vector<double> point_triple(3);
        point_triple.push_back(point.x);
        point_triple.push_back(point.y);
        point_triple.push_back(point.z);
        tensor.position.push_back(point_triple);
        // more fields
    }

    return tensor;
}

There are about 100K points in the sweep vector and this runs in about 30ms.

Is there a way to substantially reduce this?

Upvotes: 0

Views: 170

Answers (3)

Temple
Temple

Reputation: 1631

Did you thought about making step backward and creating a list when constructing points?

Upvotes: 0

vdavid
vdavid

Reputation: 2544

Do not call size() every time if it does not change

Since you already store point_sweep.points.size() into the variable num_points, you can use it in your for loop. When you iterate like that:

for (int i = 0; i < point_sweep.points.size(); i++)

Every iteration you will dereference point_sweep and dereference points to call its method size(). It should be faster to use the local variable instead:

for (int i = 0; i < num_points; i++)

Use a reference when appropriate

When you fetch your point:

const auto point = point_sweep.points.at(i);

You are calling the copy constructor for no reason. You should use a reference to it, using &: 

const auto& point = point_sweep.points.at(i);

References can be risky because every modification you perform will be applied to the original object, but since you are using a const reference, you should be safe.

Minimize the calls when pushing elements to the back of a vector

When you fill up your tensor.position vector, you may:

  1. Create the point with an intializer_list
  2. Add the item without a temporary variable, in order to be move-able

So, this code:

std::vector<double> point_triple(3);
point_triple.push_back(point.x);
point_triple.push_back(point.y);
point_triple.push_back(point.z);
tensor.position.push_back(point_triple);

Becomes:

tensor.position.push_back({point.x, point.y, point.z});

Plus it becomes easier to read, in my opinion.

Use another 3D point structure (if possible)

Also, as others have pointed out, if you can change the data structures then you may use an std::array or std::tuple or you may simply write a struct such as struct Point { double x, y, z; }. The array can be accessed almost exactly like a vector, which should make the transition a bit easier. The tuple must be accessed by std::get which needs to rewrite a bit of code. For example if you want to display the contents of the last element:

struct point_cloud_tensor
{
    std::vector<float>                            timestamp;
    std::vector<std::tuple<double,double,double>> position;
    // more fields
} tensor;
auto last_pos = tensor.position.back();
std::cout << "x=" << std::get<0>(last_pos) << ' ';
std::cout << "y=" << std::get<1>(last_pos) << ' ';
std::cout << "z=" << std::get<2>(last_pos) << std::endl;

However, with tuples you can add items with emplace_back instead of push_back, which saves you a move constructor, e.g.:

tensor.position.emplace_back(point.x, point.y, point.z);

Notice the difference in syntax. With push_back you have one parameter {point.x, point.y, point.z} but with emplace_back you have 3 parameters point.x, point.y, point.z. Basically with emplace_back you are just removing the curly braces.

Upvotes: 0

Hasan Patel
Hasan Patel

Reputation: 412

In this case, your std::vector is being used for a small sized array, for this you can replace it by std:array As mentioned, testing how fast a code can be run, is a matter of hardware so I can't be 100% sure if it is faster with this change.

Upvotes: 1

Related Questions