[silverweed]

Experiment: change all viable const std::string & function parameters in the RNTuple library with std::string_view. Also contextually change a few core/io function signatures and see if anything breaks.

Rationale: using string_view consistently is more convenient and avoids making the user perform explicit conversions.

[github-actions]

Test Results

    24 files      24 suites   3d 23h 59m 18s ⏱️
 3 883 tests  3 883 ✅ 0 💤 0 ❌
83 139 runs  83 139 ✅ 0 💤 0 ❌

Results for commit 4688681.

♻️ This comment has been updated with latest results.

[pcanal]

This (of course) means that at least here the std::string_view version is never faster than before (and often slower) :(

[silverweed]

"Often" is a bit of a guess, but yes, it would only become "faster" (assuming this is even relevant inside this complex function) if we trickled down the string_view signature to methods like TCollection::FindObject.

[pcanal]

"Often" is a bit of a guess

Well the only cases where it is not slower are the case where the user is currently have to transform a std::string or std::string_view into a C string (and they would not pay the price a double allocation only if they change their code).

Now, I am not completely opposed to the change but I recommend we do not advertise is as the performance improvement (yet) [For this purpose the PR title is good but the PR description could be improved].

only become "faster" ... if we trickled down the string_view signature to methods like TCollection::FindObject.

True and more exactly if we do so for the majority of the code paths all the way down to the actual use.

[silverweed]

Perhaps the wording on the PR comment is misleading, but this was never intended as a performance optimization. The "optimality" refers to programming convenience and doesn't want to make speed claims. It could become a performance optimization in the future but it would require consistently using string_view in a lot more places in the core/meta/IO code.

[pcanal]

This is fair but the text is explicitly calling out run-time performance:

Rationale: consistent use of string_view is more optimal than const char*/const string& due to less useless copies / string size checks / type conversions.
``
(see "optimal" and "less useless copies") - both are true only when (as you pointed out) string_view is used throughout the call stack.

Consider:

Rationale: using string_view consistently is more convenient and avoids making the user perform explicit conversions.