diff --git a/Tests/LibSQL/TestSqlHeap.cpp b/Tests/LibSQL/TestSqlHeap.cpp index 9a1eaccc4c..b2ac4de150 100644 --- a/Tests/LibSQL/TestSqlHeap.cpp +++ b/Tests/LibSQL/TestSqlHeap.cpp @@ -96,3 +96,34 @@ TEST_CASE(heap_overwrite_large_storage) auto stored_longest_string = TRY_OR_FAIL(heap->read_storage(storage_block_id)); EXPECT_EQ(longest_string.bytes(), stored_longest_string.bytes()); } + +TEST_CASE(heap_reuse_freed_blocks_after_storage_trim) +{ + ScopeGuard guard([]() { MUST(Core::System::unlink(db_path)); }); + auto heap = create_heap(); + + // First, write storage spanning 4 blocks + auto first_index = heap->request_new_block_index(); + StringBuilder builder; + MUST(builder.try_append_repeated('x', SQL::Block::DATA_SIZE * 4)); + auto long_string = builder.string_view(); + TRY_OR_FAIL(heap->write_storage(first_index, long_string.bytes())); + MUST(heap->flush()); + auto original_heap_size = MUST(heap->file_size_in_bytes()); + + // Then, overwrite the first storage and reduce it to 2 blocks + builder.clear(); + MUST(builder.try_append_repeated('x', SQL::Block::DATA_SIZE * 2)); + long_string = builder.string_view(); + TRY_OR_FAIL(heap->write_storage(first_index, long_string.bytes())); + MUST(heap->flush()); + auto heap_size_after_reduction = MUST(heap->file_size_in_bytes()); + EXPECT(heap_size_after_reduction <= original_heap_size); + + // Now add the second storage spanning 2 blocks - heap should not have grown compared to the original storage + auto second_index = heap->request_new_block_index(); + TRY_OR_FAIL(heap->write_storage(second_index, long_string.bytes())); + MUST(heap->flush()); + auto heap_size_after_second_storage = MUST(heap->file_size_in_bytes()); + EXPECT(heap_size_after_second_storage <= original_heap_size); +} diff --git a/Userland/Libraries/LibSQL/Heap.cpp b/Userland/Libraries/LibSQL/Heap.cpp index ac7599e176..12a7c5460c 100644 --- a/Userland/Libraries/LibSQL/Heap.cpp +++ b/Userland/Libraries/LibSQL/Heap.cpp @@ -80,7 +80,15 @@ ErrorOr Heap::file_size_in_bytes() const bool Heap::has_block(Block::Index index) const { - return index <= m_highest_block_written || m_write_ahead_log.contains(index); + return (index <= m_highest_block_written || m_write_ahead_log.contains(index)) + && !m_free_block_indices.contains_slow(index); +} + +Block::Index Heap::request_new_block_index() +{ + if (!m_free_block_indices.is_empty()) + return m_free_block_indices.take_last(); + return m_next_block++; } ErrorOr Heap::read_storage(Block::Index index) @@ -107,21 +115,23 @@ ErrorOr Heap::write_storage(Block::Index index, ReadonlyBytes data) // Split up the storage across multiple blocks if necessary, creating a chain u32 remaining_size = static_cast(data.size()); u32 offset_in_data = 0; + Block::Index existing_next_block_index = 0; while (remaining_size > 0) { auto block_data_size = AK::min(remaining_size, Block::DATA_SIZE); remaining_size -= block_data_size; ByteBuffer block_data; - Block::Index next_block_index = 0; if (has_block(index)) { auto existing_block = TRY(read_block(index)); block_data = existing_block.data(); TRY(block_data.try_resize(block_data_size)); - next_block_index = existing_block.next_block(); + existing_next_block_index = existing_block.next_block(); } else { block_data = TRY(ByteBuffer::create_uninitialized(block_data_size)); + existing_next_block_index = 0; } + Block::Index next_block_index = existing_next_block_index; if (next_block_index == 0 && remaining_size > 0) next_block_index = request_new_block_index(); else if (remaining_size == 0) @@ -133,6 +143,14 @@ ErrorOr Heap::write_storage(Block::Index index, ReadonlyBytes data) index = next_block_index; offset_in_data += block_data_size; } + + // Free remaining blocks in existing chain, if any + while (existing_next_block_index > 0) { + auto existing_block = TRY(read_block(existing_next_block_index)); + existing_next_block_index = existing_block.next_block(); + TRY(free_block(existing_block)); + } + return {}; } @@ -207,13 +225,28 @@ ErrorOr Heap::write_block(Block const& block) return write_raw_block_to_wal(block.index(), move(heap_data)); } +ErrorOr Heap::free_block(Block const& block) +{ + auto index = block.index(); + dbgln_if(SQL_DEBUG, "{}({})", __FUNCTION__, index); + + VERIFY(index > 0); + VERIFY(has_block(index)); + + // Zero out freed blocks to facilitate a free block scan upon opening the database later + auto zeroed_data = TRY(ByteBuffer::create_zeroed(Block::SIZE)); + TRY(write_raw_block_to_wal(index, move(zeroed_data))); + + return m_free_block_indices.try_append(index); +} + ErrorOr Heap::flush() { VERIFY(m_file); auto indices = m_write_ahead_log.keys(); quick_sort(indices); for (auto index : indices) { - dbgln_if(SQL_DEBUG, "Flushing block {} to {}", index, name()); + dbgln_if(SQL_DEBUG, "Flushing block {}", index); auto& data = m_write_ahead_log.get(index).value(); TRY(write_raw_block(index, data)); } diff --git a/Userland/Libraries/LibSQL/Heap.h b/Userland/Libraries/LibSQL/Heap.h index 2a6623b9af..c583780a13 100644 --- a/Userland/Libraries/LibSQL/Heap.h +++ b/Userland/Libraries/LibSQL/Heap.h @@ -11,6 +11,7 @@ #include #include #include +#include #include #include @@ -39,6 +40,7 @@ public: , m_next_block(next_block) , m_data(move(data)) { + VERIFY(index > 0); } Index index() const { return m_index; } @@ -73,8 +75,8 @@ public: ErrorOr open(); ErrorOr file_size_in_bytes() const; - bool has_block(Block::Index) const; - [[nodiscard]] Block::Index request_new_block_index() { return m_next_block++; } + [[nodiscard]] bool has_block(Block::Index) const; + [[nodiscard]] Block::Index request_new_block_index(); Block::Index schemas_root() const { return m_schemas_root; } @@ -126,6 +128,8 @@ private: ErrorOr read_block(Block::Index); ErrorOr write_block(Block const&); + ErrorOr free_block(Block const&); + ErrorOr read_zero_block(); ErrorOr initialize_zero_block(); ErrorOr update_zero_block(); @@ -139,6 +143,7 @@ private: u32 m_version { VERSION }; Array m_user_values { 0 }; HashMap m_write_ahead_log; + Vector m_free_block_indices; }; }