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.

Sign up for GitHub

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

Jump to bottom

Fix performance issue with contact data and AABB updates #1048

Merged
merged 1 commit into from
Sep 24, 2021
Merged

Fix performance issue with contact data and AABB updates #1048

merged 1 commit into from
Sep 24, 2021

Conversation

azeey
Copy link
Contributor

@azeey azeey commented Sep 24, 2021

🦟 Bug fix

Fixes #997

Summary

ComponentState::OneTimeChange was used when updating the AxisAlignedBox and ContactSensorData components which causes a full state serialization (all components serialized and sent over to state subscribers). In Fortress, we have some components that are expensive to serialize/deserialize, such as components::ModelSdf. However, we thought it was okay thinking they would be sent to the client once at the start of simulation. But with ComponentState::OneTimeChange, these components are serialized/deserialized repeatedly which causes a severe performance hit. Using ComponentState::PeriodicChange instead makes it so that only the changed components are sent.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

@azeey
Fix performance issue with contact data and AABB updates
43bab7b 
`ComponentState::OneTimeChange` was used when updating the
`AxisAlignedBox` and `ContactSensorData` components which causes all
components to be serialized and sent over to state subscribers.

Signed-off-by: Addisu Z. Taddese <[email protected]>
@azeey azeey self-assigned this Sep 24, 2021
@github-actions github-actions bot added the 🔮 dome Ignition Dome label Sep 24, 2021
@azeey azeey linked an issue Sep 24, 2021 that may be closed by this pull request
While using contact sensor, the simulation stuck when contact happens #997
Closed
chapulina
chapulina approved these changes Sep 24, 2021
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

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

sweet!

@chapulina chapulina added the performance Runtime performance label Sep 24, 2021
@chapulina chapulina merged commit 3fbddd1 into gazebosim:ign-gazebo4 Sep 24, 2021
@azeey azeey deleted the fix_contact_updates branch September 24, 2021 21:15
@diegoferigo diegoferigo mentioned this pull request Sep 28, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jennuine jennuine jennuine approved these changes

@chapulina chapulina chapulina approved these changes

Assignees

@azeey azeey

Labels
🔮 dome Ignition Dome performance Runtime performance
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

While using contact sensor, the simulation stuck when contact happens
3 participants
@azeey @chapulina @jennuine