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

Consolidate RobotState benchmarks in single file #2528

Merged
merged 5 commits into from Nov 15, 2023

Conversation

marioprats
Copy link
Contributor

Description

This change consolidates robot_state_benchmark.cpp and robot_state_jacobian_benchmark.cpp into a single robot_state_benchmark.cpp file, and adds further benchmarks for RobotState construction, copy, and forward kinematics. This is in preparation for a follow-up change to update memory management in RobotState.
The matrix multiplication / inverse tests have been fully rewritten into a gmock BENCHMARK for consistency.

Output of running:

./build/moveit_core/robot_state/robot_state_benchmark 2>/dev/null
---------------------------------------------------------------------------------
Benchmark                                       Time             CPU   Iterations
---------------------------------------------------------------------------------
MultiplyAffineTimesMatrix/10000000           55.7 ms         55.7 ms           13
MultiplyMatrixTimesMatrix/10000000           57.3 ms         57.3 ms           12
MultiplyIsometryTimesIsometry/10000000       57.5 ms         57.5 ms           12
InverseIsometry3d/10000000                    107 ms          107 ms            7
InverseAffineIsometry/10000000               23.3 ms         23.3 ms           30
InverseAffine/10000000                       52.7 ms         52.7 ms           14
InverseMatrix4d/10000000                      109 ms          109 ms            6
RobotStateConstruct/100                     0.005 ms        0.005 ms       123429
RobotStateConstruct/1000                    0.054 ms        0.054 ms        12827
RobotStateConstruct/10000                   0.553 ms        0.553 ms         1250
RobotStateCopy/100                          0.006 ms        0.006 ms       114604
RobotStateCopy/1000                         0.061 ms        0.061 ms        11284
RobotStateCopy/10000                        0.615 ms        0.615 ms         1141
RobotStateUpdate/10                         0.026 ms        0.026 ms        26048
RobotStateUpdate/100                        0.269 ms        0.269 ms         2615
RobotStateUpdate/1000                        2.66 ms         2.66 ms          262
RobotStateForwardKinematics/10              0.023 ms        0.023 ms        29936
RobotStateForwardKinematics/100             0.234 ms        0.234 ms         2996
RobotStateForwardKinematics/1000             2.34 ms         2.34 ms          304
MoveItJacobian                                258 ns          258 ns      2714295
KdlJacobian                                  1106 ns         1107 ns       630134

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

Copy link

codecov bot commented Nov 13, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f79a070) 50.83% compared to head (6658e1a) 50.44%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2528      +/-   ##
==========================================
- Coverage   50.83%   50.44%   -0.39%     
==========================================
  Files         391      390       -1     
  Lines       32198    32026     -172     
==========================================
- Hits        16366    16151     -215     
- Misses      15832    15875      +43     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@sjahr sjahr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this cleanup! The unification of the files makes sense to me. Could you add documentation for this file to make maintainability easier in the future? I think ~ one brief comment for every function + benchmark and explanation for some parameters is sufficient.

Copy link
Collaborator

@sjahr sjahr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@sjahr sjahr merged commit e064a84 into moveit:main Nov 15, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants