From 604df63f6c70b9692b067777ddb38d946ac0b2fc Mon Sep 17 00:00:00 2001 From: gzhao408 Date: Mon, 10 Aug 2020 11:28:28 -0700 Subject: [PATCH 1/2] [bench] add streams findbyte --- src/Makefile.bench.include | 1 + src/bench/streams_findbyte.cpp | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 src/bench/streams_findbyte.cpp diff --git a/src/Makefile.bench.include b/src/Makefile.bench.include index c230728a1c17b..c8e510b4829f7 100644 --- a/src/Makefile.bench.include +++ b/src/Makefile.bench.include @@ -47,6 +47,7 @@ bench_bench_bitcoin_SOURCES = \ bench/rollingbloom.cpp \ bench/rpc_blockchain.cpp \ bench/rpc_mempool.cpp \ + bench/streams_findbyte.cpp \ bench/strencodings.cpp \ bench/util_time.cpp \ bench/verify_script.cpp diff --git a/src/bench/streams_findbyte.cpp b/src/bench/streams_findbyte.cpp new file mode 100644 index 0000000000000..70722ee4d8f0d --- /dev/null +++ b/src/bench/streams_findbyte.cpp @@ -0,0 +1,31 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include + +static void FindByte(benchmark::Bench& bench) +{ + // Setup + FILE* file = fsbridge::fopen("streams_tmp", "w+b"); + const size_t file_size = 200; + uint8_t data[file_size] = {0}; + data[file_size-1] = 1; + fwrite(&data, sizeof(uint8_t), file_size, file); + rewind(file); + CBufferedFile bf(file, /*nBufSize=*/file_size + 1, /*nRewindIn=*/file_size, 0, 0); + + bench.run([&] { + bf.SetPos(0); + bf.FindByte(1); + }); + + // Cleanup + bf.fclose(); + fs::remove("streams_tmp"); +} + +BENCHMARK(FindByte, benchmark::PriorityLevel::HIGH); From 72efc26439da9a1344a19569fb0cab01f82ae7d1 Mon Sep 17 00:00:00 2001 From: Larry Ruane Date: Mon, 10 Aug 2020 15:23:03 -0600 Subject: [PATCH 2/2] util: improve streams.h:FindByte() performance Avoid use of the expensive mod operator (%) when calculating the buffer offset. No functional difference. Co-authored-by: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> --- src/bench/streams_findbyte.cpp | 2 +- src/streams.h | 20 +++++++++++++++----- src/test/fuzz/buffered_file.cpp | 2 +- src/test/streams_tests.cpp | 2 +- src/validation.cpp | 2 +- 5 files changed, 19 insertions(+), 9 deletions(-) diff --git a/src/bench/streams_findbyte.cpp b/src/bench/streams_findbyte.cpp index 70722ee4d8f0d..77f5940926e67 100644 --- a/src/bench/streams_findbyte.cpp +++ b/src/bench/streams_findbyte.cpp @@ -20,7 +20,7 @@ static void FindByte(benchmark::Bench& bench) bench.run([&] { bf.SetPos(0); - bf.FindByte(1); + bf.FindByte(std::byte(1)); }); // Cleanup diff --git a/src/streams.h b/src/streams.h index 878834380923f..e346aa0a3ff62 100644 --- a/src/streams.h +++ b/src/streams.h @@ -756,15 +756,25 @@ class CBufferedFile } //! search for a given byte in the stream, and remain positioned on it - void FindByte(uint8_t ch) + void FindByte(std::byte byte) { + // For best performance, avoid mod operation within the loop. + size_t buf_offset{size_t(m_read_pos % uint64_t(vchBuf.size()))}; while (true) { - if (m_read_pos == nSrcPos) + if (m_read_pos == nSrcPos) { + // No more bytes available; read from the file into the buffer, + // setting nSrcPos to one beyond the end of the new data. + // Throws exception if end-of-file reached. Fill(); - if (vchBuf[m_read_pos % vchBuf.size()] == std::byte{ch}) { - break; } - m_read_pos++; + const size_t len{std::min(vchBuf.size() - buf_offset, nSrcPos - m_read_pos)}; + const auto it_start{vchBuf.begin() + buf_offset}; + const auto it_find{std::find(it_start, it_start + len, byte)}; + const size_t inc{size_t(std::distance(it_start, it_find))}; + m_read_pos += inc; + if (inc < len) break; + buf_offset += inc; + if (buf_offset >= vchBuf.size()) buf_offset = 0; } } }; diff --git a/src/test/fuzz/buffered_file.cpp b/src/test/fuzz/buffered_file.cpp index 67cac8fa4eb4b..2f7ce60c7f045 100644 --- a/src/test/fuzz/buffered_file.cpp +++ b/src/test/fuzz/buffered_file.cpp @@ -53,7 +53,7 @@ FUZZ_TARGET(buffered_file) return; } try { - opt_buffered_file->FindByte(fuzzed_data_provider.ConsumeIntegral()); + opt_buffered_file->FindByte(std::byte(fuzzed_data_provider.ConsumeIntegral())); } catch (const std::ios_base::failure&) { } }, diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index a0392570f19fc..55e4f200b1c85 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -463,7 +463,7 @@ BOOST_AUTO_TEST_CASE(streams_buffered_file_rand) size_t find = currentPos + InsecureRandRange(8); if (find >= fileSize) find = fileSize - 1; - bf.FindByte(uint8_t(find)); + bf.FindByte(std::byte(find)); // The value at each offset is the offset. BOOST_CHECK_EQUAL(bf.GetPos(), find); currentPos = find; diff --git a/src/validation.cpp b/src/validation.cpp index 0b468549020dc..1873883c7848c 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -4565,7 +4565,7 @@ void Chainstate::LoadExternalBlockFile( try { // locate a header unsigned char buf[CMessageHeader::MESSAGE_START_SIZE]; - blkdat.FindByte(params.MessageStart()[0]); + blkdat.FindByte(std::byte(params.MessageStart()[0])); nRewind = blkdat.GetPos() + 1; blkdat >> buf; if (memcmp(buf, params.MessageStart(), CMessageHeader::MESSAGE_START_SIZE)) {