From 5a1c2bd341c9586b090b6b40c20edb011cb616eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C5=91rinc?= Date: Sun, 26 Jan 2025 19:44:03 +0100 Subject: [PATCH] optimization: Bulk serialization writes in `WriteBlockUndo` and `WriteBlock` Added `AutoFile::write_large` for batching obfuscation operations, so instead of copying the data and doing the xor in a 4096 byte array, we're doing it directly on the input. `DataStream` constructor was also added to enable presized serialization and writing in a single command. Similarly to the serialization reads, buffered writes will enable batched xor calculations - especially since currently we need to copy the write inputs Span to do the obfuscation on it, batching enables doing the xor on the internal buffer instead. > cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release && cmake --build build -j$(nproc) && build/bin/bench_bitcoin -filter='WriteBlockBench' -min-time=10000 > C++ compiler .......................... AppleClang 16.0.0.16000026 Before: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 5,182,762.89 | 192.95 | 0.5% | 11.04 | `WriteBlockBench` After: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 1,791,444.08 | 558.21 | 0.9% | 11.08 | `WriteBlockBench` > C++ compiler .......................... GNU 13.3.0 Before: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 4,128,530.90 | 242.22 | 3.8% | 19,358,001.33 | 8,601,983.31 | 2.250 | 3,079,334.76 | 0.4% | 10.64 | `WriteBlockBench` After: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 3,130,556.05 | 319.43 | 4.7% | 17,305,378.56 | 6,457,946.37 | 2.680 | 2,579,854.87 | 0.3% | 10.83 | `WriteBlockBench` Co-authored-by: Cory Fields Co-authored-by: Ryan Ofsky --- src/node/blockstorage.cpp | 8 ++++---- src/streams.cpp | 10 ++++++++++ src/streams.h | 28 ++++++++++++++++++++++++++++ src/test/streams_tests.cpp | 7 ++++--- 4 files changed, 46 insertions(+), 7 deletions(-) diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 414af2e4ac8..a061dbea11a 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -959,14 +959,14 @@ bool BlockManager::WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationSt } // Write index header - fileout << GetParams().MessageStart() << blockundo_size; + BufferedFileW(fileout, HEADER_BYTE_SIZE) << GetParams().MessageStart() << blockundo_size; pos.nPos += HEADER_BYTE_SIZE; { // Calculate checksum HashWriter hasher{}; hasher << block.pprev->GetBlockHash() << blockundo; // Write undo data & checksum - fileout << blockundo << hasher.GetHash(); + BufferedFileW(fileout, blockundo_size + sizeof(uint256)) << blockundo << hasher.GetHash(); } // rev files are written in block height order, whereas blk files are written as blocks come in (often out of order) @@ -1117,10 +1117,10 @@ FlatFilePos BlockManager::WriteBlock(const CBlock& block, int nHeight) } // Write index header - fileout << GetParams().MessageStart() << block_size; + BufferedFileW(fileout, HEADER_BYTE_SIZE) << GetParams().MessageStart() << block_size; pos.nPos += HEADER_BYTE_SIZE; // Write block - fileout << TX_WITH_WITNESS(block); + BufferedFileW(fileout, block_size) << TX_WITH_WITNESS(block); return pos; } diff --git a/src/streams.cpp b/src/streams.cpp index cd496ee2be4..3fc58e260c9 100644 --- a/src/streams.cpp +++ b/src/streams.cpp @@ -102,6 +102,16 @@ void AutoFile::write(Span src) } } +void AutoFile::write_large(Span src) +{ + if (!m_file) throw std::ios_base::failure("AutoFile::write_large: file handle is nullptr"); + util::Xor(src, m_xor, *m_position); // obfuscate in-place + if (std::fwrite(src.data(), 1, src.size(), m_file) != src.size()) { + throw std::ios_base::failure("AutoFile::write_large: write failed"); + } + if (m_position) *m_position += src.size(); +} + bool AutoFile::Commit() { return ::FileCommit(m_file); diff --git a/src/streams.h b/src/streams.h index 6d84ea76f30..9dbbb2ad0c9 100644 --- a/src/streams.h +++ b/src/streams.h @@ -163,6 +163,7 @@ public: typedef vector_type::reverse_iterator reverse_iterator; explicit DataStream() = default; + explicit DataStream(size_type n) { reserve(n); } explicit DataStream(Span sp) : DataStream{AsBytes(sp)} {} explicit DataStream(Span sp) : vch(sp.data(), sp.data() + sp.size()) {} @@ -452,6 +453,7 @@ public: void read(Span dst); void ignore(size_t nSize); void write(Span src); + void write_large(Span src); // Note that src will be mutated template AutoFile& operator<<(const T& obj) @@ -468,6 +470,32 @@ public: } }; +class BufferedFileW +{ + AutoFile& m_file; + uint32_t m_buffer_size; + DataStream m_buf; + +public: + explicit BufferedFileW(AutoFile& file, const uint32_t buffer_size) + : m_file(file), m_buffer_size{buffer_size}, m_buf{buffer_size} {} + + ~BufferedFileW() + { + Assert(m_buf.size() <= m_buffer_size); + m_file.write_large(m_buf); + } + + void write(Span src) { m_buf.write(src); } + + template + BufferedFileW& operator<<(const T& obj) + { + Serialize(m_buf, obj); + return *this; + } +}; + class BufferedFileR { DataStream m_buf; diff --git a/src/test/streams_tests.cpp b/src/test/streams_tests.cpp index 7b5bea86060..c72f77325f7 100644 --- a/src/test/streams_tests.cpp +++ b/src/test/streams_tests.cpp @@ -572,11 +572,12 @@ BOOST_AUTO_TEST_CASE(streams_datastream_write_large) const uint32_t v1{m_rng.rand32()}, v2{m_rng.rand32()}, v3{m_rng.rand32()}; const fs::path tmp_path{m_args.GetDataDirBase() / "test_datastream_write_large.bin"}; - // Write out the values to file + // Write out the values through a precisely sized BufferedFileW { AutoFile file{fsbridge::fopen(tmp_path, "w+b")}; - file << v1 << v2; - file.write(AsBytes(Span{&v3, 1})); + BufferedFileW f(file, sizeof(v1) + sizeof(v2) + sizeof(v3)); + f << v1 << v2; + f.write(AsBytes(Span{&v3, 1})); } // Read back and verify using BufferedFileR {