Skip to content

Commit fe6a299

Browse files
committed
Merge bitcoin/bitcoin#24852: util: optimize HexStr
5e61532 util: optimizes HexStr (Martin Leitner-Ankerl) 4e2b99f bench: Adds a benchmark for HexStr (Martin Leitner-Ankerl) 67c8411 test: Adds a test for HexStr that checks all 256 bytes (Martin Leitner-Ankerl) Pull request description: In my benchmark, this rewrite improves runtime 27% (g++) to 46% (clang++) for the benchmark `HexStrBench`: g++ 11.2.0 | ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 0.94 | 1,061,381,310.36 | 0.7% | 12.00 | 3.01 | 3.990 | 1.00 | 0.0% | 0.01 | `HexStrBench` master | 0.68 | 1,465,366,544.25 | 1.7% | 6.00 | 2.16 | 2.778 | 1.00 | 0.0% | 0.01 | `HexStrBench` branch clang++ 13.0.1 | ns/byte | byte/s | err% | ins/byte | cyc/byte | IPC | bra/byte | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 0.80 | 1,244,713,415.92 | 0.9% | 10.00 | 2.56 | 3.913 | 0.50 | 0.0% | 0.01 | `HexStrBench` master | 0.43 | 2,324,188,940.72 | 0.2% | 4.00 | 1.37 | 2.914 | 0.25 | 0.0% | 0.01 | `HexStrBench` branch Note that the idea for this change comes from denis2342 in #23364. This is a rewrite so no unaligned accesses occur. Also, the lookup table is now calculated at compile time, which hopefully makes the code a bit easier to review. ACKs for top commit: laanwj: Code review ACK 5e61532 aureleoules: tACK 5e61532. theStack: ACK 5e61532 🚤 Tree-SHA512: 40b53d5908332473ef24918d3a80ad1292b60566c02585fa548eb4c3189754971be5a70325f4968fce6d714df898b52d9357aba14d4753a8c70e6ffd273a2319
2 parents 33aaf43 + 5e61532 commit fe6a299

File tree

4 files changed

+64
-6
lines changed

4 files changed

+64
-6
lines changed

src/Makefile.bench.include

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ bench_bench_bitcoin_SOURCES = \
4444
bench/rollingbloom.cpp \
4545
bench/rpc_blockchain.cpp \
4646
bench/rpc_mempool.cpp \
47+
bench/strencodings.cpp \
4748
bench/util_time.cpp \
4849
bench/verify_script.cpp
4950

src/bench/strencodings.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
// Copyright (c) 2022 The Bitcoin Core developers
2+
// Distributed under the MIT software license, see the accompanying
3+
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
4+
5+
#include <bench/bench.h>
6+
#include <bench/data.h>
7+
#include <util/strencodings.h>
8+
9+
static void HexStrBench(benchmark::Bench& bench)
10+
{
11+
auto const& data = benchmark::data::block413567;
12+
bench.batch(data.size()).unit("byte").run([&] {
13+
auto hex = HexStr(data);
14+
ankerl::nanobench::doNotOptimizeAway(hex);
15+
});
16+
}
17+
18+
BENCHMARK(HexStrBench);

src/test/util_tests.cpp

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,24 @@ BOOST_AUTO_TEST_CASE(util_HexStr)
198198
BOOST_CHECK_EQUAL(HexStr(in_s), out_exp);
199199
BOOST_CHECK_EQUAL(HexStr(in_b), out_exp);
200200
}
201+
202+
{
203+
auto input = std::string();
204+
for (size_t i=0; i<256; ++i) {
205+
input.push_back(static_cast<char>(i));
206+
}
207+
208+
auto hex = HexStr(input);
209+
BOOST_TEST_REQUIRE(hex.size() == 512);
210+
static constexpr auto hexmap = std::string_view("0123456789abcdef");
211+
for (size_t i = 0; i < 256; ++i) {
212+
auto upper = hexmap.find(hex[i * 2]);
213+
auto lower = hexmap.find(hex[i * 2 + 1]);
214+
BOOST_TEST_REQUIRE(upper != std::string_view::npos);
215+
BOOST_TEST_REQUIRE(lower != std::string_view::npos);
216+
BOOST_TEST_REQUIRE(i == upper*16 + lower);
217+
}
218+
}
201219
}
202220

203221
BOOST_AUTO_TEST_CASE(span_write_bytes)

src/util/strencodings.cpp

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <tinyformat.h>
1010

1111
#include <algorithm>
12+
#include <array>
1213
#include <cstdlib>
1314
#include <cstring>
1415
#include <limits>
@@ -452,17 +453,37 @@ std::string Capitalize(std::string str)
452453
return str;
453454
}
454455

456+
namespace {
457+
458+
using ByteAsHex = std::array<char, 2>;
459+
460+
constexpr std::array<ByteAsHex, 256> CreateByteToHexMap()
461+
{
462+
constexpr char hexmap[16] = {'0', '1', '2', '3', '4', '5', '6', '7', '8', '9', 'a', 'b', 'c', 'd', 'e', 'f'};
463+
464+
std::array<ByteAsHex, 256> byte_to_hex{};
465+
for (size_t i = 0; i < byte_to_hex.size(); ++i) {
466+
byte_to_hex[i][0] = hexmap[i >> 4];
467+
byte_to_hex[i][1] = hexmap[i & 15];
468+
}
469+
return byte_to_hex;
470+
}
471+
472+
} // namespace
473+
455474
std::string HexStr(const Span<const uint8_t> s)
456475
{
457476
std::string rv(s.size() * 2, '\0');
458-
static constexpr char hexmap[16] = { '0', '1', '2', '3', '4', '5', '6', '7',
459-
'8', '9', 'a', 'b', 'c', 'd', 'e', 'f' };
460-
auto it = rv.begin();
477+
static constexpr auto byte_to_hex = CreateByteToHexMap();
478+
static_assert(sizeof(byte_to_hex) == 512);
479+
480+
char* it = rv.data();
461481
for (uint8_t v : s) {
462-
*it++ = hexmap[v >> 4];
463-
*it++ = hexmap[v & 15];
482+
std::memcpy(it, byte_to_hex[v].data(), 2);
483+
it += 2;
464484
}
465-
assert(it == rv.end());
485+
486+
assert(it == rv.data() + rv.size());
466487
return rv;
467488
}
468489

0 commit comments

Comments
 (0)