refactor: Enable some string performance tidy rules #36579

pull maflcko wants to merge 4 commits into bitcoin:master from maflcko:2610-refactor-string-tidy changing 21 files +63 −66

maflcko commented at 8:00 AM on October 20, 2026: member

Enabling the string performance clang-tidy rules should be fine and easy.

E.g. on aarch64, when running the BlockToJsonVerboseWrite bench, I can see a minimal 0.3% drop in instructions and 0.6% drop in branches.
refactor: Enable performance-string-view-conversions
No need to create a std::string copy
fa7973e113
refactor: Enable performance-prefer-single-char-overloads faae016796

refactor: Enable performance-faster-string-find

It is an alias of the previously enabled rule. See https://releases.llvm.org/23.1.0/tools/clang/tools/extra/docs/clang-tidy/checks/performance/faster-string-find.html

bbbb9c6d73

DrahtBot added the label Refactoring on Oct 20, 2026
DrahtBot commented at 8:00 AM on October 20, 2026: contributor


The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



External sites
- Code coverage & benchmarks: https://corecheck.dev/bitcoin/bitcoin/pulls/36579
- Ralph review <img src=https://review.fish.foo/assets/bitcoin/bitcoin/36579.png alt="Ralph review status"> https://git.fish.foo/bitcoin/bitcoin/pulls/36579#author-ralph


Reviews

See the guideline and AI policy for information on the review process.

Type Reviewers

ACK stickies-v, hebasto, l0rinc

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #36443 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36443.svg"> (descriptor: Reconstruct a multipath descriptor from its expansion by achow101)
- #36442 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36442.svg"> (descriptor: Represent multipath descriptors in a single Descriptor object by achow101)
- #36033 <img src="https://drahtbot.space/ack_count/bitcoin/bitcoin/36033.svg"> ([wip,nomerge,rfc] build: Require C++23 compiler by maflcko)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
maflcko renamed this:
~~refactor: Enable some string performance checks~~
refactor: Enable some string performance tidy rules
on Oct 20, 2026
stickies-v approved
stickies-v commented at 9:54 AM on October 20, 2026: contributor

ACK bbbb9c6d73a3750185c62bcb520b6ba74ac34be9

nit: util:subprocess.h:1091 contains a performance-prefer-single-char-overloads violation, not currently checked in CI because it's in an #ifdef WIN32 block1
refactor: Use single-char overload in execute_process WIN32 block fabb6bbe53
stickies-v commented at 10:25 AM on October 20, 2026: contributor

re-ACK fabb6bbe53e9cba85b62da2416287c41975d71dd
hebasto commented at 10:39 AM on October 20, 2026: member

ACK fabb6bbe53e9cba85b62da2416287c41975d71dd.

Type	Reviewers
ACK	stickies-v, hebasto, l0rinc

in src/rpc/util.cpp:1234 in fabb6bbe53

1230 | @@ -1231,7 +1231,7 @@ void RPCResult::CheckInnerDoc() const
1231 |  std::string RPCArg::ToStringObj(const bool oneline) const
1232 |  {
1233 |      std::string res;
1234 | -    res += "\"";
1235 | +    res += '\"';

l0rinc commented at 3:57 PM on October 20, 2026:

nit: we don't need to escape " anymore:

    res += '"';

If you decide to apply this, please consider this one, too:

diff --git a/src/rest.cpp b/src/rest.cpp
--- a/src/rest.cpp	(revision 9f63677902b51a3582d2d232e570b725ec9082ed)
+++ b/src/rest.cpp	(date 1792500052439)
@@ -179,7 +179,7 @@
     std::string formats;
     for (const auto& rf_name : rf_names) {
         if (strlen(rf_name.name) > 0) {
-            formats.append(".");
+            formats += '.';
             formats.append(rf_name.name);
             formats.append(", ");
         }

maflcko commented at 12:27 PM on October 21, 2026:

thx, I'll roll them into one of my other pulls, to avoid review churn here

l0rinc approved
l0rinc commented at 4:01 PM on October 20, 2026: contributor

ACK fabb6bbe53e9cba85b62da2416287c41975d71dd

Manually went over each one, change seems boring and trivial. I don't think of it as an optimization, but rather as reenabling suppressed tidy rules. Let non-blocking nit only, lgtm.
sedited merged this on Oct 21, 2026
sedited closed this on Oct 21, 2026
maflcko deleted the branch on Oct 21, 2026

Contributors

Labels

refactor: Enable some string performance tidy rules #36579

External sites

Reviews

Conflicts