-
Notifications
You must be signed in to change notification settings - Fork 302
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
Cache link poses to improve performance #669
Conversation
1857cf2
to
4614265
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A few questions for the PR reviewers (cc @azeey).
Codecov Report
@@ Coverage Diff @@
## ign-gazebo4 #669 +/- ##
===============================================
+ Coverage 64.96% 65.01% +0.04%
===============================================
Files 232 232
Lines 16640 16645 +5
===============================================
+ Hits 10811 10822 +11
+ Misses 5829 5823 -6
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice, changes look good to me. Just one question about the non-static link pose comment
Signed-off-by: Ashton Larkin <[email protected]>
e85f4d9
to
58cc8d3
Compare
Signed-off-by: Ashton Larkin <[email protected]>
Signed-off-by: Ashton Larkin [email protected]
🦟 Bug fix
Related to gazebosim/gz-physics#219.
Summary
This PR is motivated by #656 and gazebosim/gz-physics#219.
gazebosim/gz-physics#219 points out that the
UpdateSim
method in the physics system goes over all links since the physics system doesn't know which links changed. For links that belong to non-static models, we are always performing pose updates, even if the pose hasn't changed. I'm proposing that we cache each non-static link's pose from the previous iteration and only perform a pose update if the link's pose is different from the previous iteration.Testing this change by profiling a world as done in #656 (3000 non-static simple shapes, unbounded RTF, headless, and TPE for the physics engine) seems to result in a noticeable performance improvement.
Before link caching,
UpdateSim
takes around8.25ms
.Local pose
, which contains the pose updates, takes around5.2ms
, which is more than half of theUpdateSim
time:After link caching,
UpdateSim
takes around5.7ms
, andLocal pose
now takes around2.4ms
:Taking a look at the some of the other code in
UpdateSim
, I am wondering if we can do something similar for non-static link velocities and accelerations. @chapulina what do you think about this?Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge