Improve performance of type name demangling

[jacobperron]

Feature description

It was noticed in the addition of #266, that there was a performance hit, possibly due to the use of the regex library. See #266 (comment) for a discussion.

A simple benchmark is timing the rcl graph tests before and after applying the change:

for i in {1..10}; do; /usr/bin/time -o time.txt -a --format "%E" build/rcl/test/test_graph__rmw_fastrtps_cpp; done
cat time.txt

On my machine,
before: 03.06 seconds
after: 04.28 seconds

Implementation considerations

We should try replacing the usage of regex and cutting down on string copies (as suggested) to see if performance can be improved.

[jacobperron]

Closed by #488

[jacobperron]

I've compared the usage of std::regex_replace with an implementation based on std::string::replace (code) and the latter is significantly faster (~23x). But, after replacing all usage of regex_replace with my custom function in rmw_fastrtps I didn't notice any performance improvement. The regex calls must not be the culprit for the performance degradation.

[clalancette]

I've compared the usage of std::regex_replace with an implementation based on std::string::replace (code) and the latter is significantly faster (~23x). But, after replacing all usage of regex_replace with my custom function in rmw_fastrtps I didn't notice any performance improvement. The regex calls must not be the culprit for the performance degradation.

Good to know. I'm still in favor of doing this, but we can defer to later (and we'll have to figure out the other performance degradation).

[jacobperron]

I've spent some time trying to cut down on string copies (e.g. using std::string::reserve, mutate operator "+=", removing temporary variables, using output parameters), but none of my attempts showed a significant improvement in performance.

In fact, when I revert the change introduced in #266 I see slightly worse performance compared to the master branch.

master:      4.2775 +- 0.006 (seconds)
revert #266: 4.2805 +- 0.005 (seconds)

I'm using the same benchmark from when this ticket was created. I'm not sure why I'm unable get the same result. Maybe it's because of the change in Fast-RTPS version since the ticket was opened? Or perhaps it was a fluke. I could probably come up with a better benchmark test.

In any case, I think it makes sense to add the "find and replace" function to rcpputils so it can be re-used by multiple RMW implementations.

[jacobperron]

Here are the related PRs that are eligible for backport into Dashing:

• Add function 'find_and_replace' rcpputils#13
• Use rcpputils::find_and_replace instead of std::regex_replace rmw_opensplice#272
• Use rpputils::find_and_replace instead of std::regex_replace rmw_connext#359
• Use rcpputils::find_and_replace instead of std::regex_replace #291

---

@clalancette I'm not convinced there is more performance improvement that can be made related to the type name demangling, so I'm closing this ticket.

[clalancette]

@clalancette I'm not convinced there is more performance improvement that can be made related to the type name demangling, so I'm closing this ticket.

That's fine. We can always revisit and improve in the future if there is a need.