[clang-tidy] report inefficient `std::string::append()` usage with `char` in `performance-faster-string-find`

[firewave]

I have been looking into providing helper for efficient string concatenation in danmar/cppcheck#8344.

I initially had the following for char.

#include <string>

void concat1(std::string& str, char c)
{
  str.append(1, c);
}

Turns out that is less efficient then either of the following (which behave exactly the same):

void concat2(std::string& str, char c)
{
  str += c;
}

void concat3(std::string& str, char c)
{
  str.push_back(c);
}

https://godbolt.org/z/1P4E7Wdcc

[llvmbot]

@llvm/issue-subscribers-clang-tidy

Author: Oliver Stöneberg (firewave)

Details

I have been looking into providing helper for efficient string concatenation in https://github.com/danmar/cppcheck/pull/8344.

I initially had the following for char.

#include <string>

void concat1(std::string& str, char c)
{
  str.append(1, c);
}

Turns out that is less efficient then either of the following (which behave exactly the same):

void concat2(std::string& str, char c)
{
  str += c;
}

void concat3(std::string& str, char c)
{
  str.push_back(c);
}

https://godbolt.org/z/1P4E7Wdcc

[nicovank]

that is less efficient then either of the following (which behave exactly the same):

I'm not sure godbolt is showing that, what's the rationale? This depends more on the C++ stdlib implementation than the compiler. Quick Bench shows no real significant difference: https://quick-bench.com/q/77jtfX5ErCNicQFxBWQn45qpduo.

I'm not sure avoiding append (or preferring one over another in general) is a good recommendation, on different machines or with updates to the standard library and compiler, which option is marginally faster is likely to change. The current check suggests char over string overloads for chars, which does have small benefits.

[firewave]

That is a fair assessment. I wanted to include a Quick Bench link but since it is seriously outdated that might not be representative any more. And llvm-mca was also not an option since it includes external calls. And as I usually made a big mess out of my tickets which involve performance issues I wanted to get it right at the first try. I forgot to mention that in the initial report.

In my case it was giving a 10% reduction of Ir in callgrind (using Clang and libstdc++).

On a side note I do not trust either Quick Bench or llvm-mca because I used both in the past and the work I did optimizing the isolated code did not translate when used it in the actual application. It seems Clang is smarter/stupidier when it is part of a bigger chunk of code (or the tools are flawed).

[nicovank]

I join you that Quick Bench is not updated or always reliable (never really used callgrind or llvm-mca so I'm not familiar). Running this locally it looks like there might be a small difference, but I don't have time to look into it more at the moment. If there is a significant execution time difference, maybe worth shooting it upstream to libstdc++ / libc++?

[firewave]

Somewhat related: #26925 (comment)