-
Notifications
You must be signed in to change notification settings - Fork 282
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 top level and static to speed up physics system #656
Conversation
Signed-off-by: Louise Poubel <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-gazebo4 #656 +/- ##
================================================
- Coverage 77.37% 64.76% -12.62%
================================================
Files 217 232 +15
Lines 12217 16601 +4384
================================================
+ Hits 9453 10751 +1298
- Misses 2764 5850 +3086
Continue to review full report at Codecov.
|
This worked great on SubT. Performance went from 65-70% to around 95% when running a single robot in a world. |
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.
Changes seem okay to me. It seems like there wasn't much of an improvement if I ran this with the 3000 shapes world with static models instead of non-static, which was a little surprising to me. But I think that this is a step in the right direction, and we will probably need to continue making changes like this.
Also, I didn't see any issues in the code, but I'm not too familiar with the physics system, so if someone else wants to take a look at the changes as a sanity check, that might be a good idea (I think @nkoenig may have already done this).
Signed-off-by: Ashton Larkin <[email protected]>
Signed-off-by: Ashton Larkin <[email protected]>
…#993) Signed-off-by: Ashton Larkin <[email protected]> Signed-off-by: Louise Poubel <[email protected]>
🦟 Bug fix
Summary
Profiling a world as follows:
-z 1000000000
)-s
and only thePhysics
system)I noticed that the
PhysicsPrivate::UpdateSim
was taking way too long for what it does. All that function is doing is getting resulting data from theign-physics
step and populating the ECM, and that takes ~70% of an iteration:Before:
I think there's a lot of room for improvement there, but here are just a couple of little improvements. Caching a couple of values that never change (top-level model and static), instead of reevaluating them at every iteration, we can reduce that to closer to 60%:
After:
For static, one thing to consider is leveraging the ECM and adding
Static
components to links that are static, so that theEach
call can iterate only over static links.Checklist
codecheck
passed (See contributing)Note to maintainers: Remember to use Squash-Merge