Skip to content

Commit 8d801e3

Browse files
l0rincryanofskytheuni
committed
optimization: bulk serialization writes in WriteBlockUndo and WriteBlock
Similarly to the serialization reads optimization, buffered writes will enable batched XOR calculations. This is especially beneficial since the current implementation requires copying the write input's `std::span` to perform obfuscation. Batching allows us to apply XOR operations on the internal buffer instead, reducing unnecessary data copying and improving performance. ------ > macOS Sequoia 15.3.1 > C++ compiler .......................... Clang 19.1.7 > cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ && cmake --build build -j$(nproc) && build/bin/bench_bitcoin -filter='WriteBlockBench' -min-time=10000 Before: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 5,149,564.31 | 194.19 | 0.8% | 10.95 | `WriteBlockBench` After: | ns/op | op/s | err% | total | benchmark |--------------------:|--------------------:|--------:|----------:|:---------- | 2,990,564.63 | 334.39 | 1.5% | 11.27 | `WriteBlockBench` ------ > Ubuntu 24.04.2 LTS > C++ compiler .......................... GNU 13.3.0 > cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release -DCMAKE_C_COMPILER=gcc -DCMAKE_CXX_COMPILER=g++ && cmake --build build -j$(nproc) && build/bin/bench_bitcoin -filter='WriteBlockBench' -min-time=20000 Before: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 5,152,973.58 | 194.06 | 2.2% | 19,350,886.41 | 8,784,539.75 | 2.203 | 3,079,335.21 | 0.4% | 23.18 | `WriteBlockBench` After: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 4,145,681.13 | 241.21 | 4.0% | 15,337,596.85 | 5,732,186.47 | 2.676 | 2,239,662.64 | 0.1% | 23.94 | `WriteBlockBench` Co-authored-by: Ryan Ofsky <ryan@ofsky.org> Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
1 parent 520965e commit 8d801e3

File tree

4 files changed

+155
-17
lines changed

4 files changed

+155
-17
lines changed

‎src/node/blockstorage.cpp

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -942,11 +942,12 @@ bool BlockManager::WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationSt
942942

943943
{
944944
// Open history file to append
945-
AutoFile fileout{OpenUndoFile(pos)};
946-
if (fileout.IsNull()) {
945+
AutoFile file{OpenUndoFile(pos)};
946+
if (file.IsNull()) {
947947
LogError("OpenUndoFile failed for %s while writing block undo", pos.ToString());
948948
return FatalError(m_opts.notifications, state, _("Failed to write undo data."));
949949
}
950+
BufferedWriter fileout{file};
950951

951952
// Write index header
952953
fileout << GetParams().MessageStart() << blockundo_size;
@@ -959,11 +960,7 @@ bool BlockManager::WriteBlockUndo(const CBlockUndo& blockundo, BlockValidationSt
959960
fileout << blockundo << hasher.GetHash();
960961
}
961962

962-
// Make sure `AutoFile` goes out of scope before we call `FlushUndoFile`
963-
if (fileout.fclose()) {
964-
LogError("WriteBlockUndo: fclose failed");
965-
return false;
966-
}
963+
fileout.flush(); // Make sure `AutoFile`/`BufferedWriter` go out of scope before we call `FlushUndoFile`
967964
}
968965

969966
// rev files are written in block height order, whereas blk files are written as blocks come in (often out of order)
@@ -1090,12 +1087,13 @@ FlatFilePos BlockManager::WriteBlock(const CBlock& block, int nHeight)
10901087
LogError("FindNextBlockPos failed for %s while writing block", pos.ToString());
10911088
return FlatFilePos();
10921089
}
1093-
AutoFile fileout{OpenBlockFile(pos, /*fReadOnly=*/false)};
1094-
if (fileout.IsNull()) {
1090+
AutoFile file{OpenBlockFile(pos, /*fReadOnly=*/false)};
1091+
if (file.IsNull()) {
10951092
LogError("OpenBlockFile failed for %s while writing block", pos.ToString());
10961093
m_opts.notifications.fatalError(_("Failed to write block."));
10971094
return FlatFilePos();
10981095
}
1096+
BufferedWriter fileout{file};
10991097

11001098
// Write index header
11011099
fileout << GetParams().MessageStart() << block_size;

‎src/streams.cpp

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,21 +87,29 @@ void AutoFile::write(std::span<const std::byte> src)
8787
}
8888
if (m_position.has_value()) *m_position += src.size();
8989
} else {
90-
if (!m_position.has_value()) throw std::ios_base::failure("AutoFile::write: position unknown");
9190
std::array<std::byte, 4096> buf;
92-
while (src.size() > 0) {
91+
while (src.size()) {
9392
auto buf_now{std::span{buf}.first(std::min<size_t>(src.size(), buf.size()))};
94-
std::copy(src.begin(), src.begin() + buf_now.size(), buf_now.begin());
95-
util::Xor(buf_now, m_xor, *m_position);
96-
if (std::fwrite(buf_now.data(), 1, buf_now.size(), m_file) != buf_now.size()) {
97-
throw std::ios_base::failure{"AutoFile::write: failed"};
98-
}
93+
std::copy_n(src.begin(), buf_now.size(), buf_now.begin());
94+
write_buffer(buf_now);
9995
src = src.subspan(buf_now.size());
100-
*m_position += buf_now.size();
10196
}
10297
}
10398
}
10499

100+
void AutoFile::write_buffer(std::span<std::byte> src)
101+
{
102+
if (!m_file) throw std::ios_base::failure("AutoFile::write_buffer: file handle is nullptr");
103+
if (m_xor.size()) {
104+
if (!m_position) throw std::ios_base::failure("AutoFile::write_buffer: obfuscation position unknown");
105+
util::Xor(src, m_xor, *m_position); // obfuscate in-place
106+
}
107+
if (std::fwrite(src.data(), 1, src.size(), m_file) != src.size()) {
108+
throw std::ios_base::failure("AutoFile::write_buffer: write failed");
109+
}
110+
if (m_position) *m_position += src.size();
111+
}
112+
105113
bool AutoFile::Commit()
106114
{
107115
return ::FileCommit(m_file);

‎src/streams.h

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,9 @@ class AutoFile
445445
/** Wrapper around TruncateFile(). */
446446
bool Truncate(unsigned size);
447447

448+
//! Write a mutable buffer more efficiently than write(), obfuscating the buffer in-place.
449+
void write_buffer(std::span<std::byte> src);
450+
448451
//
449452
// Stream subset
450453
//
@@ -658,4 +661,45 @@ class BufferedReader
658661
}
659662
};
660663

664+
/**
665+
* Wrapper that buffers writes to an underlying stream.
666+
* Requires underlying stream to support write_buffer() method
667+
* for efficient buffer flushing and obfuscation.
668+
*/
669+
template <typename S>
670+
class BufferedWriter
671+
{
672+
S& m_dst;
673+
DataBuffer m_buf;
674+
size_t m_buf_pos{0};
675+
676+
public:
677+
explicit BufferedWriter(S& stream LIFETIMEBOUND, size_t size = 1 << 16) : m_dst{stream}, m_buf(size) {}
678+
679+
~BufferedWriter() { flush(); }
680+
681+
void flush()
682+
{
683+
if (m_buf_pos) m_dst.write_buffer(std::span{m_buf}.first(m_buf_pos));
684+
m_buf_pos = 0;
685+
}
686+
687+
void write(std::span<const std::byte> src)
688+
{
689+
while (const auto available{std::min(src.size(), m_buf.size() - m_buf_pos)}) {
690+
std::copy_n(src.begin(), available, m_buf.begin() + m_buf_pos);
691+
m_buf_pos += available;
692+
if (m_buf_pos == m_buf.size()) flush();
693+
src = src.subspan(available);
694+
}
695+
}
696+
697+
template <typename T>
698+
BufferedWriter& operator<<(const T& obj)
699+
{
700+
Serialize(*this, obj);
701+
return *this;
702+
}
703+
};
704+
661705
#endif // BITCOIN_STREAMS_H

‎src/test/streams_tests.cpp

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -607,6 +607,94 @@ BOOST_AUTO_TEST_CASE(buffered_reader_matches_autofile_random_content)
607607
fs::remove(test_file.FileName(pos));
608608
}
609609

610+
BOOST_AUTO_TEST_CASE(buffered_writer_matches_autofile_random_content)
611+
{
612+
const size_t file_size{1 + m_rng.randrange<size_t>(1 << 17)};
613+
const size_t buf_size{1 + m_rng.randrange(file_size)};
614+
const FlatFilePos pos{0, 0};
615+
616+
const FlatFileSeq test_buffered{m_args.GetDataDirBase(), "buffered_write_test", node::BLOCKFILE_CHUNK_SIZE};
617+
const FlatFileSeq test_direct{m_args.GetDataDirBase(), "direct_write_test", node::BLOCKFILE_CHUNK_SIZE};
618+
const std::vector obfuscation{m_rng.randbytes<std::byte>(8)};
619+
620+
{
621+
DataBuffer test_data{m_rng.randbytes<std::byte>(file_size)};
622+
623+
AutoFile direct_file{test_direct.Open(pos, /*read_only=*/false), obfuscation};
624+
625+
AutoFile buffered_file{test_buffered.Open(pos, /*read_only=*/false), obfuscation};
626+
BufferedWriter buffered{buffered_file, buf_size};
627+
628+
for (size_t total_written{0}; total_written < file_size;) {
629+
const size_t write_size{Assert(std::min(1 + m_rng.randrange(m_rng.randbool() ? buf_size : 2 * buf_size), file_size - total_written))};
630+
631+
auto current_span = std::span{test_data}.subspan(total_written, write_size);
632+
direct_file.write(current_span);
633+
buffered.write(current_span);
634+
635+
total_written += write_size;
636+
}
637+
// Destructors of AutoFile and BufferedWriter will flush/close here
638+
}
639+
640+
// Compare the resulting files
641+
DataBuffer direct_result{file_size};
642+
{
643+
AutoFile verify_direct{test_direct.Open(pos, /*read_only=*/true), obfuscation};
644+
verify_direct.read(direct_result);
645+
646+
DataBuffer excess_byte{1};
647+
BOOST_CHECK_EXCEPTION(verify_direct.read(excess_byte), std::ios_base::failure, HasReason{"end of file"});
648+
}
649+
650+
DataBuffer buffered_result{file_size};
651+
{
652+
AutoFile verify_buffered{test_buffered.Open(pos, /*read_only=*/true), obfuscation};
653+
verify_buffered.read(buffered_result);
654+
655+
DataBuffer excess_byte{1};
656+
BOOST_CHECK_EXCEPTION(verify_buffered.read(excess_byte), std::ios_base::failure, HasReason{"end of file"});
657+
}
658+
659+
BOOST_CHECK_EQUAL_COLLECTIONS(
660+
direct_result.begin(), direct_result.end(),
661+
buffered_result.begin(), buffered_result.end()
662+
);
663+
664+
fs::remove(test_direct.FileName(pos));
665+
fs::remove(test_buffered.FileName(pos));
666+
}
667+
668+
BOOST_AUTO_TEST_CASE(buffered_writer_reader)
669+
{
670+
const uint32_t v1{m_rng.rand32()}, v2{m_rng.rand32()}, v3{m_rng.rand32()};
671+
const fs::path test_file{m_args.GetDataDirBase() / "test_buffered_write_read.bin"};
672+
673+
// Write out the values through a precisely sized BufferedWriter
674+
{
675+
AutoFile file{fsbridge::fopen(test_file, "w+b")};
676+
BufferedWriter f(file, sizeof(v1) + sizeof(v2) + sizeof(v3));
677+
f << v1 << v2;
678+
f.write(std::as_bytes(std::span{&v3, 1}));
679+
}
680+
// Read back and verify using BufferedReader
681+
{
682+
uint32_t _v1{0}, _v2{0}, _v3{0};
683+
AutoFile file{fsbridge::fopen(test_file, "rb")};
684+
BufferedReader f(std::move(file), sizeof(v1) + sizeof(v2) + sizeof(v3));
685+
f >> _v1 >> _v2;
686+
f.read(std::as_writable_bytes(std::span{&_v3, 1}));
687+
BOOST_CHECK_EQUAL(_v1, v1);
688+
BOOST_CHECK_EQUAL(_v2, v2);
689+
BOOST_CHECK_EQUAL(_v3, v3);
690+
691+
DataBuffer excess_byte{1};
692+
BOOST_CHECK_EXCEPTION(f.read(excess_byte), std::ios_base::failure, HasReason{"end of file"});
693+
}
694+
695+
fs::remove(test_file);
696+
}
697+
610698
BOOST_AUTO_TEST_CASE(streams_hashed)
611699
{
612700
DataStream stream{};

0 commit comments

Comments
 (0)