Optimize load time and memory consumption by reusing element descriptions

[azeey]

🦟 Bug fix

Toward gazebosim/jetty_demo#3

Summary

As far as I can tell, element descriptions are not mutated, so there is no reason to clone them. But I'm not 100% sure if this has unintended consequences.

I created a small test where a large sdf is loaded to show the before and after memory consumption results:
Before

$ time build/sdf_memory 3k_shapes.sdf
Successfully loaded file
8647.16MB
build/sdf_memory 3k_shapes.sdf  6.77s user 3.13s system 99% cpu 9.904 total

After

$ time build/sdf_memory 3k_shapes.sdf
Successfully loaded file
289.069MB
build/sdf_memory 3k_shapes.sdf  0.40s user 0.19s system 97% cpu 0.611 total

This is a significant speedup and reduction in memory usage, so it would be worth figuring out if we can truly do this without any negative consequences.

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.

[iche033]

I think the side-effect of this change is that if the user manually updates the element description of one ElementPtr, all other cloned ElementPtrs's descriptions are changed as well? This is a probably rare use case. We should probably still make a note in the migration guide. The memory usage reduction and speedup may be worth the change.

[azeey]

I'm thinking about deprecating the GetElementDescription API and add two functions ElementDescription and MutableElementDescription. The first will return a shared_ptr<const Element> which should be used in most places. The second would clone the description if it's the first time it's called. This would be a type of copy-on-write, so we would only clone if MutableElementDescription is called.

[scpeters]

I think the side-effect of this change is that if the user manually updates the element description of one ElementPtr, all other cloned ElementPtrs's descriptions are changed as well? This is a probably rare use case. We should probably still make a note in the migration guide. The memory usage reduction and speedup may be worth the change.

👍 to update migration guide

I'm thinking about deprecating the GetElementDescription API and add two functions ElementDescription and MutableElementDescription. The first will return a shared_ptr<const Element> which should be used in most places. The second would clone the description if it's the first time it's called. This would be a type of copy-on-write, so we would only clone if MutableElementDescription is called.

👍 to proposed APIs

[azeey]

I've added the new API and updated the migration guide. Could you take another look?

[scpeters]

changes seem reasonable to me, though I haven't tested with gz-sim. I've left some minor comments and opened azeey#2 with all the suggested changes

[azeey]

Thanks for all the suggestions @scpeters. I've merged your PR.