Introduce a cache to improve performance of construction of SDFormat by arjo129 · Pull Request #1610 · gazebosim/sdformat

arjo129 · 2025-12-29T01:03:21Z

🦟 Bug fix

Summary

This commit should reduce load times significantly as it caches the first parse of the spec so that it does not need to be reparsed during construction through static initiallization of a cache.

On my m4 macbook air this reduces the following benchmark:

#include <sdf/Model.hh>
#include <sdf/Root.hh>

void deserialize()
{
    std::string sdf = "<?xml version=\"1.0\" ?><sdf version='1.6'></sdf>";
    sdf::Root root;
    sdf::Errors errors = root.LoadSdfString(sdf);
    if (!root.Model())
    {
        return;
    }
    sdf::Model _model = *root.Model();
    return;
}

int main(int argc, char **argv)
{
    std::chrono::steady_clock::time_point begin = std::chrono::steady_clock::now();
    for (int i = 0; i < 100; i++)
    deserialize();
    std::chrono::steady_clock::time_point end = std::chrono::steady_clock::now();

    std::cout << "Whole function call took: " <<
        std::chrono::duration_cast<std::chrono::microseconds>(end - begin).count()
        << "[µs]" << std::endl;
}

from running for 300000µs to 3000µs.

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.)

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.

This commit should reduce load times significantly as it caches the first parse of the spec so that it does not need to be reparsed at library load time through static initiallization of a pass. On my m4 macbook air this reduces the following benchmark: void deserialize() { std::string sdf = "<?xml version=\"1.0\" ?><sdf version='1.6'></sdf>"; sdf::Root root; sdf::Errors errors = root.LoadSdfString(sdf); if (!root.Model()) { return; } sdf::Model _model = *root.Model(); return; } int main(int argc, char **argv) { std::chrono::steady_clock::time_point begin = std::chrono::steady_clock::now(); for (int i = 0; i < 100; i++) deserialize(); std::chrono::steady_clock::time_point end = std::chrono::steady_clock::now(); std::cout << "Whole function call took: " << std::chrono::duration_cast<std::chrono::microseconds>(end - begin).count() << "[µs]" << std::endl; } from running for 300000µs to 3000µs. Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>

iche033

nice, this should replace gazebosim/gz-sim#3082

- Add copy constructor unit tests - Add doxygen comments Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>

arjo129 · 2025-12-30T06:03:11Z

Before I merge this, I think its worth pointing out that I think this breaks ABI (thanks to adding a copy constructor), in which case I should re-target this to main and we release these improvements in Kura. I am not 100% sure. According to KDE's guidelines adding new constructors is non-ABI breaking, however imo adding a copy constructor after-the fact may break ABI as the compilers might assume default copy for older versions. I think if someone can enlighten me as to whether my suspicion is right or wrong, I will retarget this PR accordingly.

azeey · 2025-12-30T18:28:42Z

That's a great speed up!! Thanks for looking into it. We already do something like this in parser.cc for handling include tags :

sdformat/src/parser.cc

Lines 1741 to 1751 in 9c1bd31

    
           // NOTE: sdf::init is an expensive call. For performance reason, 
        
           // a new sdf pointer is created here by cloning a fresh sdf template 
        
           // pointer instead of calling init every iteration. 
        
           // SDFPtr includeSDF(new SDF); 
        
           // init(includeSDF, _config); 
        
           static SDFPtr includeSDFTemplate; 
        
           if (!includeSDFTemplate) 
        
           { 
        
             includeSDFTemplate.reset(new SDF); 
        
             init(includeSDFTemplate, _config); 
        
           }

So the approach is sound.

Before I merge this, I think its worth pointing out that I think this breaks ABI (thanks to adding a copy constructor), in which case I should re-target this to main and we release these improvements in Kura. I am not 100% sure. According to KDE's guidelines adding new constructors is non-ABI breaking, however imo adding a copy constructor after-the fact may break ABI as the compilers might assume default copy for older versions. I think if someone can enlighten me as to whether my suspicion is right or wrong, I will retarget this PR accordingly.

I don't think it breaks ABI because the compiler generates the copy constructor inside the shared library. From the binary symbol perspective, defining a new copy constructor here just means we're changing the behavior of the existing copy constructor. You can see the generated code on godbolt by commenting out the custom copy constructor: https://godbolt.org/z/4cjddMeTd

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>

arjo129 · 2025-12-31T04:38:19Z

Thanks for the clarification!

I've resorted to using gz::utils::NeverDestroy.

Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org> Signed-off-by: Arjo Chakravarty <arjo129@gmail.com>

azeey · 2026-01-02T16:24:57Z

@Mergifyio backport main sdf15 sdf14 sdf12

mergify · 2026-01-02T16:25:12Z

backport main sdf15 sdf14 sdf12

✅ Backports have been created

Details

#1612 Introduce a cache to improve performance of construction of SDFormat (backport #1610) has been created for branch main
#1613 Introduce a cache to improve performance of construction of SDFormat (backport #1610) has been created for branch sdf15
#1614 Introduce a cache to improve performance of construction of SDFormat (backport #1610) has been created for branch sdf14
#1615 Introduce a cache to improve performance of construction of SDFormat (backport #1610) has been created for branch sdf12 but encountered conflicts

…1610) Reduces load times significantly as it caches the first parse of the spec so that it does not need to be reparsed during construction through static initiallization of a cache. --------- Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org> Signed-off-by: Arjo Chakravarty <arjo129@gmail.com> Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org> (cherry picked from commit 403d96a)

…1610) Reduces load times significantly as it caches the first parse of the spec so that it does not need to be reparsed during construction through static initiallization of a cache. --------- Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org> Signed-off-by: Arjo Chakravarty <arjo129@gmail.com> Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org> (cherry picked from commit 403d96a) # Conflicts: # src/SDF_TEST.cc

…1610) Reduces load times significantly as it caches the first parse of the spec so that it does not need to be reparsed during construction through static initiallization of a cache. --------- Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org> Signed-off-by: Arjo Chakravarty <arjo129@gmail.com> Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org> (cherry picked from commit 403d96a)

…1610) Reduces load times significantly as it caches the first parse of the spec so that it does not need to be reparsed during construction through static initiallization of a cache. --------- Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org> Signed-off-by: Arjo Chakravarty <arjo129@gmail.com> Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org> (cherry picked from commit 403d96a) # Conflicts: # src/SDF_TEST.cc

arjo129 requested a review from iche033 December 29, 2025 01:03

arjo129 requested review from azeey and scpeters as code owners December 29, 2025 01:03

github-actions bot added the 🪵 jetty Gazebo Jetty label Dec 29, 2025

azeey added this to Core development Dec 29, 2025

github-project-automation bot moved this to Inbox in Core development Dec 29, 2025

arjo129 force-pushed the arjo/fix/cache_perf branch from e3328a2 to 69c70c1 Compare December 29, 2025 01:06

arjo129 force-pushed the arjo/fix/cache_perf branch from 69c70c1 to 60107a5 Compare December 29, 2025 01:09

Merge branch 'sdf16' into arjo/fix/cache_perf

e7cd29b

iche033 reviewed Dec 30, 2025

View reviewed changes

Comment thread include/sdf/SDFImpl.hh

Comment thread src/SDFImplPrivate.hh

Address PR Feedback

2d195bb

- Add copy constructor unit tests - Add doxygen comments Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>

arjo129 changed the base branch from sdf16 to main December 30, 2025 05:45

arjo129 changed the base branch from main to sdf16 December 30, 2025 05:45

azeey requested changes Dec 30, 2025

View reviewed changes

Comment thread src/Root.cc Outdated

github-project-automation bot moved this from Inbox to In review in Core development Dec 30, 2025

Use gz::util::NeverDestroy

f37c13b

Signed-off-by: Arjo Chakravarty <arjo@openrobotics.org>

azeey reviewed Dec 31, 2025

View reviewed changes

Comment thread src/Root.cc Outdated

Comment thread src/Root.cc

arjo129 and others added 2 commits December 31, 2025 16:33

Update src/Root.cc

25b7dab

Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org> Signed-off-by: Arjo Chakravarty <arjo129@gmail.com>

Remove global initialization.

8f73b91

Co-authored-by: Addisu Z. Taddese <addisu@openrobotics.org> Signed-off-by: Arjo Chakravarty <arjo129@gmail.com>

azeey approved these changes Jan 2, 2026

View reviewed changes

azeey merged commit 403d96a into sdf16 Jan 2, 2026
16 checks passed

azeey deleted the arjo/fix/cache_perf branch January 2, 2026 16:24

github-project-automation bot moved this from In review to Done in Core development Jan 2, 2026

mergify bot mentioned this pull request Jan 2, 2026

Introduce a cache to improve performance of construction of SDFormat (backport #1610) #1612

Merged

9 tasks

mergify bot mentioned this pull request Jan 2, 2026

Introduce a cache to improve performance of construction of SDFormat (backport #1610) #1613

Merged

9 tasks

mergify bot mentioned this pull request Jan 2, 2026

Introduce a cache to improve performance of construction of SDFormat (backport #1610) #1614

Merged

9 tasks

mergify bot mentioned this pull request Jan 2, 2026

Introduce a cache to improve performance of construction of SDFormat (backport #1610) #1615

Merged

9 tasks

arjo129 mentioned this pull request Jan 7, 2026

Improve Model Sdf deserialization performance gazebosim/gz-sim#3082

Closed

9 tasks

iche033 mentioned this pull request Feb 18, 2026

Skip Model SDF serialization gazebosim/gz-sim#3070

Closed

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce a cache to improve performance of construction of SDFormat#1610

Introduce a cache to improve performance of construction of SDFormat#1610
azeey merged 6 commits intosdf16from
arjo/fix/cache_perf

arjo129 commented Dec 29, 2025 •

edited

Loading

Uh oh!

iche033 left a comment

Uh oh!

Uh oh!

Uh oh!

arjo129 commented Dec 30, 2025

Uh oh!

Uh oh!

azeey commented Dec 30, 2025

Uh oh!

arjo129 commented Dec 31, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

azeey commented Jan 2, 2026

Uh oh!

mergify bot commented Jan 2, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

arjo129 commented Dec 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦟 Bug fix

Summary

Checklist

Uh oh!

iche033 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

arjo129 commented Dec 30, 2025

Uh oh!

Uh oh!

azeey commented Dec 30, 2025

Uh oh!

arjo129 commented Dec 31, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

azeey commented Jan 2, 2026

Uh oh!

mergify bot commented Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Backports have been created

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arjo129 commented Dec 29, 2025 •

edited

Loading

mergify bot commented Jan 2, 2026 •

edited

Loading