Skip to content

Commit 8d608f0

Browse files
author
MPCoreDeveloper
committed
perf(delete): batch commit-time tombstone marker writes per storage page (C5)
ApplyBufferedTombstones -> TombstoneRecords still issued one 4-byte pwrite per deleted row even after #373 batched the per-marker length reads. The DELETE commit phase now patches every negative-prefix marker into the in-memory whole-file snapshot first (markers may straddle page boundaries because records are not page-aligned, so patching must happen on the contiguous buffer, not on per-page copies) and then flushes each touched storage page exactly once. A flushed page differs from the on-disk bytes only in its flipped marker words, so the full-page write is byte-for-byte equivalent to the individual marker writes it replaces. Runs under the same lock as before (appendLock on the commit path / table write lock on the durable path), so rollback semantics are unchanged. Fair-PK harness (--pk, same machine as master, 2 runs each): legacy DELETE ~70K -> ~81K ops/s (+16%) fixed-width DEL ~97K -> ~141K ops/s (+45%); DELETE gap vs SQLite -> ~2.4x Full suite: 1762 tests, 0 failed.
1 parent 2ce18f6 commit 8d608f0

3 files changed

Lines changed: 102 additions & 20 deletions

File tree

‎docs/CHANGELOG.md‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2727
runtime `AreRecordsEncrypted` gate (so default-config plaintext databases benefit without
2828
`NoEncryptMode`). Fixed-size hash-indexed SET columns are re-pointed with one lock per index
2929
(`HashIndex.RemoveBatchKeys`/`AddBatchKeys`).
30+
- **Commit-time tombstones now batch the marker writes (C5)** - the DELETE commit phase read the
31+
whole file once (#373) but still applied one 4-byte negative-prefix marker per row
32+
(one pwrite each). `TombstoneRecords` now patches every marker into the in-memory snapshot first
33+
(markers may straddle page boundaries, so patching happens on the contiguous buffer) and flushes
34+
each touched storage page once, byte-for-byte equivalent. Fair-PK harness (`--pk`, median of runs,
35+
same machine as master): legacy DELETE ~70K -> **~81K ops/s** (+16%); fixed-width DELETE
36+
~97K -> **~141K ops/s** (+45%) - the DELETE gap vs SQLite on fixed-width drops to ~2.4x.
3037
- **Bulk descending PK-delete on the generic DELETE path** - `IIndex` now offers `DeleteBulk`;
3138
`BTree.DeleteBulk` sorts each batch in **descending key order** so consecutive removals run along
3239
the rightmost leaf path (dramatically fewer internal-separator promotions than deleting in

‎docs/performance/EXECUTION_PLAN_UPDATE_DELETE.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
| #368 | **Commit-time tombstones** (transactionele/batch deletes) | **SQL DELETE 0,82 s → 0,24 s** (~12K → ~41-58K ops/s) — de grote sprong |
1212
| #369 | C4 (batch markers + evict-dedup) + B3 (structured delete, geen dubbele parse) | veilig; neutraal binnen ruis op benchmark |
1313
| #370 | B1 (key-only decode: alleen PK + hash-indexkolommen) | veilig; neutraal binnen ruis op small-row benchmark |
14-
| 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 |
14+
| #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 |
15+
| 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 |
1516

1617
**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.
1718

@@ -86,7 +87,7 @@ Gemeten op master (Release, zelfde machine; median van 3 runs voor de comparativ
8687

8788
### Bewuste vervolgstappen (niet in deze sessie, zie secties 3-4)
8889
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.
89-
2. **Commit-marker schrijfbatching** (writes blijven per-offset; range-read zit er al in via #373).
90+
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.
9091
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.
9192
4. **Fase C/D (platform):** AOT/R2R als aparte meet-as, median-of-N in de harness, `dotnet-trace` per Fase-B-stap.
9293

‎src/SharpCoreDB/Services/Storage.Append.cs‎

Lines changed: 92 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -708,24 +708,7 @@ public void TombstoneRecords(string path, long[] offsets)
708708

709709
if (wholeFile is not null)
710710
{
711-
foreach (var offset in offsets)
712-
{
713-
if (offset < 0 || offset + 4 > wholeFile.Length)
714-
{
715-
continue;
716-
}
717-
718-
int currentLength = BinaryPrimitives.ReadInt32LittleEndian(wholeFile.AsSpan((int)offset, 4));
719-
if (currentLength <= 0)
720-
{
721-
continue; // already tombstoned or invalid
722-
}
723-
724-
BinaryPrimitives.WriteInt32LittleEndian(marker, -(4 + currentLength));
725-
WriteRecordInPlace(path, offset, marker, ReadOnlySpan<byte>.Empty);
726-
727-
pagesToEvict?.Add(ComputePageId(path, offset));
728-
}
711+
WriteTombstoneMarkersBatched(path, offsets, wholeFile, pagesToEvict);
729712
}
730713
else
731714
{
@@ -768,6 +751,97 @@ public void TombstoneRecords(string path, long[] offsets)
768751
}
769752
}
770753

754+
/// <summary>
755+
/// Applies commit-time tombstone markers for a batch whose record lengths are already resolved
756+
/// from a whole-file snapshot. The snapshot is patched in memory first (a marker is just a
757+
/// 4-byte negative length-prefix flip, possibly crossing a storage-page boundary), then every
758+
/// touched page is flushed with a single write — #373 batched the per-marker length reads; this
759+
/// batches the marker writes (previously one 4-byte pwrite per marker, which dominated the
760+
/// DELETE commit phase on dense batches). A flushed page differs from the on-disk bytes only in
761+
/// its marker words, so the full-page write is byte-for-byte equivalent to the individual
762+
/// marker writes it replaces.
763+
/// </summary>
764+
/// <remarks>
765+
/// Safety: this helper runs under the same lock as the old per-marker loop. In the transaction
766+
/// commit path that is <see cref="appendLock"/> (only the committing thread writes the file);
767+
/// in the durable non-transactional DELETE path the caller already holds the table write lock.
768+
/// Because each page is written at most once and always from the patched in-memory snapshot, a
769+
/// rollback can never observe a partially applied marker.
770+
/// </remarks>
771+
private void WriteTombstoneMarkersBatched(string path, long[] offsets, byte[] wholeFile, HashSet<int>? pagesToEvict)
772+
{
773+
int pageSizeInt = this.pageSize > 0 ? this.pageSize : 4096;
774+
long pageBytes = pageSizeInt;
775+
776+
// Pass 1 — patch every valid marker into the snapshot buffer and record the pages touched.
777+
// A marker's 4 bytes may straddle a page boundary (records are not page-aligned), which is
778+
// why the patch happens on the contiguous buffer and NOT on per-page copies.
779+
var pages = new HashSet<long>();
780+
for (int i = 0; i < offsets.Length; i++)
781+
{
782+
long offset = offsets[i];
783+
if (offset < 0 || offset + 4 > wholeFile.Length)
784+
{
785+
continue;
786+
}
787+
788+
int currentLength = BinaryPrimitives.ReadInt32LittleEndian(wholeFile.AsSpan((int)offset, 4));
789+
if (currentLength <= 0)
790+
{
791+
continue; // already tombstoned or invalid
792+
}
793+
794+
BinaryPrimitives.WriteInt32LittleEndian(wholeFile.AsSpan((int)offset, 4), -(4 + currentLength));
795+
pages.Add((offset / pageBytes) * pageBytes);
796+
pages.Add(((offset + 3) / pageBytes) * pageBytes);
797+
}
798+
799+
if (pages.Count == 0)
800+
{
801+
return;
802+
}
803+
804+
// Pass 2 — flush each touched page once from the patched buffer.
805+
bool isOvf = path.EndsWith(".ovf", StringComparison.OrdinalIgnoreCase);
806+
SafeFileHandle? writeHandle = isOvf ? null : GetOrOpenWriteHandle(path);
807+
FileStream? ovfStream = null;
808+
try
809+
{
810+
if (isOvf)
811+
{
812+
ovfStream = new FileStream(path, FileMode.Open, FileAccess.Write, FileShare.ReadWrite | FileShare.Delete, 4096, FileOptions.None);
813+
}
814+
815+
var sortedPages = new long[pages.Count];
816+
pages.CopyTo(sortedPages);
817+
Array.Sort(sortedPages);
818+
819+
foreach (long pageStart in sortedPages)
820+
{
821+
int writeLength = (int)Math.Min(pageBytes, wholeFile.Length - pageStart);
822+
if (writeLength <= 0)
823+
{
824+
continue;
825+
}
826+
827+
pagesToEvict?.Add(ComputePageId(path, pageStart));
828+
if (isOvf)
829+
{
830+
ovfStream!.Position = pageStart;
831+
ovfStream.Write(wholeFile, (int)pageStart, writeLength);
832+
}
833+
else
834+
{
835+
RandomAccess.Write(writeHandle!, wholeFile.AsSpan((int)pageStart, writeLength), pageStart);
836+
}
837+
}
838+
}
839+
finally
840+
{
841+
ovfStream?.Dispose();
842+
}
843+
}
844+
771845
/// <inheritdoc />
772846
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
773847
public long[] AppendBytesMultiple(string path, List<byte[]> dataBlocks)

0 commit comments

Comments
 (0)