Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Some memory saving is possible when getting points from voxel hash map #349

Merged
merged 3 commits into from
Jun 26, 2024

Conversation

l00p3
Copy link
Contributor

@l00p3 l00p3 commented Jun 24, 2024

In these two methods of VoxelHasMap the memory for vectors "points" is reserved but never cleared if not completely filled. I checked and size and capacity often doesn't match, resulting in some small waste of memory.
Also, in the method PointCloud() std::for_each is probably better and emplace_back instead of push_back, just to be pedantic. The static cast for max_points_per_voxel_ from int to size_t is there just to be consistent with the method GetPoints().

@Pyrestone
Copy link

Please make sure this change doesn't negatively affect runtime performance. Constantly re-allocating that chunk of memory to different sizes might be very detrimental, and a poor exchange for some intermediate, temporary memory savings.

@l00p3
Copy link
Contributor Author

l00p3 commented Jun 24, 2024

Yes, I am currently running some experiment to check it. Thanks.

@l00p3
Copy link
Contributor Author

l00p3 commented Jun 24, 2024

Please make sure this change doesn't negatively affect runtime performance. Constantly re-allocating that chunk of memory to different sizes might be very detrimental, and a poor exchange for some intermediate, temporary memory savings.

These the results on a couple of sequences (should be enough). No change on my machine.
merged

Theoretically makes sense, shrink_to_fit() is just a request to change the capacity. The compiler optimizes if it is actually needed or not. Also, in case of capacity change the complexity is linear in the dimension of the vector because it just goes at the end of the vector and changes the "opinter" to the last "available element", no copy should happen.

@l00p3 l00p3 merged commit a6c96ed into PRBonn:main Jun 26, 2024
17 checks passed
@l00p3 l00p3 deleted the luca-pedantic-small-update branch July 3, 2024 13:38
@nachovizzo nachovizzo added the voxelization All the topic related to voxelization utilities label Aug 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core voxelization All the topic related to voxelization utilities
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants