-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
PCL RFC 0002: Better type for indices
Kunal Tyagi edited this page Jan 20, 2020
·
8 revisions
- Title: PCL-RFC-0002: Better type for indices
- Author: kunaltyagi
- Gitter room: PointCloudLibrary/PCL-RFC-02
- Past discussion: #3351
- People of Interest: jasjuang, aPonza, taketwo, SergioRAgostinho
Currently, there's mixed use of int
, long
, unsigned int
, unsigned long
, unsigned long long
and std::size_t
for indices. Some effort has been made to unify towards std::size_t
to reduce the limit of 2 billions points due to int
.
- Follows a best practice
- Allows operations on actually limitless points (if using 64 bit systems)
- One of the following complication if using
GSL
- Extra build-time dependency on internet
- Copy-pasted source file
-
git
sub-tree/sub-module
- A new typedef
using pcl::index_t = std::ptrdiff_t;
- Not platform independent
- Wastes memory: Largest RAM supported is ~32TB (2^48). That's a clear 25% loss of efficiency for extreme cases, and larger for average use-case.
- None for existing internal migration
- Major API breakage for changing
Indices
fromstd::vector<int>
tostd::vector<index_t>
Minor to medium, split over multiple phases:
- Internal API modification is underway, would need modification from
std::size_t
topcl::index_t
- Modification of public API will be faster since it is easily identifiable (function declaration, data members)
Multi-step migration:
- Replace
std::vector<int>
withIndicesVec
- Replace
std::size_t
withpcl::index_t
- Replace
IndicesVec = std::vector<int>
withIndicesVec = std::vector<pcl::index_t>
Is there anyone who has been adversely impacted with size increase and cache misses due to std::size_t
instead of int
? If so, there might be merit in discussing alternatives to std::ptrdiff_t
(such as std::int32_t
instead, which is the same but platform independent and brings back performance)