Getting seemingly useless "performance-inefficient-string-concatenation" warning with clang-tidy

Question

In my C++ app I'm concatenating a few strings using the + operator as follows in a reproducible example:

#include <string>
#include <curlpp/Easy.hpp>
#include <curlpp/Options.hpp>
#include <sstream>

std::string download_file_contents(const std::string& server_url)
{
    const curlpp::options::Url url(server_url);
    curlpp::Easy request;
    request.setOpt(url);

    std::ostringstream output_stream;
    const curlpp::options::WriteStream write_stream(&output_stream);
    request.setOpt(write_stream);
    request.perform();

    return output_stream.str();
}

const std::string server_files_url = "https://example.com";

int main()
{
    for (const std::map<std::string, std::string> files = { {"a", "b" }, { "c", "d" }};
        const auto& [key, value] : files)
    {
        try
        {
            const auto file_download_url = server_files_url + "/?" + key; // Warning occurs here at "+ key"
            const auto file_contents = download_file_contents(file_download_url);
            std::cout << key << " downloaded, value: " << value << std::endl;
        }
        catch (const std::exception& exception)
        {
            std::cerr << "Exception: " << exception.what() << std::endl;
        }
    }
}

The string concatentation code is called in a for loop. Using string.append() doesn't seem to make sense here since I need to make a new variable for e.g. the file_download_url for each iteration: Modifying the server_files_url is not semantically correct. Why do I still get the clang warning performance-inefficient-string-concatenation? I do not really have a performance problem from using the + operator twice in a single line of code per loop iteration. ~~I also believe the MSVC compiler would be smart enough to optimize the string concatentation as much as possible in release mode to avoid any potential performance hit.~~ I guess clang-tidy does not care about potential compiler optimizations being performed or not.

Any idea how to properly rewrite this code to avoid the warning?

It warns about using `operator+`. And its just a warning. `clang-tidy` cannot know what would be correct or if you could possibly rearrange the code, it just warns you — 463035818_is_not_an_ai, Jun 23 '23 at 13:51
"Using string.append() doesn't seem to make sense here" why not? It would avoid one temporary string — 463035818_is_not_an_ai, Jun 23 '23 at 13:52
I'd advise against worrying about it. One thing tools are missing is a sense of proportion. Even assuming you optimize this, you're saving something on the order of nanoseconds. But you're creating a URL to download a file--an operation where differences of nanoseconds or even microseconds are basically lost in the noise. Under the circumstances, you should be optimizing strictly for simplicity and readability; the speed of this particular code is irrelevant. — Jerry Coffin, Jun 23 '23 at 14:43
Your code is calculating `server_files_url + "/?"` (calling `operator+()`, storing the result in a temporary) on every loop iteration. You can hoist that to before the loop, so it is only done once. So the offending line might be `const auto file_download_url = server_files_url_with_question + key`. [An optimising compiler might or might not do that optimisation]. — Peter, Jun 23 '23 at 15:08

score 0 · Answer 1 · answered Jun 23 '23 at 13:51

Well, it looks like using std::ostringstream does the trick to avoid unecessary string creation:

#include <string>
#include <curlpp/Easy.hpp>
#include <curlpp/Options.hpp>
#include <sstream>

std::string download_file_contents(const std::string& server_url)
{
    const curlpp::options::Url url(server_url);
    curlpp::Easy request;
    request.setOpt(url);

    std::ostringstream output_stream;
    const curlpp::options::WriteStream write_stream(&output_stream);
    request.setOpt(write_stream);
    request.perform();

    return output_stream.str();
}

const std::string server_files_url = "https://example.com";

int main()
{
    for (const std::map<std::string, std::string> files = { {"a", "b" }, { "c", "d" }};
        const auto& [key, value] : files)
    {
        try
        {
            std::ostringstream file_download_url_stream;
            file_download_url_stream << server_files_url << "/?" << key;
            const auto file_download_url = file_download_url_stream.str();
            
            const auto file_contents = download_file_contents(file_download_url);
            std::cout << key << " downloaded, value: " << value << std::endl;
        }
        catch (const std::exception& exception)
        {
            std::cerr << "Exception: " << exception.what() << std::endl;
        }
    }
}

Getting seemingly useless "performance-inefficient-string-concatenation" warning with clang-tidy

1 Answers1