From 39902e7d86f9e1b9142ab0afd4b60b1fa153fd91 Mon Sep 17 00:00:00 2001 From: Remy Boutonnet Date: Wed, 11 Nov 2026 10:53:37 +0100 Subject: [PATCH 1/4] IO: keep an aligned buffer's capacity when it moves --- io/fs/AlignedBuffer.h | 6 ++++++ test/io/AlignedBufferMoveTest.cpp | 35 +++++++++++++++++++++++++++++++ test/io/CMakeLists.txt | 1 + 3 files changed, 42 insertions(+) create mode 100644 test/io/AlignedBufferMoveTest.cpp diff --git a/io/fs/AlignedBuffer.h b/io/fs/AlignedBuffer.h index add90c434f..79d4bff5d0 100644 --- a/io/fs/AlignedBuffer.h +++ b/io/fs/AlignedBuffer.h @@ -44,8 +44,10 @@ class AlignedBuffer { _buffer = other._buffer; _size = other._size; + _capacity = other._capacity; other._buffer = nullptr; other._size = 0; + other._capacity = 0; } AlignedBuffer& operator=(AlignedBuffer&& other) noexcept { @@ -53,10 +55,14 @@ class AlignedBuffer { return *this; } + free(_buffer); + _buffer = other._buffer; _size = other._size; + _capacity = other._capacity; other._buffer = nullptr; other._size = 0; + other._capacity = 0; return *this; } diff --git a/test/io/AlignedBufferMoveTest.cpp b/test/io/AlignedBufferMoveTest.cpp new file mode 100644 index 0000000000..05c32d152e --- /dev/null +++ b/test/io/AlignedBufferMoveTest.cpp @@ -0,0 +1,35 @@ +#include + +#include "TuringTest.h" + +#include "AlignedBuffer.h" + +using namespace turing::test; + +namespace { + +constexpr size_t CAPACITY = 16384; + +} + +class AlignedBufferMoveTest : public TuringTest { +}; + +TEST_F(AlignedBufferMoveTest, MoveConstructionKeepsTheCapacity) { + fs::AlignedBuffer buffer(CAPACITY); + const fs::AlignedBuffer moved(std::move(buffer)); + + EXPECT_EQ(moved.capacity(), CAPACITY); +} + +TEST_F(AlignedBufferMoveTest, MoveAssignmentKeepsTheCapacity) { + fs::AlignedBuffer buffer(CAPACITY); + fs::AlignedBuffer assigned; + assigned = std::move(buffer); + + EXPECT_EQ(assigned.capacity(), CAPACITY); +} + +int main(int argc, char** argv) { + return turing::test::turingTestMain(argc, argv); +} diff --git a/test/io/CMakeLists.txt b/test/io/CMakeLists.txt index 9864f740ad..0e0b7830c7 100644 --- a/test/io/CMakeLists.txt +++ b/test/io/CMakeLists.txt @@ -1,4 +1,5 @@ add_turing_test(test_io_path PathTest.cpp) +add_turing_test(test_io_aligned_buffer_move AlignedBufferMoveTest.cpp) add_turing_test(test_io_bytebufferiterator ByteBufferIteratorTest.cpp) add_turing_test(test_io_unaligned_scalar_read UnalignedScalarReadTest.cpp) add_turing_test(test_io_filewriter FileWriterTest.cpp) From 3a51201efa1a09e3efc4df778b69a90d06b2b4f5 Mon Sep 17 00:00:00 2001 From: Remy Boutonnet Date: Wed, 11 Nov 2026 10:55:19 +0100 Subject: [PATCH 2/4] IO: zero the end of every page a page writer flushes --- io/fs/FilePageWriter.cpp | 11 +-- test/io/CMakeLists.txt | 1 + test/io/FilePageWriterZeroTailTest.cpp | 99 ++++++++++++++++++++ test/storage/dump/PropertyDumpFormatTest.cpp | 2 +- 4 files changed, 105 insertions(+), 8 deletions(-) create mode 100644 test/io/FilePageWriterZeroTailTest.cpp diff --git a/io/fs/FilePageWriter.cpp b/io/fs/FilePageWriter.cpp index 5bdebf404d..d6b23c6b36 100644 --- a/io/fs/FilePageWriter.cpp +++ b/io/fs/FilePageWriter.cpp @@ -3,6 +3,8 @@ #include #include +#include + #include "BioAssert.h" using namespace fs; @@ -80,9 +82,6 @@ void FilePageWriter::finish() { } if (_buffer.size() > 0) { - if (_buffer.size() != _buffer.capacity()) { - std::memset(_buffer.data() + _buffer.size(), 0, _buffer.avail()); - } flush(); sync(); } @@ -92,10 +91,6 @@ void FilePageWriter::finish() { } void FilePageWriter::nextPage() { - if (_buffer.size() != _buffer.capacity()) { - std::memset(_buffer.data() + _buffer.size(), 0, _buffer.avail()); - } - flush(); } @@ -114,6 +109,8 @@ void FilePageWriter::reserveSpace(size_t byteCount) { } void FilePageWriter::flush() { + std::fill_n(_buffer.data() + _buffer.size(), _buffer.avail(), 0); + ssize_t remainingBytes = buffer().capacity(); ssize_t bytesWritten = 0; diff --git a/test/io/CMakeLists.txt b/test/io/CMakeLists.txt index 0e0b7830c7..47a7a35665 100644 --- a/test/io/CMakeLists.txt +++ b/test/io/CMakeLists.txt @@ -4,6 +4,7 @@ add_turing_test(test_io_bytebufferiterator ByteBufferIteratorTest.cpp) add_turing_test(test_io_unaligned_scalar_read UnalignedScalarReadTest.cpp) add_turing_test(test_io_filewriter FileWriterTest.cpp) add_turing_test(test_io_filepagewriter FilePageWriterTest.cpp) +add_turing_test(test_io_filepagewriter_zero_tail FilePageWriterZeroTailTest.cpp) add_turing_test(test_io_filepagereader FilePageReaderTest.cpp) add_turing_test(test_io_fileresult FileResultTest.cpp) add_turing_test(test_io_s3 S3Test.cpp) diff --git a/test/io/FilePageWriterZeroTailTest.cpp b/test/io/FilePageWriterZeroTailTest.cpp new file mode 100644 index 0000000000..88ee45d846 --- /dev/null +++ b/test/io/FilePageWriterZeroTailTest.cpp @@ -0,0 +1,99 @@ +#include +#include + +#include +#include +#include +#include +#include + +#include "TuringTest.h" + +#include "FilePageWriter.h" +#include "Path.h" + +using namespace turing::test; + +namespace { + +constexpr size_t PAGE_SIZE = 16384; +constexpr uint64_t VALUE = 42; + +} + +class FilePageWriterZeroTailTest : public TuringTest { +protected: + void initialize() override { + _path = fs::Path {_outDir + "/pages"}; + } + + void writeStalePageThenValue(fs::FilePageWriter& writer) { + const std::vector stalePage(PAGE_SIZE, 0xFF); + writer.writeToCurrentPage(std::span {stalePage}); + writer.nextPage(); + + writer.writeToCurrentPage(VALUE); + } + + void expectZerosAfterValue() { + std::ifstream file(_path.c_str(), std::ios::binary); + const std::vector bytes {std::istreambuf_iterator {file}, + std::istreambuf_iterator {}}; + ASSERT_EQ(bytes.size(), 2 * PAGE_SIZE); + + const uint8_t* secondPage = bytes.data() + PAGE_SIZE; + uint64_t value {0}; + memcpy(&value, secondPage, sizeof(uint64_t)); + EXPECT_EQ(value, VALUE); + + const bool tailIsZero = std::all_of(secondPage + sizeof(uint64_t), + bytes.data() + bytes.size(), + [](uint8_t byte) { return byte == 0; }); + EXPECT_TRUE(tailIsZero); + } + + fs::Path _path; +}; + +TEST_F(FilePageWriterZeroTailTest, SeekWritesZerosAfterTheBytesOfItsPage) { + { + auto writer = fs::FilePageWriter::open(_path, PAGE_SIZE); + ASSERT_TRUE(writer.has_value()); + + writeStalePageThenValue(writer.value()); + writer->seek(0); + ASSERT_FALSE(writer->errorOccured()); + } + + expectZerosAfterValue(); +} + +TEST_F(FilePageWriterZeroTailTest, NextPageWritesZerosAfterTheBytesOfItsPage) { + { + auto writer = fs::FilePageWriter::open(_path, PAGE_SIZE); + ASSERT_TRUE(writer.has_value()); + + writeStalePageThenValue(writer.value()); + writer->nextPage(); + ASSERT_FALSE(writer->errorOccured()); + } + + expectZerosAfterValue(); +} + +TEST_F(FilePageWriterZeroTailTest, FinishWritesZerosAfterTheBytesOfItsPage) { + { + auto writer = fs::FilePageWriter::open(_path, PAGE_SIZE); + ASSERT_TRUE(writer.has_value()); + + writeStalePageThenValue(writer.value()); + writer->finish(); + ASSERT_FALSE(writer->errorOccured()); + } + + expectZerosAfterValue(); +} + +int main(int argc, char** argv) { + return turing::test::turingTestMain(argc, argv); +} diff --git a/test/storage/dump/PropertyDumpFormatTest.cpp b/test/storage/dump/PropertyDumpFormatTest.cpp index 168ab15509..25606c297c 100644 --- a/test/storage/dump/PropertyDumpFormatTest.cpp +++ b/test/storage/dump/PropertyDumpFormatTest.cpp @@ -86,7 +86,7 @@ TEST_F(PropertyDumpFormatTest, stringDumpBytes) { uint64_t hash = 0; hashDumpFileContent(path, hash); - EXPECT_EQ(hash, 3120130461636103366ull); + EXPECT_EQ(hash, 5183985201322724322ull); { auto reader = fs::FilePageReader::open(path, DumpConfig::PAGE_SIZE); From f3629a21c335dcdf5fce4ac7ef052033af30720c Mon Sep 17 00:00:00 2001 From: Remy Boutonnet Date: Wed, 11 Nov 2026 10:55:24 +0100 Subject: [PATCH 3/4] Test: write initialised bytes in FileWriterTest --- test/io/FileWriterTest.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/io/FileWriterTest.cpp b/test/io/FileWriterTest.cpp index bb0270ff15..161b73df48 100644 --- a/test/io/FileWriterTest.cpp +++ b/test/io/FileWriterTest.cpp @@ -27,7 +27,7 @@ TEST_F(FileWriterTest, General) { // FileWriter with small buffer writer.setFile(&file); - std::array bytes; + std::array bytes {}; bytes[0] = 0; bytes[3] = 3; bytes[6] = 6; From f61d4f6cfae74f8ee6cb8dfbe7fafa777cfab42f Mon Sep 17 00:00:00 2001 From: Remy Boutonnet Date: Wed, 11 Nov 2026 10:56:49 +0100 Subject: [PATCH 4/4] Test: print a written-entity case by its name --- .../SubqueryWrittenEntityMatrixTest.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/test/query/ir/subquery/SubqueryWrittenEntityMatrixTest.cpp b/test/query/ir/subquery/SubqueryWrittenEntityMatrixTest.cpp index f74233b258..fac8d4586e 100644 --- a/test/query/ir/subquery/SubqueryWrittenEntityMatrixTest.cpp +++ b/test/query/ir/subquery/SubqueryWrittenEntityMatrixTest.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -544,16 +545,25 @@ std::string_view consumerName(Consumer consumer) { return ""; } -std::string caseName(const testing::TestParamInfo& info) { - const WrittenEntityCase& testCase = info.param; - - std::string name {formName(testCase._form)}; +void buildCaseName(const WrittenEntityCase& testCase, std::string& name) { + name = formName(testCase._form); name += testCase._entity == Entity::NODE ? "_Node_" : "_Edge_"; name += writeName(testCase._remyWrite); name += "_"; name += writeName(testCase._otherWrite); name += testCase._optional ? "_Optional_" : "_"; name += consumerName(testCase._consumer); +} + +void PrintTo(const WrittenEntityCase& testCase, std::ostream* stream) { + std::string name; + buildCaseName(testCase, name); + *stream << name; +} + +std::string caseName(const testing::TestParamInfo& info) { + std::string name; + buildCaseName(info.param, name); return name; }