Thanks for the review, I deliberately avoided std::string::replace() because each call shifts the remaining suffix before the next match is searched (assuming different sizes).
The result builder (this PR) reads from the original string and constructs the output once, avoiding those repeated suffix copies.
I benchmarked both implementations (see code below, lower is better).
Grow: 2-byte search → 32-byte substitute
result builder (PR) ██▒░░░░░░░░░░░░░░░░░ 14.62 ns/replacement
in-place replace ████████████████████ 127.71 ns/replacement (+773.5%, 8.74x slower)
Same size: 2-byte search → 2-byte substitute
result builder (PR) ████████████████████ 12.08 ns/replacement
in-place replace █████████████████░░░ 10.33 ns/replacement (-14.5%)
Shrink: 2-byte search → 1-byte substitute
result builder (PR) █▓░░░░░░░░░░░░░░░░░░ 11.40 ns/replacement
in-place replace ████████████████████ 120.06 ns/replacement (+953.2%, 10.53x slower)
The result builder is basically the same speed regardless of the replacement type, while the in-place version is 8.74x slower when replacements grow and 10.53x slower when they shrink.
I would therefore prefer to keep the result builder for this general helper.
Note that the equal-size row is only a control and is not relevant in this particular case, but it's a general helper method, so it should be considered.
<details>
<summary>Benchmark code and raw results</summary>
// Copyright (c) 2026-present The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or https://opensource.org/license/mit/.
#include <bench/bench.h>
#include <cstddef>
#include <stdexcept>
#include <string>
#include <string_view>
namespace {
constexpr size_t NUM_REPLACEMENTS{8192};
constexpr std::string_view SEARCH{"%w"};
constexpr std::string_view GROWING_SUBSTITUTE{"0123456789abcdef0123456789abcdef"};
constexpr std::string_view SAME_SIZE_SUBSTITUTE{"xx"};
constexpr std::string_view SHRINKING_SUBSTITUTE{"x"};
void ReplaceAllBuilder(std::string& in_out, std::string_view search, std::string_view substitute)
{
if (search.empty()) return;
auto pos{in_out.find(search)};
if (pos == std::string::npos) return;
std::string result;
result.reserve(in_out.size());
std::string::size_type start{0};
for (; pos != std::string::npos; pos = in_out.find(search, start)) {
result.append(in_out, start, pos - start).append(substitute);
start = pos + search.size();
}
result.append(in_out, start);
in_out.swap(result);
}
void ReplaceAllInPlace(std::string& in_out, std::string_view search, std::string_view substitute)
{
if (search.empty()) return;
auto pos{in_out.find(search)};
if (pos == std::string::npos) return;
for (; pos != std::string::npos; pos = in_out.find(search, pos + substitute.size())) {
in_out.replace(pos, search.size(), substitute);
}
}
using ReplaceAllFn = void (*)(std::string&, std::string_view, std::string_view);
void BenchReplaceAll(benchmark::Bench& bench, ReplaceAllFn replace_all, std::string_view substitute)
{
std::string input;
input.reserve(NUM_REPLACEMENTS * SEARCH.size());
std::string expected;
expected.reserve(NUM_REPLACEMENTS * substitute.size());
for (size_t i{0}; i < NUM_REPLACEMENTS; ++i) {
input.append(SEARCH);
expected.append(substitute);
}
auto result{input};
replace_all(result, SEARCH, substitute);
if (result != expected) throw std::runtime_error{"Unexpected ReplaceAll result"};
bench.batch(NUM_REPLACEMENTS).unit("replacement").run([&] {
auto value{input};
replace_all(value, SEARCH, substitute);
ankerl::nanobench::doNotOptimizeAway(value);
});
}
void ReplaceAllBuilderGrow(benchmark::Bench& bench) { BenchReplaceAll(bench, ReplaceAllBuilder, GROWING_SUBSTITUTE); }
void ReplaceAllInPlaceGrow(benchmark::Bench& bench) { BenchReplaceAll(bench, ReplaceAllInPlace, GROWING_SUBSTITUTE); }
void ReplaceAllBuilderSameSize(benchmark::Bench& bench) { BenchReplaceAll(bench, ReplaceAllBuilder, SAME_SIZE_SUBSTITUTE); }
void ReplaceAllInPlaceSameSize(benchmark::Bench& bench) { BenchReplaceAll(bench, ReplaceAllInPlace, SAME_SIZE_SUBSTITUTE); }
void ReplaceAllBuilderShrink(benchmark::Bench& bench) { BenchReplaceAll(bench, ReplaceAllBuilder, SHRINKING_SUBSTITUTE); }
void ReplaceAllInPlaceShrink(benchmark::Bench& bench) { BenchReplaceAll(bench, ReplaceAllInPlace, SHRINKING_SUBSTITUTE); }
} // namespace
BENCHMARK(ReplaceAllBuilderGrow);
BENCHMARK(ReplaceAllInPlaceGrow);
BENCHMARK(ReplaceAllBuilderSameSize);
BENCHMARK(ReplaceAllInPlaceSameSize);
BENCHMARK(ReplaceAllBuilderShrink);
BENCHMARK(ReplaceAllInPlaceShrink);
AppleClang 21, arm64, Release:
$ build-bench/bin/bench_bitcoin -filter='ReplaceAll.*' -min-time=5000
| ns/replacement | replacement/s | err% | total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
| 14.62 | 68,412,155.97 | 3.5% | 5.65 | `ReplaceAllBuilderGrow`
| 12.08 | 82,773,428.57 | 1.8% | 5.57 | `ReplaceAllBuilderSameSize`
| 11.40 | 87,704,897.85 | 4.7% | 5.17 | `ReplaceAllBuilderShrink`
| 127.71 | 7,830,201.42 | 1.5% | 5.65 | `ReplaceAllInPlaceGrow`
| 10.33 | 96,762,552.16 | 1.9% | 5.37 | `ReplaceAllInPlaceSameSize`
| 120.06 | 8,329,402.21 | 1.1% | 5.55 | `ReplaceAllInPlaceShrink`
</details>
