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

Improve load times by skipping serialization of entities when unecessary. (backport #2596) #2684

Merged
merged 2 commits into from
Nov 20, 2024

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Nov 19, 2024

🦟 Bug fix

Fixes #

Summary

I was investigating
gazebosim/gazebo_test_cases#1576 , in my investigation it came to my notice that sdf::Root takes a long time to be constructed. While I am submitting changes upstream to reduce the impact of the creation of sdf::Root, in the event that we don't serialize a model, I'm proposing we just send an empty string accross. This should minimize both network traffic and make load times more manageable.

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

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

🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸🔸


This is an automatic backport of pull request #2596 done by [Mergify](https://mergify.com).

…ary. (#2596)

* A hack to greatly improve load times

I was investigating
gazebosim/gazebo_test_cases#1576 , in my
investigation it came to my notice that `sdf::Element` takes forever to
destroy (We should open a ticket somewhere about this). If we are
skipping serialization we might as well not create and destroy an
SDF Element. This hack greatly speeds up the load time for gazebo.

Signed-off-by: Arjo Chakravarty <[email protected]>

* Remove magic word

Signed-off-by: Arjo Chakravarty <[email protected]>

* Send empty string instead of using sentinel value.

Signed-off-by: Arjo Chakravarty <[email protected]>

* Style

Signed-off-by: Arjo Chakravarty <[email protected]>

* fix custom sensor system example build (#2649)

Signed-off-by: Ian Chen <[email protected]>

* remove stray change

Signed-off-by: Arjo Chakravarty <[email protected]>

---------

Signed-off-by: Arjo Chakravarty <[email protected]>
Signed-off-by: Ian Chen <[email protected]>
Co-authored-by: Ian Chen <[email protected]>
(cherry picked from commit 1a88131)

# Conflicts:
#	include/gz/sim/components/Model.hh
Copy link
Contributor Author

mergify bot commented Nov 19, 2024

Cherry-pick of 1a88131 has failed:

On branch mergify/bp/ign-gazebo6/pr-2596
Your branch is up to date with 'origin/ign-gazebo6'.

You are currently cherry-picking commit 1a881310c.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   include/gz/sim/components/Model.hh

no changes added to commit (use "git add" and/or "git commit -a")

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

Signed-off-by: Arjo Chakravarty <[email protected]>
@arjo129 arjo129 removed the conflicts label Nov 19, 2024
@arjo129 arjo129 merged commit 8022579 into ign-gazebo6 Nov 20, 2024
10 checks passed
@arjo129 arjo129 deleted the mergify/bp/ign-gazebo6/pr-2596 branch November 20, 2024 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏯 fortress Ignition Fortress
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants