Skip to content

Conversation

iche033
Copy link
Contributor

@iche033 iche033 commented Sep 5, 2025

🦟 Bug fix

Related gazebosim/sdformat#1478

Summary

Skip model sdf serialization. The main problem is actually in the perf of deserialization (root.LoadSdfString), see gazebosim/sdformat#1478. This change returns an empty string in the Serialize function which will cause the Deserialize function to just skip trying to load the sdf string.

Testing with the Jetty demo world, the reduced load time from 55s to 42s and resuming from paused state (ie hit Play button) is now almost instantaneous.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • 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
  • Was GenAI used to generate this PR? If so, make sure to add "Generated-by" to your commits. (See this policy for more info.)

Generated-by: Remove this if GenAI was not used.

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by and Generated-by messages.

}

// Its super expensive to create an SDFElement for some reason
// https://github.com/gazebosim/sdformat/issues/1478
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove all the code in here as well since we know we won't be deserializing anything.

@arjo129
Copy link
Contributor

arjo129 commented Sep 6, 2025

I'm curious what completely removing serialization breaks. Do we have any downstream systems that use this model info?

@iche033
Copy link
Contributor Author

iche033 commented Sep 6, 2025

Currently the physics and entity_semantics systems use ModelSdf component but these systems live on the server side so do no depend on serialization / deserialization. No GUI plugins in Gazebo currently use this component. However, if users writing their own GUI plugins depend on the ModelSdf component, this will break their code. Similarly, users keeping a copy of their own ECM will also also be affected by this change.

@iche033
Copy link
Contributor Author

iche033 commented Sep 9, 2025

I have an alternative workaround in #3082. That PR does not break any existing behavior. It improves the load time but just not as much as this PR.

@iche033 iche033 force-pushed the iche033/skip_modlsdf_serialization branch from c006f60 to a53c290 Compare October 10, 2025 01:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🪵 jetty Gazebo Jetty

Projects

Status: Inbox

Development

Successfully merging this pull request may close these issues.

3 participants