diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 544137a8..f15414c0 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -27,6 +27,16 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 runtime `AreRecordsEncrypted` gate (so default-config plaintext databases benefit without `NoEncryptMode`). Fixed-size hash-indexed SET columns are re-pointed with one lock per index (`HashIndex.RemoveBatchKeys`/`AddBatchKeys`). +- **Buffered in-place UPDATE overwrites are now flushed per storage page (C6, Fase B)** - the + UPDATE commit path buffered one record per row (B7) and flushed each with two pwrites + (length prefix + payload), so ~10K-row UPDATEs were dominated by per-row write syscalls + (~88K ops/s on the fair-PK fixed-width table). `FlushBufferedOverwrites` now batches: the + current on-disk page content is read once, the row payloads are patched into the copy, and each + touched page is written once (payloads crossing a page boundary take the direct path first; + length prefixes are unchanged because same-length overwrites are the only in-place case). + Fair-PK harness (`--pk`, same machine as master): fixed-width UPDATE ~88K -> **~153-164K ops/s** + (+75-85%, gap vs SQLite ~2,75x -> ~1,7x); legacy UPDATE ~53-56K -> ~64-70K (+20-25%); DELETE + unchanged within noise. Runs under the same commit lock; rollback semantics unchanged. - **Duplicate-key hash-index removal is no longer quadratic (P5)** - `HashIndex.RemoveBatchKeys`/ `RemoveBatch` previously removed every position from a key's list with one O(list) shift per duplicate, i.e. O(m·n) for a key holding n rows with m duplicate-key deletions in one batch. diff --git a/docs/performance/EXECUTION_PLAN_UPDATE_DELETE.md b/docs/performance/EXECUTION_PLAN_UPDATE_DELETE.md index d1837725..bd749983 100644 --- a/docs/performance/EXECUTION_PLAN_UPDATE_DELETE.md +++ b/docs/performance/EXECUTION_PLAN_UPDATE_DELETE.md @@ -14,6 +14,7 @@ | #376 | **Bulk aflopende PK-delete** (`DeleteRecordsCore` verzamelt PK-sleutels eenmalig; `IIndex.DeleteBulk`/`BTree.DeleteBulk` sorteert aflopend → rechter-bladpad, minder separator-promoties) | correct (identieke keyset, één bezoek per key); fair-PK legacy-DELETE ~69-72K ops/s (binnen ruis op geordende batches) — winst bij ongeordende keysets | | #377 | **Commit-marker writes gebatcht** (`TombstoneRecords` patcht alle markers eerst in het whole-file snapshot — markers mogen page-grenzen kruisen — en flusht elke geraakte storage-page één keer i.p.v. één 4B-pwrite per marker) | fair-PK (median, zelfde machine): legacy ~70K → **~81K ops/s** (+16%); fixed-width ~97K → **~141K ops/s** (+45%) — DELETE-gap vs SQLite op FW → ~2,4x | | P5 (deze branch) | **Duplicate-key hash-removal O(m·n) → O(n) per key** (`RemoveBatchKeys`/`RemoveBatch`: directe allocatie-vrije pad voor single-row keys; gedupliceerde keys gedeferred in set + één O(list)-compaction) | correct (regressietests op volle + partiële duplicate-groepen, beide backends, incl. reopen); benchmark-neutraal op unieke keys | +| C6 (deze branch) | **In-place UPDATE-overwrites per storage-page geflusht** (`FlushBufferedOverwrites`: page-content één keer lezen, payloads patchen, één write per page; cross-page records direct) | fair-PK: FW-UPDATE ~88K → **~153-164K ops/s** (+75-85%, gap → ~1,7x SQLite); legacy-UPDATE ~53-56K → ~64-70K; DELETE binnen ruis; full suite 1765/0 | **Root cause (niet in Grok-doc):** `ExecuteBatchSQL` draait elke batch in een storage-transactie; zonder #368 deed batch-DELETE nog steeds de #366 full-file compactie (~690 ms in `tableFlushLoop`). Daardoor waren eerdere “winst”-metingen niet-duurzaam/logisch-only. diff --git a/src/SharpCoreDB/Services/Storage.Append.cs b/src/SharpCoreDB/Services/Storage.Append.cs index 95de6614..29e05c87 100644 --- a/src/SharpCoreDB/Services/Storage.Append.cs +++ b/src/SharpCoreDB/Services/Storage.Append.cs @@ -7,6 +7,7 @@ namespace SharpCoreDB.Services; using SharpCoreDB.Constants; using System; +using System.Buffers; using System.Buffers.Binary; using System.Collections.Concurrent; using System.Collections.Generic; @@ -1081,11 +1082,14 @@ private void FlushBufferedOverwrites() try { - Span lengthPrefix = stackalloc byte[4]; - foreach (var (offset, record) in overwrites) + if (!TryFlushBufferedOverwritesBatched(path, overwrites)) { - BinaryPrimitives.WriteInt32LittleEndian(lengthPrefix, record.Length); - WriteRecordInPlace(path, offset, lengthPrefix, record); + Span lengthPrefix = stackalloc byte[4]; + foreach (var (offset, record) in overwrites) + { + BinaryPrimitives.WriteInt32LittleEndian(lengthPrefix, record.Length); + WriteRecordInPlace(path, offset, lengthPrefix, record); + } } } catch (IOException) @@ -1097,6 +1101,130 @@ private void FlushBufferedOverwrites() bufferedOverwrites.Clear(); } + /// + /// Batch-flushes buffered in-place overwrites with one write per touched storage page instead + /// of two pwrites per row (the UPDATE commit previously did one length-prefix write + one + /// payload write per buffered row). Current on-disk page content is read once, row payloads are + /// patched into the copy, and the page is written once; length prefixes are unchanged + /// (same-length overwrites only). Cross-page payloads take the direct per-record path first so + /// later page reads already include them. + /// + /// + /// Safety: runs under in the commit path (single writer per file). + /// Returns false when batching is not applicable so the caller falls back to the per-record + /// loop — re-writing the same bytes is idempotent. + /// + /// True when every overwrite was flushed by this method. + private bool TryFlushBufferedOverwritesBatched(string path, Dictionary overwrites) + { + if (overwrites.Count < 64 || path.EndsWith(".ovf", StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + int pageBytes = this.pageSize > 0 ? this.pageSize : 4096; + long fileLength = File.Exists(path) ? new FileInfo(path).Length : 0; + if (fileLength <= 0) + { + return false; + } + + var entries = new (long Offset, byte[] Payload)[overwrites.Count]; + int n = 0; + foreach (var (offset, payload) in overwrites) + { + if (offset >= 0 && offset + 4 + payload.Length <= fileLength) + { + entries[n++] = (offset, payload); + } + } + + if (n == 0) + { + return false; + } + + Array.Sort(entries, 0, n, Comparer<(long Offset, byte[] Payload)>.Create(static (a, b) => a.Offset.CompareTo(b.Offset))); + + var pages = new Dictionary>(); + var pageStarts = new List(); + var direct = new List<(long Offset, byte[] Payload)>(); + for (int i = 0; i < n; i++) + { + long offset = entries[i].Offset; + byte[] payload = entries[i].Payload; + long pageStart = (offset / pageBytes) * pageBytes; + if (offset + 4 + payload.Length - pageStart > pageBytes) + { + direct.Add((offset, payload)); + continue; + } + + if (!pages.TryGetValue(pageStart, out var patches)) + { + patches = new List<(int, byte[])>(); + pages[pageStart] = patches; + pageStarts.Add(pageStart); + } + + patches.Add(((int)(offset - pageStart + 4), payload)); + } + + Span lengthPrefix = stackalloc byte[4]; + foreach (var (offset, payload) in direct) + { + BinaryPrimitives.WriteInt32LittleEndian(lengthPrefix, payload.Length); + WriteRecordInPlace(path, offset, lengthPrefix, payload); + } + + if (pages.Count == 0) + { + return direct.Count > 0; + } + + SafeFileHandle readHandle = GetOrOpenReadHandle(path); + SafeFileHandle writeHandle = GetOrOpenWriteHandle(path); + byte[]? pageBuffer = null; + try + { + pageBuffer = ArrayPool.Shared.Rent(pageBytes); + pageStarts.Sort(); + foreach (long pageStart in pageStarts) + { + int writeLength = (int)Math.Min(pageBytes, fileLength - pageStart); + if (writeLength <= 0) + { + continue; + } + + if (RandomAccess.Read(readHandle, pageBuffer.AsSpan(0, writeLength), pageStart) != writeLength) + { + return false; // partial read — fall back to the idempotent per-record loop + } + + foreach (var (relOffset, payload) in pages[pageStart]) + { + payload.CopyTo(pageBuffer.AsSpan(relOffset, payload.Length)); + } + + RandomAccess.Write(writeHandle, pageBuffer.AsSpan(0, writeLength), pageStart); + if (this.pageCache != null) + { + this.pageCache.EvictPage(ComputePageId(path, pageStart)); + } + } + } + finally + { + if (pageBuffer != null) + { + ArrayPool.Shared.Return(pageBuffer); + } + } + + return true; + } + /// [MethodImpl(MethodImplOptions.AggressiveOptimization)] public byte[]? ReadBytesFrom(string path, long offset) diff --git a/tests/SharpCoreDB.Tests/FixedWidthBulkDeleteTests.cs b/tests/SharpCoreDB.Tests/FixedWidthBulkDeleteTests.cs index ade8a7f5..f010f900 100644 --- a/tests/SharpCoreDB.Tests/FixedWidthBulkDeleteTests.cs +++ b/tests/SharpCoreDB.Tests/FixedWidthBulkDeleteTests.cs @@ -137,6 +137,61 @@ public void DescendingUpdateBatch_FallsBackToGenericLoop_AndAppliesEveryRow() finally { (db as IDisposable)?.Dispose(); } } + [Fact] + public void AscendingUpdateBatch_BatchedPageFlush_AppliesEveryRow_AcrossReopen() + { + // Regression for the commit flush of buffered in-place overwrites: a single UPDATE batch of + // 2000 rows lands in one transaction with 2000 buffered overwrites, which FlushBuffered- + // Overwrites now writes with one read-modify-write per touched storage page instead of two + // pwrites per row. Values must survive point reads and a reopen. + IDatabase? db = CreateDb(); + try + { + db.ExecuteSQL("CREATE TABLE docs (id INTEGER PRIMARY KEY, name TEXT, score REAL)"); + InsertDocs(db, 1, 2000); + db.Flush(); + + var table = TableOf(db, "docs"); + var stmts = new List(2000); + for (int i = 1; i <= 2000; i++) + { + stmts.Add(string.Format(CultureInfo.InvariantCulture, + "UPDATE docs SET score = 9.25 WHERE id = {0}", i)); + } + + db.ExecuteBatchSQL(stmts); + db.Flush(); + Assert.Equal(1, table.BulkContiguousUpdateBatches); + + var c = db.ExecuteQuery("SELECT COUNT(*) AS n FROM docs"); + Assert.Equal(2000L, Convert.ToInt64(c[0].Values.First())); + for (int i = 1; i <= 2000; i += 137) + { + var rows = db.ExecuteQuery("SELECT score, name FROM docs WHERE id = @id", + new Dictionary { ["@id"] = i }); + Assert.Single(rows); + Assert.Equal(9.25, Convert.ToDouble(rows[0]["score"])); + } + } + finally { (db as IDisposable)?.Dispose(); } + + // Reopen: the read-modify-write page flushes must have persisted exactly. + db = CreateDb(); + try + { + var scan = db.ExecuteQuery("SELECT id FROM docs"); + Assert.Equal(2000, scan.Count); + for (int i = 1; i <= 2000; i += 333) + { + var rows = db.ExecuteQuery("SELECT score FROM docs WHERE id = @id", + new Dictionary { ["@id"] = i }); + Assert.Single(rows); + Assert.Equal(9.25, Convert.ToDouble(rows[0]["score"])); + } + } + finally { (db as IDisposable)?.Dispose(); } + } + [Fact] public void BulkDelete_ScanAndCountStayConsistent() {