Don't reparse parent elements when cloning.

[arjo129]

🦟 Bug fix

Fixes #

Summary

Improves performance on sdf::Element::Clone by not requiring the parent elements to be reparsed. This was originally part of #1479. But it makes sense to seperate out the work.

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.

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

[scpeters]

I think macOS build failures are due to osrf/homebrew-simulation#2834

[scpeters]

I ran the PERFORMANCE_parser_urdf test several times with std_err redirected to /dev/null and see about a 18% reduction in average test time (1 - 0.928 / 1.133) ~= 0.18

sdf15

Ran the test 10 times:

Minimum	Average	Maximum
1.110 s	1.133 s	1.149 s

$ time for i in $(seq 10); do bin/PERFORMANCE_parser_urdf 2> /dev/null ; done
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (1103 ms)
[----------] 1 test from URDFParser (1103 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1103 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (1123 ms)
[----------] 1 test from URDFParser (1123 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1123 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (1107 ms)
[----------] 1 test from URDFParser (1107 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1108 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (1149 ms)
[----------] 1 test from URDFParser (1149 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1149 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (1117 ms)
[----------] 1 test from URDFParser (1117 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1117 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (1100 ms)
[----------] 1 test from URDFParser (1100 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1100 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (1109 ms)
[----------] 1 test from URDFParser (1109 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1109 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (1112 ms)
[----------] 1 test from URDFParser (1112 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1112 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (1116 ms)
[----------] 1 test from URDFParser (1116 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1116 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (1118 ms)
[----------] 1 test from URDFParser (1118 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (1118 ms total)
[  PASSED  ] 1 test.

real	0m11.335s
user	0m8.720s
sys	0m2.549s

this branch

Ran the test 10 times:

Minimum	Average	Maximum
0.890 s	0.928 s	0.945 s

$ time for i in $(seq 10); do bin/PERFORMANCE_parser_urdf 2> /dev/null ; done
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (906 ms)
[----------] 1 test from URDFParser (906 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (906 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (894 ms)
[----------] 1 test from URDFParser (894 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (894 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (920 ms)
[----------] 1 test from URDFParser (920 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (920 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (892 ms)
[----------] 1 test from URDFParser (892 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (892 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (923 ms)
[----------] 1 test from URDFParser (923 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (923 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (921 ms)
[----------] 1 test from URDFParser (921 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (921 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (908 ms)
[----------] 1 test from URDFParser (908 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (908 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (945 ms)
[----------] 1 test from URDFParser (945 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (945 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (903 ms)
[----------] 1 test from URDFParser (903 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (903 ms total)
[  PASSED  ] 1 test.
Running main() from /Users/scpeters/ws/ionic/src/sdformat/test/gtest_vendor/src/gtest_main.cc
[==========] Running 1 test from 1 test suite.
[----------] Global test environment set-up.
[----------] 1 test from URDFParser
[ RUN      ] URDFParser.AtlasURDF_5runs_performance
[       OK ] URDFParser.AtlasURDF_5runs_performance (890 ms)
[----------] 1 test from URDFParser (890 ms total)

[----------] Global test environment tear-down
[==========] 1 test from 1 test suite ran. (890 ms total)
[  PASSED  ] 1 test.

real	0m9.280s
user	0m6.680s
sys	0m2.490s

[azeey]

This makes sense. Reparse was added in #589 to deal with the possibility that the meaning of an Element's value depends on the attributes in the element and those attributes might be different when reparenting. However, this does not apply when cloning since there will be no change in the attributes in the cloned element.