diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 195b81f3..785ff85b 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -27,6 +27,13 @@ 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`). +- **Commit-time tombstones now batch the marker writes (C5)** - the DELETE commit phase read the + whole file once (#373) but still applied one 4-byte negative-prefix marker per row + (one pwrite each). `TombstoneRecords` now patches every marker into the in-memory snapshot first + (markers may straddle page boundaries, so patching happens on the contiguous buffer) and flushes + each touched storage page once, byte-for-byte equivalent. Fair-PK harness (`--pk`, median of runs, + same machine as master): legacy DELETE ~70K -> **~81K ops/s** (+16%); fixed-width DELETE + ~97K -> **~141K ops/s** (+45%) - the DELETE gap vs SQLite on fixed-width drops to ~2.4x. - **Bulk descending PK-delete on the generic DELETE path** - `IIndex` now offers `DeleteBulk`; `BTree.DeleteBulk` sorts each batch in **descending key order** so consecutive removals run along the rightmost leaf path (dramatically fewer internal-separator promotions than deleting in diff --git a/docs/performance/EXECUTION_PLAN_UPDATE_DELETE.md b/docs/performance/EXECUTION_PLAN_UPDATE_DELETE.md index 95d3397c..b1bd3855 100644 --- a/docs/performance/EXECUTION_PLAN_UPDATE_DELETE.md +++ b/docs/performance/EXECUTION_PLAN_UPDATE_DELETE.md @@ -11,7 +11,8 @@ | #368 | **Commit-time tombstones** (transactionele/batch deletes) | **SQL DELETE 0,82 s → 0,24 s** (~12K → ~41-58K ops/s) — de grote sprong | | #369 | C4 (batch markers + evict-dedup) + B3 (structured delete, geen dubbele parse) | veilig; neutraal binnen ruis op benchmark | | #370 | B1 (key-only decode: alleen PK + hash-indexkolommen) | veilig; neutraal binnen ruis op small-row benchmark | -| B5-bulk (open) | **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 | +| #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 | +| C5 (open) | **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 | **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. @@ -86,7 +87,7 @@ Gemeten op master (Release, zelfde machine; median van 3 runs voor de comparativ ### Bewuste vervolgstappen (niet in deze sessie, zie secties 3-4) 1. **Batch-PK stale/lazy-rebuild** na grote delete-batches — grootste open post; vereist eerst PK-index-refresh-infra (`Table.Index` is een plain property zonder lazy rebuild). Ontwerp nodig; daarna kan de per-rij read in DELETE vervallen. -2. **Commit-marker schrijfbatching** (writes blijven per-offset; range-read zit er al in via #373). +2. **Commit-marker schrijfbatching** (writes blijven per-offset; range-read zit er al in via #373) — **opgelost op de C5-branch**: markers worden per storage-page gebundeld weggeschreven. 3. **Fase B (structureel):** fixed-width in-place engine + PageBased als OLTP-default — de weg naar ~1,2-1,5× van SQLite op UPDATE/DELETE. 4. **Fase C/D (platform):** AOT/R2R als aparte meet-as, median-of-N in de harness, `dotnet-trace` per Fase-B-stap. diff --git a/src/SharpCoreDB/Services/Storage.Append.cs b/src/SharpCoreDB/Services/Storage.Append.cs index 4ce7b1b4..95de6614 100644 --- a/src/SharpCoreDB/Services/Storage.Append.cs +++ b/src/SharpCoreDB/Services/Storage.Append.cs @@ -708,24 +708,7 @@ public void TombstoneRecords(string path, long[] offsets) if (wholeFile is not null) { - foreach (var offset in offsets) - { - if (offset < 0 || offset + 4 > wholeFile.Length) - { - continue; - } - - int currentLength = BinaryPrimitives.ReadInt32LittleEndian(wholeFile.AsSpan((int)offset, 4)); - if (currentLength <= 0) - { - continue; // already tombstoned or invalid - } - - BinaryPrimitives.WriteInt32LittleEndian(marker, -(4 + currentLength)); - WriteRecordInPlace(path, offset, marker, ReadOnlySpan.Empty); - - pagesToEvict?.Add(ComputePageId(path, offset)); - } + WriteTombstoneMarkersBatched(path, offsets, wholeFile, pagesToEvict); } else { @@ -768,6 +751,97 @@ public void TombstoneRecords(string path, long[] offsets) } } + /// + /// Applies commit-time tombstone markers for a batch whose record lengths are already resolved + /// from a whole-file snapshot. The snapshot is patched in memory first (a marker is just a + /// 4-byte negative length-prefix flip, possibly crossing a storage-page boundary), then every + /// touched page is flushed with a single write — #373 batched the per-marker length reads; this + /// batches the marker writes (previously one 4-byte pwrite per marker, which dominated the + /// DELETE commit phase on dense batches). A flushed page differs from the on-disk bytes only in + /// its marker words, so the full-page write is byte-for-byte equivalent to the individual + /// marker writes it replaces. + /// + /// + /// Safety: this helper runs under the same lock as the old per-marker loop. In the transaction + /// commit path that is (only the committing thread writes the file); + /// in the durable non-transactional DELETE path the caller already holds the table write lock. + /// Because each page is written at most once and always from the patched in-memory snapshot, a + /// rollback can never observe a partially applied marker. + /// + private void WriteTombstoneMarkersBatched(string path, long[] offsets, byte[] wholeFile, HashSet? pagesToEvict) + { + int pageSizeInt = this.pageSize > 0 ? this.pageSize : 4096; + long pageBytes = pageSizeInt; + + // Pass 1 — patch every valid marker into the snapshot buffer and record the pages touched. + // A marker's 4 bytes may straddle a page boundary (records are not page-aligned), which is + // why the patch happens on the contiguous buffer and NOT on per-page copies. + var pages = new HashSet(); + for (int i = 0; i < offsets.Length; i++) + { + long offset = offsets[i]; + if (offset < 0 || offset + 4 > wholeFile.Length) + { + continue; + } + + int currentLength = BinaryPrimitives.ReadInt32LittleEndian(wholeFile.AsSpan((int)offset, 4)); + if (currentLength <= 0) + { + continue; // already tombstoned or invalid + } + + BinaryPrimitives.WriteInt32LittleEndian(wholeFile.AsSpan((int)offset, 4), -(4 + currentLength)); + pages.Add((offset / pageBytes) * pageBytes); + pages.Add(((offset + 3) / pageBytes) * pageBytes); + } + + if (pages.Count == 0) + { + return; + } + + // Pass 2 — flush each touched page once from the patched buffer. + bool isOvf = path.EndsWith(".ovf", StringComparison.OrdinalIgnoreCase); + SafeFileHandle? writeHandle = isOvf ? null : GetOrOpenWriteHandle(path); + FileStream? ovfStream = null; + try + { + if (isOvf) + { + ovfStream = new FileStream(path, FileMode.Open, FileAccess.Write, FileShare.ReadWrite | FileShare.Delete, 4096, FileOptions.None); + } + + var sortedPages = new long[pages.Count]; + pages.CopyTo(sortedPages); + Array.Sort(sortedPages); + + foreach (long pageStart in sortedPages) + { + int writeLength = (int)Math.Min(pageBytes, wholeFile.Length - pageStart); + if (writeLength <= 0) + { + continue; + } + + pagesToEvict?.Add(ComputePageId(path, pageStart)); + if (isOvf) + { + ovfStream!.Position = pageStart; + ovfStream.Write(wholeFile, (int)pageStart, writeLength); + } + else + { + RandomAccess.Write(writeHandle!, wholeFile.AsSpan((int)pageStart, writeLength), pageStart); + } + } + } + finally + { + ovfStream?.Dispose(); + } + } + /// [MethodImpl(MethodImplOptions.AggressiveOptimization)] public long[] AppendBytesMultiple(string path, List dataBlocks)