Skip to content

Commit c0dc2af

Browse files
author
MPCoreDeveloper
committed
perf(index): remove quadratic duplicate-key hash removal on batch DELETE (P5)
HashIndex.RemoveBatchKeys/RemoveBatch removed every position from a key's List with one O(list) List.Remove shift per duplicate: O(m·n) for a key holding n rows with m duplicate-key deletions in a single batch. Batch removal now keeps the direct allocation-free path for single-row keys (the common unique-key case, verified benchmark-neutral at ~80K legacy / ~133K fixed-width ops/s on --pk) and defers duplicate-key positions into a per-key set that is applied with one O(list) compaction per key. Regression tests (HashIndexDuplicateKeyBatchDeleteTests) cover full and partial duplicate-group deletes on both index backends (managed List + unsafe native) including a reopen; full suite 1764 tests, 0 failed.
1 parent 8985a08 commit c0dc2af

4 files changed

Lines changed: 215 additions & 37 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+
- **Duplicate-key hash-index removal is no longer quadratic (P5)** - `HashIndex.RemoveBatchKeys`/
31+
`RemoveBatch` previously removed every position from a key's list with one O(list) shift per
32+
duplicate, i.e. O(m·n) for a key holding n rows with m duplicate-key deletions in one batch.
33+
Batch removal now keeps the direct allocation-free path for single-row keys and defers
34+
duplicate-key positions into a per-key set that is applied in one O(list) compaction. New
35+
regression tests cover full and partial duplicate-group deletes on both index backends
36+
(managed `List` + unsafe native backend) including a reopen; full suite 1764 tests, 0 failed.
3037
- **Commit-time tombstones now batch the marker writes (C5)** - the DELETE commit phase read the
3138
whole file once (#373) but still applied one 4-byte negative-prefix marker per row
3239
(one pwrite each). `TombstoneRecords` now patches every marker into the in-memory snapshot first

‎docs/performance/EXECUTION_PLAN_UPDATE_DELETE.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
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 |
1414
| #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 |
15+
| #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 |
16+
| 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 |
1617

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

‎src/SharpCoreDB/DataStructures/HashIndex.cs‎

Lines changed: 90 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -240,40 +240,15 @@ public void RemoveBatch(List<Dictionary<string, object>> rows, long[] positions)
240240
{
241241
if (rows.Count == 0) return;
242242

243-
_lock.EnterWriteLock();
244-
try
245-
{
246-
for (int i = 0; i < rows.Count; i++)
247-
{
248-
if (!rows[i].TryGetValue(_columnName, out var key) || key is null)
249-
continue;
250-
251-
var normalizedKey = NormalizeKey(key);
252-
253-
if (_useUnsafeEqualityIndex)
254-
{
255-
var keyBytes = BuildUnsafeKey(normalizedKey);
256-
if (_unsafeIndex.Remove(keyBytes, positions[i]))
257-
{
258-
_unsafeTotalRows--;
259-
}
260-
continue;
261-
}
262-
263-
if (_index.TryGetValue(normalizedKey, out var list))
264-
{
265-
list.Remove(positions[i]);
266-
if (list.Count == 0)
267-
{
268-
_index.Remove(normalizedKey);
269-
}
270-
}
271-
}
272-
}
273-
finally
243+
// Extract each row's indexed key once (null when the column is absent) and delegate to
244+
// the key-based batch removal, which compacts duplicate-key lists in a single pass.
245+
var keys = new object?[rows.Count];
246+
for (int i = 0; i < rows.Count; i++)
274247
{
275-
_lock.ExitWriteLock();
248+
rows[i].TryGetValue(_columnName, out keys[i]);
276249
}
250+
251+
RemoveBatchKeys(keys, positions);
277252
}
278253

279254
/// <summary>
@@ -292,11 +267,21 @@ internal void RemoveBatchKeys(object?[] keys, long[] positions)
292267
_lock.EnterWriteLock();
293268
try
294269
{
270+
// Deferred duplicate-key removals. A key whose position list has more than one entry and
271+
// appears more than once in the batch previously cost one O(list) List.Remove shift per
272+
// duplicate — O(m·n) for a key with n rows and m duplicate-key deletions in the batch.
273+
// Instead the first occurrence is removed directly and later occurrences are collected,
274+
// after which the list is compacted in one O(list) pass. Single-row keys (the common
275+
// case for unique-ish indexed values) stay on the allocation-free direct path.
276+
Dictionary<object, HashSet<long>>? deferred = null;
277+
295278
for (int i = 0; i < keys.Length; i++)
296279
{
297280
var key = keys[i];
298281
if (key is null)
282+
{
299283
continue;
284+
}
300285

301286
var normalizedKey = NormalizeKey(key);
302287

@@ -307,15 +292,54 @@ internal void RemoveBatchKeys(object?[] keys, long[] positions)
307292
{
308293
_unsafeTotalRows--;
309294
}
295+
310296
continue;
311297
}
312298

313-
if (_index.TryGetValue(normalizedKey, out var list))
299+
if (!_index.TryGetValue(normalizedKey, out var list))
314300
{
315-
list.Remove(positions[i]);
316-
if (list.Count == 0)
301+
continue;
302+
}
303+
304+
if (list.Count > 1)
305+
{
306+
deferred ??= new Dictionary<object, HashSet<long>>(_comparer);
307+
if (deferred.TryGetValue(normalizedKey, out var dupSet))
308+
{
309+
(dupSet ??= new HashSet<long>()).Add(positions[i]);
310+
deferred[normalizedKey] = dupSet;
311+
}
312+
else
313+
{
314+
list.Remove(positions[i]);
315+
if (list.Count == 0)
316+
{
317+
_index.Remove(normalizedKey);
318+
}
319+
else
320+
{
321+
deferred.Add(normalizedKey, null);
322+
}
323+
}
324+
325+
continue;
326+
}
327+
328+
// Single-row key: direct removal, no duplicate tracking needed.
329+
list.Remove(positions[i]);
330+
if (list.Count == 0)
331+
{
332+
_index.Remove(normalizedKey);
333+
}
334+
}
335+
336+
if (deferred != null)
337+
{
338+
foreach (var kvp in deferred)
339+
{
340+
if (kvp.Value is not null)
317341
{
318-
_index.Remove(normalizedKey);
342+
CompactPositionList(kvp.Key, kvp.Value);
319343
}
320344
}
321345
}
@@ -326,6 +350,36 @@ internal void RemoveBatchKeys(object?[] keys, long[] positions)
326350
}
327351
}
328352

353+
/// <summary>
354+
/// Removes every position in <paramref name="removed"/> from the key's position list with one
355+
/// O(list) compaction pass (value-based membership, so ordering is irrelevant).
356+
/// </summary>
357+
private void CompactPositionList(object key, HashSet<long> removed)
358+
{
359+
if (!_index.TryGetValue(key, out var list))
360+
{
361+
return;
362+
}
363+
364+
int write = 0;
365+
for (int read = 0; read < list.Count; read++)
366+
{
367+
if (!removed.Contains(list[read]))
368+
{
369+
list[write++] = list[read];
370+
}
371+
}
372+
373+
if (write == 0)
374+
{
375+
_index.Remove(key);
376+
}
377+
else if (write < list.Count)
378+
{
379+
list.RemoveRange(write, list.Count - write);
380+
}
381+
}
382+
329383
/// <summary>
330384
/// Key-based overload of <see cref="AddBatch"/>: callers that already know each indexed key
331385
/// (e.g. an in-place UPDATE re-point) add all rows with one lock acquisition per index.
Lines changed: 116 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,116 @@
1+
// <copyright file="HashIndexDuplicateKeyBatchDeleteTests.cs" company="MPCoreDeveloper">
2+
// Copyright (c) 2026 MPCoreDeveloper. All rights reserved.
3+
// Licensed under the MIT License. See LICENSE file in the project root for full license information.
4+
// </copyright>
5+
namespace SharpCoreDB.Tests;
6+
7+
using Microsoft.Extensions.DependencyInjection;
8+
using SharpCoreDB.Interfaces;
9+
using System;
10+
using System.Collections.Generic;
11+
using System.Globalization;
12+
using System.IO;
13+
using Xunit;
14+
15+
/// <summary>
16+
/// Regression coverage for duplicate-key hash-index removal: batch-DELETEs that hit a non-unique
17+
/// indexed column with large position groups exercise HashIndex.RemoveBatchKeys's deferred
18+
/// duplicate-key compaction (previously one O(list) List.Remove shift per duplicate).
19+
/// </summary>
20+
public sealed class HashIndexDuplicateKeyBatchDeleteTests : IDisposable
21+
{
22+
private readonly DatabaseFactory _factory;
23+
private readonly string _dirPath;
24+
25+
public HashIndexDuplicateKeyBatchDeleteTests()
26+
{
27+
var services = new ServiceCollection();
28+
services.AddSharpCoreDB();
29+
_factory = services.BuildServiceProvider().GetRequiredService<DatabaseFactory>();
30+
_dirPath = Path.Combine(Path.GetTempPath(), $"SCDB_HashDup_{Guid.NewGuid():N}");
31+
}
32+
33+
public void Dispose()
34+
{
35+
try { if (Directory.Exists(_dirPath)) Directory.Delete(_dirPath, true); } catch { }
36+
}
37+
38+
[Theory]
39+
[InlineData(false)]
40+
[InlineData(true)]
41+
public void BatchDelete_DuplicatedNameGroups_RemovesOnlyThoseKeys_AcrossReopen(bool useUnsafeEqualityIndex)
42+
{
43+
IDatabase? db = _factory.Create(_dirPath, "pw", isReadOnly: false,
44+
config: new DatabaseConfig
45+
{
46+
NoEncryptMode = true,
47+
AutoFixedWidthRecords = false,
48+
EnableUnsafeEqualityIndex = useUnsafeEqualityIndex,
49+
});
50+
try
51+
{
52+
db.ExecuteSQL("CREATE TABLE docs (id INTEGER PRIMARY KEY, name TEXT, score REAL)");
53+
db.ExecuteSQL("CREATE INDEX idx_docs_name ON docs(name)");
54+
55+
// 2000 rows, 200 rows per duplicated name group (dup0..dup9).
56+
var stmts = new List<string>(2000);
57+
for (int i = 1; i <= 2000; i++)
58+
{
59+
stmts.Add(string.Format(CultureInfo.InvariantCulture,
60+
"INSERT INTO docs VALUES ({0}, 'dup{1}', {2})", i, i % 10, i * 0.5));
61+
}
62+
63+
db.ExecuteBatchSQL(stmts);
64+
db.Flush();
65+
66+
Assert.Equal(2000, db.ExecuteQuery("SELECT id FROM docs").Count);
67+
68+
// Delete three full duplicate groups in one batch: dup1, dup5, dup9 (600 rows).
69+
db.ExecuteBatchSQL(
70+
[
71+
"DELETE FROM docs WHERE name = 'dup1'",
72+
"DELETE FROM docs WHERE name = 'dup5'",
73+
"DELETE FROM docs WHERE name = 'dup9'",
74+
]);
75+
db.Flush();
76+
77+
Assert.Equal(1400, db.ExecuteQuery("SELECT id FROM docs").Count);
78+
Assert.Empty(db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup1'"));
79+
Assert.Empty(db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup5'"));
80+
Assert.Empty(db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup9'"));
81+
Assert.Equal(200, db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup2'").Count);
82+
Assert.Equal(200, db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup0'").Count);
83+
84+
// Partial group delete: remove half of dup2 by id, keeping the rest reachable.
85+
var partial = new List<string>(100);
86+
for (int i = 2; i <= 2000; i += 20)
87+
{
88+
partial.Add($"DELETE FROM docs WHERE id = {i}");
89+
}
90+
91+
db.ExecuteBatchSQL(partial);
92+
db.Flush();
93+
Assert.Equal(100, db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup2'").Count);
94+
}
95+
finally { (db as IDisposable)?.Dispose(); }
96+
97+
// Reopen: tombstoned rows stay gone, live duplicated-key groups stay fully reachable.
98+
db = _factory.Create(_dirPath, "pw", isReadOnly: false,
99+
config: new DatabaseConfig
100+
{
101+
NoEncryptMode = true,
102+
AutoFixedWidthRecords = false,
103+
EnableUnsafeEqualityIndex = useUnsafeEqualityIndex,
104+
});
105+
try
106+
{
107+
Assert.Equal(1300, db.ExecuteQuery("SELECT id FROM docs").Count);
108+
Assert.Empty(db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup1'"));
109+
Assert.Empty(db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup5'"));
110+
Assert.Empty(db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup9'"));
111+
Assert.Equal(100, db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup2'").Count);
112+
Assert.Equal(200, db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup0'").Count);
113+
}
114+
finally { (db as IDisposable)?.Dispose(); }
115+
}
116+
}

0 commit comments

Comments
 (0)