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

Optimize Eigen block operations #5974

Merged
merged 1 commit into from
Mar 9, 2024

Conversation

mvieth
Copy link
Member

@mvieth mvieth commented Mar 3, 2024

According to https://eigen.tuxfamily.org/dox/group__TutorialBlockOperations.html , Eigen should receive as much information as possible at compile time, to generate optimal machine code. This means specifying the block size as a template parameter (if fixed size), using topLeftCorner if block starts at (0, 0)

According to https://eigen.tuxfamily.org/dox/group__TutorialBlockOperations.html , Eigen should receive as much information as possible at compile time, to generate optimal machine code.
This means specifying the block size as a template parameter (if fixed size), using topLeftCorner if block starts at (0, 0)
@larshg
Copy link
Contributor

larshg commented Mar 8, 2024

Looks good. Did you do any benchmark to see how much it changed?

@mvieth
Copy link
Member Author

mvieth commented Mar 9, 2024

Looks good. Did you do any benchmark to see how much it changed?

Not really. I don't expect any orders-of-magnitude speedup to be honest, but the changes were quite easy and straightforward, so even a chance of a speedup seems worth it 😄 And this also sets a better example for future code

@mvieth mvieth merged commit eebcfcb into PointCloudLibrary:master Mar 9, 2024
13 checks passed
@mvieth mvieth deleted the eigen_block_operations branch March 9, 2024 11:10
@mvieth mvieth added changelog: enhancement Meta-information for changelog generation module: registration labels Apr 5, 2024
mvieth added a commit to mvieth/pcl that referenced this pull request Apr 21, 2024
Partially revert PointCloudLibrary#5974 and PointCloudLibrary#5988 and PointCloudLibrary#5907
After the 1.14.1 release, we can re-apply these changes.
The ABI checker reported:
`The parameter file_name became passed in r12 register instead of r13. Applications will read the wrong memory block instead of the parameter value.` and `The parameter cloud_tgt_demean became passed in r12 register instead of r13.`
These changes are not super important, so I think it doesn't hurt to temporarily revert them to achieve 100% ABI compatibility between 1.14.0 and 1.14.1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog: enhancement Meta-information for changelog generation module: registration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants