Skip to content

Commit d5f29b8

Browse files
measure(diagnostics): instrument the batch INSERT and the delete fast path; attribute DELETE (index-maint 82%)
Items 2 and 3 of the agreed sequence (plus 4: AesGcmEncryption parity). Item 3 - the §2 profiler had a blind spot that cost two rounds of ad-hoc probes: the batch INSERT path had NO stamps at all, and the fixed-width bulk-delete fast path bypasses DeleteRecordsCore, so a 10K-row DELETE recorded nothing while 100% of its wall time happened there. Added: - Table.InsertBatch: Validate (covers validation + serialization) and RowLocate (batch PK probes); - the fixed-width bulk-delete fast path: RowLocate (contiguous span read), IndexMaintenance (PK DeleteBulk + every loaded hash index), EngineWrite (tombstone / commit-time buffer). Still uncovered and recorded in the plan: the bulk UPDATE path's index-maintenance block and the SQL/engine overhead outside the table (parse, plan cache, commit) - which is why a stage report's total is not wall time. Item 2 - DELETE attributed (10K by PK, 50K rows, one batch transaction): arm ops/s wall us/row in stages uninstrumented plaintext + idx_docs_name 103,495 9.66 4.29 5.37 at-rest + idx_docs_name 100,209 9.98 5.15 4.83 at-rest, no index 140,489 7.12 4.01 3.11 Stage shares at-rest: index-maintenance 81.9%, row-locate 16.3%, engine-write (the tombstone) 1.8%. So DELETE carries NO at-rest tax (1.03x) and the plan's expected culprit (the tombstone) is noise: the cost is index maintenance, paid per loaded index per row - the delete decodes each indexed column out of the fixed-width record, which for TEXT means an overflow-arena read (and a decrypt at-rest), purely to compute the hash key. The lever is therefore bulk/deferred index removal (DeferredIndexUpdater exists), not a cheaper delete. Recorded in plan section 7 so Phase 5 is planned against the right cause. Item 4 - AesGcmEncryption (DatabaseFile / PageEncryption / single-file provider) had the same per-call defect as CryptoService: a fresh AesGcm + OS-CSPRNG nonce per page/blob operation. Now one cipher per instance (they are long-lived holders) and [random prefix(8)][counter(4)] nonces, throwing instead of wrapping the invocation field; Dispose releases the cached cipher so reuse-after-dispose cannot encrypt with the pre-clear key. AesGcmEncryptionNonceTests pins round-trips (array + span + page APIs), nonce uniqueness with the counter sequence, concurrency of ONE instance (never measured in this project before), and page nonce uniqueness. Gate: SharpCoreDB.slnx 0 errors; SharpCoreDB.Tests 1857 total / 0 failed / 16 skipped (+4 tests). Docs: plan section 7 (DELETE measurement + instrumentation coverage) and CHANGELOG Performance.
1 parent eaeb30f commit d5f29b8

5 files changed

Lines changed: 286 additions & 10 deletions

File tree

docs/CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3535
- **`USING DISKANN` silently built a `FlatIndex`**`VectorQueryOptimizer.BuildIndex` fell through to the `Flat` default arm for the `"DISKANN"` string, so the DDL produced an exact scan behind a DiskANN-shaped name. `DISKANN` now maps to `VectorIndexType.DiskAnn`.
3636

3737
### Performance
38+
- **Page/blob encryption no longer imports the key per operation**`AesGcmEncryption` (used by `DatabaseFile`, `PageEncryption` and the single-file provider, all of which hold one instance for a long time) constructed a fresh `AesGcm` and drew an OS-CSPRNG nonce on every page or blob operation. It now caches one cipher per instance and builds nonces as `[random prefix(8)][operation counter(4)]` with the same uniqueness argument as `CryptoService` (no repeat within an instance, 2^-64 across instances — stronger than a random nonce per call), and it throws rather than wrapping the invocation field. `Dispose` releases the cached cipher, so a reuse-after-dispose cannot silently encrypt with the pre-clear key. Pinned by `AesGcmEncryptionNonceTests`, including that one instance is safe under concurrent use (an assumption this project had never measured) and that page nonces are unique.
39+
3840

3941
- **Per-record encryption is no longer dominated by cipher setup** — `CryptoService` constructed a fresh `AesGcm` (key import: **0.69 µs of a 1.34 µs call**, 59%) and drew a nonce from the OS CSPRNG on *every* `Encrypt`/`Decrypt`, while the storage layer calls it once per record **and once per overflow-arena block** — so a row with three TEXT columns paid that setup three to four times, which turned out to be the entire at-rest write tax (measured per-row: +1.5 µs fixed-size = one call; +6.4 µs with TEXT = three to four calls). The cipher is now cached per key (keyed by the full key bytes with structural comparison — never a fingerprint, which could serve a wrong-but-similar key and silently corrupt data), nonces are `[random prefix(8)][operation counter(4)]` built from the counter that already guards GCM exhaustion, and `ResetEncryptionCounter` swaps the prefix *first* so even a reset without key rotation cannot replay a nonce. That construction is a **stronger** uniqueness guarantee than a random nonce per call, whose collision probability grew with the record count. Published, same-machine, four-arm fair-PK comparison: at-rest tax **INSERT 1.90× → 1.34×, UPDATE 1.62× → 1.31×, READ 1.51× → 1.34×, DELETE 1.10× → 1.08×** (INSERT **63,532 → 88,906 ops/s**, +40%; UPDATE +24%; READ +20%), and the gaps vs SQLite in the shipping default posture close to **UPDATE 1.8× → 1.6×, INSERT 2.8× → 2.1×, READ 1.33× → 1.08×**. The security-relevant properties are pinned by `CryptoServiceNonceTests` (nonce uniqueness + counter sequence, key-switch round-trips, thread safety under concurrency, prefix swap on reset, per-instance prefixes).
4042

docs/performance/INSERT_UPDATE_PERFORMANCE_PLAN.md

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,6 +1073,34 @@ the SQL path reach the contiguous fast path where one exists.
10731073
**Lower priority than UPDATE** (Direct-API DELETE is already 118.9–132.5K), but it falls out of
10741074
Phase 2's engine work almost for free, so sequence it there rather than as its own project.
10751075

1076+
**Measured (2026-09-14), and the answer is not the row copy.** The delete path had NO stage coverage at all
1077+
before this measurement — `WritePathProfiler` recorded nothing for a 10K-row DELETE because the fixed-width
1078+
fast path bypasses `DeleteRecordsCore` entirely, so the profiler is now wired into that fast path
1079+
(`RowLocate` around the contiguous span read, `IndexMaintenance` around the PK `DeleteBulk` + every loaded
1080+
hash index, `EngineWrite` around the tombstone/buffer step). With it, on the acceptance DELETE shape (10K
1081+
deletes by PK, 50K rows, one batch transaction):
1082+
1083+
| arm | ops/s | wall µs/row | in stages | uninstrumented |
1084+
|---|---:|---:|---:|---:|
1085+
| plaintext, with `idx_docs_name` | 103,495 | 9.66 | 4.29 | 5.37 |
1086+
| at-rest, with `idx_docs_name` | 100,209 | 9.98 | 5.15 | 4.83 |
1087+
| at-rest, **without** the index | 140,489 | 7.12 | 4.01 | 3.11 |
1088+
1089+
Stage shares at-rest: **`index-maintenance` 81.9%** (42.2 ms of 10K deletes), `row-locate` 16.3%,
1090+
`engine-write` (the tombstone) **1.8%**. Two conclusions that change the plan:
1091+
- **the at-rest tax on DELETE is zero (1.03×)** — unlike INSERT/UPDATE, there is no encryption cost to chase;
1092+
- **index maintenance is the cost (a 1.40× win from dropping one index, 82% of the instrumented time)**, and it
1093+
is paid *per loaded index per row*: the delete decodes each indexed column out of the fixed-width record —
1094+
which for a TEXT column means an overflow-arena read (and, at-rest, a decrypt) — purely to compute the hash
1095+
key. So the lever is **not** "make the delete cheaper" but "do the index removal in bulk / deferred", which
1096+
is what `DeferredIndexUpdater` exists for. The tombstone (the part the plan expected to matter) is noise.
1097+
1098+
**Instrumentation coverage (2026-09-14, §2).** Added: `Table.InsertBatch` (Validate — covering validation and
1099+
serialization — plus RowLocate around the batch PK probes; the path had none) and the fixed-width bulk-delete
1100+
fast path (RowLocate / IndexMaintenance / EngineWrite). Still uncovered, recorded honestly: the bulk **UPDATE**
1101+
path's index-maintenance block and the SQL/engine overhead outside the table (parse, plan cache, commit) —
1102+
the last column above is exactly that share, and it is why the totals in a stage report are not wall time.
1103+
10761104
---
10771105

10781106
## 8. Acceptance targets

src/SharpCoreDB/DataStructures/Table.CRUD.cs

Lines changed: 31 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ namespace SharpCoreDB.DataStructures;
1010
using SharpCoreDB.Services;
1111
using SharpCoreDB.Storage.Hybrid;
1212
using SharpCoreDB.Optimizations;
13+
using SharpCoreDB.Diagnostics;
1314

1415
/// <summary>
1516
/// CRUD operations for Table - Insert, Select, Update, Delete.
@@ -258,11 +259,18 @@ public long[] InsertBatch(List<Dictionary<string, object>> rows)
258259
if (this.isReadOnly) throw new InvalidOperationException(ReadOnlyInsertError);
259260

260261
// ✅ PHASE 1 OPTIMIZATION: Validate and serialize OUTSIDE lock
262+
// §2 instrumentation: this path had NO stage coverage before (measured 2026-09-14), which is why
263+
// attributing the at-rest INSERT tax needed ad-hoc probes instead of the profiler. This stamp covers
264+
// validation *and* serialization (the method does both).
265+
long validateStart = Diagnostics.WritePathProfiler.Stamp();
261266
var (serializedRows, validatedRows) = ValidateAndSerializeBatchOutsideLock(rows);
262-
267+
Diagnostics.WritePathProfiler.Add(Diagnostics.WritePathProfiler.Stage.Validate, validateStart);
268+
263269
// ✅ PHASE 2A FRIDAY: Batch validate primary keys BEFORE critical section
264270
// This improves cache locality and fails fast on duplicates
271+
long pkProbeStart = Diagnostics.WritePathProfiler.Stamp();
265272
ValidateBatchPrimaryKeysUpfront(validatedRows);
273+
Diagnostics.WritePathProfiler.Add(Diagnostics.WritePathProfiler.Stage.RowLocate, pkProbeStart);
266274

267275
// ✅ MINIMAL LOCK: Only for PK check, engine insert, and index updates
268276
this.rwLock.EnterWriteLock();
@@ -2675,8 +2683,10 @@ private bool HasExplicitNamedIndex(string column)
26752683
// Physical deletes (PageBased marks slots deleted; Columnar/AppendOnly are logical).
26762684
if (StorageMode == StorageMode.PageBased)
26772685
{
2686+
long pageDeleteStart = WritePathProfiler.Stamp();
26782687
foreach (var (storagePosition, _) in recordsToDelete)
26792688
engine.Delete(Name, storagePosition);
2689+
WritePathProfiler.Add(WritePathProfiler.Stage.EngineWrite, pageDeleteStart);
26802690
}
26812691
else if (StorageMode != StorageMode.Columnar)
26822692
{
@@ -2687,6 +2697,7 @@ private bool HasExplicitNamedIndex(string column)
26872697

26882698
// Primary-key B-tree cleanup: bulk-delete in descending key order (one pass through the
26892699
// rightmost leaf path instead of arbitrary per-row order → fewer separator promotions).
2700+
long indexStart = WritePathProfiler.Stamp();
26902701
if (this.PrimaryKeyIndex >= 0)
26912702
{
26922703
var pkCol = this.Columns[this.PrimaryKeyIndex];
@@ -2727,6 +2738,8 @@ private bool HasExplicitNamedIndex(string column)
27272738
kvp.Value.RemoveBatchKeys(keys, positions);
27282739
}
27292740

2741+
WritePathProfiler.Add(WritePathProfiler.Stage.IndexMaintenance, indexStart);
2742+
27302743
// Unloaded indexes rebuild lazily (columnar only - page-based indexes stay in sync).
27312744
if (StorageMode == StorageMode.Columnar)
27322745
{
@@ -2744,15 +2757,20 @@ private bool HasExplicitNamedIndex(string column)
27442757
// Transactional delete: buffer the physical offsets so the in-place marker is
27452758
// applied at COMMIT (rollback discards the buffer). Durable in O(delete) — the
27462759
// flush-time full-file rewrite is no longer needed for transactional deletes.
2760+
long bufferStart = WritePathProfiler.Stamp();
27472761
foreach (var position in positions.Where(static position => position >= 0))
27482762
{
27492763
this.storage.BufferTombstoneForCommit(DataFile, position);
27502764
}
2765+
2766+
WritePathProfiler.Add(WritePathProfiler.Stage.EngineWrite, bufferStart);
27512767
}
27522768
else
27532769
{
27542770
// Durable DELETE: physically mark the removed records so a reopen skips them.
2771+
long tombstoneStart = WritePathProfiler.Stamp();
27552772
TombstoneDeletedPositions(positions);
2773+
WritePathProfiler.Add(WritePathProfiler.Stage.EngineWrite, tombstoneStart);
27562774

27572775
// Legacy variable-length (non-fixed-width) columnar tables keep older stale versions
27582776
// of a key in the file (UPDATE appends a new version). Tombstoning only the newest
@@ -4042,7 +4060,11 @@ this.storage is null ||
40424060

40434061
// Resolve every target record's position through the PK B-tree and read + verify the whole
40444062
// contiguous span (one range read) — shared by the UPDATE and DELETE contiguous fast paths.
4063+
// §2 instrumentation: this fast path bypassed DeleteRecordsCore entirely (measured 2026-09-14: the
4064+
// profiler recorded nothing for a 10K-row DELETE while 100% of the wall time was spent here).
4065+
long deleteLocateStart = WritePathProfiler.Stamp();
40454066
var raw = TryReadContiguousFixedWidthRecords(keys, stride, layout, positions, encrypted);
4067+
WritePathProfiler.Add(WritePathProfiler.Stage.RowLocate, deleteLocateStart);
40464068
if (raw is null)
40474069
{
40484070
return false;
@@ -4052,6 +4074,7 @@ this.storage is null ||
40524074
// loaded hash-index entry, decoding only the indexed columns from the raw fixed-width records
40534075
// (no full-row deserialization). Variable values resolve through the overflow arena, mirroring
40544076
// the fixed-width codec used by the generic path.
4077+
long deleteIndexStart = WritePathProfiler.Stamp();
40554078
this.Index.DeleteBulk(keys);
40564079

40574080
var arena = GetOverflowArena();
@@ -4099,19 +4122,26 @@ this.storage is null ||
40994122
hashIdx.RemoveBatchKeys(decoded, positions);
41004123
}
41014124

4125+
WritePathProfiler.Add(WritePathProfiler.Stage.IndexMaintenance, deleteIndexStart);
4126+
41024127
if (this.storage is { IsInTransaction: true })
41034128
{
41044129
// Transactional delete: buffer the physical offsets so the in-place marker is applied
41054130
// at COMMIT (see DeleteRecordsCore — rollback discards the buffer).
4131+
long deleteBufferStart = WritePathProfiler.Stamp();
41064132
foreach (var position in positions.Where(static position => position >= 0))
41074133
{
41084134
this.storage.BufferTombstoneForCommit(DataFile, position);
41094135
}
4136+
4137+
WritePathProfiler.Add(WritePathProfiler.Stage.EngineWrite, deleteBufferStart);
41104138
}
41114139
else
41124140
{
41134141
// Durable DELETE: physically mark the removed records so a reopen skips them.
4142+
long deleteTombstoneStart = WritePathProfiler.Stamp();
41144143
TombstoneDeletedPositions(positions);
4144+
WritePathProfiler.Add(WritePathProfiler.Stage.EngineWrite, deleteTombstoneStart);
41154145
}
41164146

41174147
Interlocked.Add(ref _cachedRowCount, -count);

src/SharpCoreDB/Services/AesGcmEncryption.cs

Lines changed: 84 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,75 @@ public sealed class AesGcmEncryption(byte[] key, bool disableEncrypt = false) :
2222
private readonly byte[] _key = disableEncrypt ? [] : [.. key];
2323
private readonly ArrayPool<byte> _pool = ArrayPool<byte>.Shared;
2424

25+
/// <summary>
26+
/// PERF: one cipher per INSTANCE instead of one per call. Constructing <see cref="AesGcm"/> imports the
27+
/// key — measured **0.69 µs of a 1.34 µs Encrypt call, 59%** on this machine — and the holders of this
28+
/// class (<c>DatabaseFile</c>, <c>PageEncryption</c> and the single-file provider) keep one instance for
29+
/// a long time, so the cache is both safe and effective. GCM keeps no per-instance mutable state, so
30+
/// concurrent use of the shared cipher is safe — asserted by a concurrency test, not assumed.
31+
/// </summary>
32+
private AesGcm? _cipher;
33+
34+
/// <summary>
35+
/// SECURITY: the 64-bit random fixed field of this instance's GCM nonces; every nonce is
36+
/// <c>[prefix(8)][counter(4)]</c> with <see cref="_operationCount"/> as the invocation field, capped at
37+
/// <see cref="Constants.CryptoConstants.MAX_GCM_OPERATIONS"/> so it cannot wrap. Two instances with the
38+
/// same key collide only if their 64-bit prefixes do, independent of how many records they write — a
39+
/// random nonce per call had a collision probability that grew with the operation count instead.
40+
/// </summary>
41+
private byte[] _noncePrefix = CreateNoncePrefix();
42+
43+
/// <summary>Invocation field of this instance's nonces; see <see cref="_noncePrefix"/>.</summary>
44+
private long _operationCount;
45+
46+
/// <summary>Bytes of the nonce reserved for the per-instance random fixed field.</summary>
47+
private const int NoncePrefixSize = 8;
48+
49+
/// <summary>Draws a fresh 64-bit nonce prefix from the OS CSPRNG.</summary>
50+
private static byte[] CreateNoncePrefix()
51+
{
52+
var prefix = new byte[NoncePrefixSize];
53+
RandomNumberGenerator.Fill(prefix);
54+
return prefix;
55+
}
56+
57+
/// <summary>The instance's cipher, created once (first use) and disposed with the instance.</summary>
58+
private AesGcm Cipher
59+
{
60+
[MethodImpl(MethodImplOptions.AggressiveInlining)]
61+
get => Volatile.Read(ref _cipher) ?? CreateCipher();
62+
}
63+
64+
private AesGcm CreateCipher()
65+
{
66+
var created = new AesGcm(_key, TagSize);
67+
var existing = Interlocked.CompareExchange(ref _cipher, created, null);
68+
if (existing is not null)
69+
{
70+
created.Dispose(); // another thread got there first
71+
return existing;
72+
}
73+
74+
return created;
75+
}
76+
77+
/// <summary>
78+
/// Writes <c>[prefix(8)][counter(4)]</c> into <paramref name="nonce"/>. Throws rather than wrapping the
79+
/// invocation field: a repeated GCM nonce under one key leaks the keystream.
80+
/// </summary>
81+
private void BuildNonce(Span<byte> nonce)
82+
{
83+
long count = Interlocked.Increment(ref _operationCount);
84+
if (count >= Constants.CryptoConstants.MAX_GCM_OPERATIONS)
85+
{
86+
throw new InvalidOperationException(
87+
$"Encryption limit reached ({count} operations). Key rotation required to prevent GCM nonce reuse.");
88+
}
89+
90+
Volatile.Read(ref _noncePrefix).CopyTo(nonce);
91+
System.Buffers.Binary.BinaryPrimitives.WriteUInt32LittleEndian(nonce[NoncePrefixSize..], (uint)count);
92+
}
93+
2594
// Size constants for AES-GCM
2695
private const int NonceSize = 12; // AesGcm.NonceByteSizes.MaxSize = 12
2796
private const int TagSize = 16; // AesGcm.TagByteSizes.MaxSize = 16
@@ -154,12 +223,12 @@ public byte[] Encrypt(byte[] data, ReadOnlySpan<byte> associatedData)
154223
if (disableEncrypt)
155224
return data;
156225

157-
using var aes = new AesGcm(_key, TagSize);
226+
var aes = Cipher;
158227

159228
Span<byte> nonce = stackalloc byte[NonceSize];
160229
Span<byte> tag = stackalloc byte[TagSize];
161230

162-
RandomNumberGenerator.Fill(nonce);
231+
BuildNonce(nonce);
163232

164233
byte[]? cipherArray = null;
165234
try
@@ -213,7 +282,7 @@ public byte[] Decrypt(byte[] encryptedData, ReadOnlySpan<byte> associatedData)
213282
if (cipherLength < 0)
214283
throw new ArgumentException("Invalid encrypted data length", nameof(encryptedData));
215284

216-
using var aes = new AesGcm(_key, TagSize);
285+
var aes = Cipher;
217286

218287
ReadOnlySpan<byte> nonce = encryptedData.AsSpan(0, NonceSize);
219288
ReadOnlySpan<byte> cipher = encryptedData.AsSpan(NonceSize, cipherLength);
@@ -257,12 +326,12 @@ public int Encrypt(ReadOnlySpan<byte> data, Span<byte> output, ReadOnlySpan<byte
257326
if (output.Length < totalSize)
258327
throw new ArgumentException("Output buffer too small", nameof(output));
259328

260-
using var aes = new AesGcm(_key, TagSize);
329+
var aes = Cipher;
261330

262331
Span<byte> nonce = stackalloc byte[NonceSize];
263332
Span<byte> tag = stackalloc byte[TagSize];
264333

265-
RandomNumberGenerator.Fill(nonce);
334+
BuildNonce(nonce);
266335

267336
byte[]? cipherArray = null;
268337
try
@@ -338,7 +407,7 @@ public int Decrypt(ReadOnlySpan<byte> encryptedData, Span<byte> output, ReadOnly
338407
if (output.Length < cipherLength)
339408
throw new ArgumentException("Output buffer too small", nameof(output));
340409

341-
using var aes = new AesGcm(_key, TagSize);
410+
var aes = Cipher;
342411

343412
var nonce = encryptedData[..NonceSize];
344413
var cipher = encryptedData.Slice(NonceSize, cipherLength);
@@ -375,12 +444,12 @@ public void EncryptPage(Span<byte> page, ReadOnlySpan<byte> associatedData)
375444
if (dataSize <= 0)
376445
throw new ArgumentException("Page buffer too small for encryption overhead", nameof(page));
377446

378-
using var aes = new AesGcm(_key, TagSize);
447+
var aes = Cipher;
379448

380449
Span<byte> nonce = stackalloc byte[NonceSize];
381450
Span<byte> tag = stackalloc byte[TagSize];
382451

383-
RandomNumberGenerator.Fill(nonce);
452+
BuildNonce(nonce);
384453

385454
byte[]? tempArray = null;
386455
try
@@ -432,7 +501,7 @@ public void DecryptPage(Span<byte> page, ReadOnlySpan<byte> associatedData)
432501
if (cipherLength <= 0)
433502
throw new ArgumentException("Page buffer too small for decryption", nameof(page));
434503

435-
using var aes = new AesGcm(_key, TagSize);
504+
var aes = Cipher;
436505

437506
var nonce = page[..NonceSize];
438507
var cipher = page.Slice(NonceSize, cipherLength);
@@ -461,7 +530,13 @@ public void DecryptPage(Span<byte> page, ReadOnlySpan<byte> associatedData)
461530
[MethodImpl(MethodImplOptions.AggressiveInlining)]
462531
public void Dispose()
463532
{
533+
// Dispose the cached cipher as well: leaving it alive would let a reuse-after-dispose silently
534+
// encrypt with the pre-clear key, which is exactly the trap the key clearing below exists to avoid.
535+
Interlocked.Exchange(ref _cipher, null)?.Dispose();
536+
464537
if (_key.Length > 0)
465538
Array.Clear(_key);
539+
540+
Array.Clear(_noncePrefix);
466541
}
467542
}

0 commit comments

Comments
 (0)