Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`).
- **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.
Batch removal now keeps the direct allocation-free path for single-row keys and defers
duplicate-key positions into a per-key set that is applied in one O(list) compaction. New
regression tests cover full and partial duplicate-group deletes on both index backends
(managed `List` + unsafe native backend) including a reopen; full suite 1764 tests, 0 failed.
- **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
Expand Down
3 changes: 2 additions & 1 deletion docs/performance/EXECUTION_PLAN_UPDATE_DELETE.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
| #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 |
| #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 |
| #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 |

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

Expand Down
126 changes: 90 additions & 36 deletions src/SharpCoreDB/DataStructures/HashIndex.cs
Original file line number Diff line number Diff line change
Expand Up @@ -240,40 +240,15 @@
{
if (rows.Count == 0) return;

_lock.EnterWriteLock();
try
{
for (int i = 0; i < rows.Count; i++)
{
if (!rows[i].TryGetValue(_columnName, out var key) || key is null)
continue;

var normalizedKey = NormalizeKey(key);

if (_useUnsafeEqualityIndex)
{
var keyBytes = BuildUnsafeKey(normalizedKey);
if (_unsafeIndex.Remove(keyBytes, positions[i]))
{
_unsafeTotalRows--;
}
continue;
}

if (_index.TryGetValue(normalizedKey, out var list))
{
list.Remove(positions[i]);
if (list.Count == 0)
{
_index.Remove(normalizedKey);
}
}
}
}
finally
// Extract each row's indexed key once (null when the column is absent) and delegate to
// the key-based batch removal, which compacts duplicate-key lists in a single pass.
var keys = new object?[rows.Count];
for (int i = 0; i < rows.Count; i++)
{
_lock.ExitWriteLock();
rows[i].TryGetValue(_columnName, out keys[i]);
}

RemoveBatchKeys(keys, positions);
}

/// <summary>
Expand All @@ -285,18 +260,28 @@
/// <param name="keys">The indexed key values (null entries are skipped).</param>
/// <param name="positions">Corresponding storage positions.</param>
[MethodImpl(MethodImplOptions.AggressiveOptimization)]
internal void RemoveBatchKeys(object?[] keys, long[] positions)

Check failure on line 263 in src/SharpCoreDB/DataStructures/HashIndex.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this method to reduce its Cognitive Complexity from 30 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=MPCoreDeveloper_SharpCoreDB&issues=AaBrZlBQhda6KO8M_ys4&open=AaBrZlBQhda6KO8M_ys4&pullRequest=378
{
if (keys.Length == 0) return;

_lock.EnterWriteLock();
try
{
// Deferred duplicate-key removals. A key whose position list has more than one entry and
// appears more than once in the batch previously cost one O(list) List.Remove shift per
// duplicate — O(m·n) for a key with n rows and m duplicate-key deletions in the batch.
// Instead the first occurrence is removed directly and later occurrences are collected,
// after which the list is compacted in one O(list) pass. Single-row keys (the common
// case for unique-ish indexed values) stay on the allocation-free direct path.
Dictionary<object, HashSet<long>>? deferred = null;

for (int i = 0; i < keys.Length; i++)
{
var key = keys[i];
if (key is null)
{
continue;
}

var normalizedKey = NormalizeKey(key);

Expand All @@ -307,15 +292,54 @@
{
_unsafeTotalRows--;
}

continue;
}

if (_index.TryGetValue(normalizedKey, out var list))
if (!_index.TryGetValue(normalizedKey, out var list))
{
list.Remove(positions[i]);
if (list.Count == 0)
continue;
}

if (list.Count > 1)
{
deferred ??= new Dictionary<object, HashSet<long>>(_comparer);
if (deferred.TryGetValue(normalizedKey, out var dupSet))
{
(dupSet ??= new HashSet<long>()).Add(positions[i]);
deferred[normalizedKey] = dupSet;
}
else
{
list.Remove(positions[i]);
if (list.Count == 0)
{
_index.Remove(normalizedKey);
}
else
{
deferred.Add(normalizedKey, null);
}
}

continue;
}

// Single-row key: direct removal, no duplicate tracking needed.
list.Remove(positions[i]);
if (list.Count == 0)
{
_index.Remove(normalizedKey);
}
}

if (deferred != null)
{
foreach (var kvp in deferred)

Check warning on line 338 in src/SharpCoreDB/DataStructures/HashIndex.cs

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Loops should be simplified using the "Where" LINQ method

See more on https://sonarcloud.io/project/issues?id=MPCoreDeveloper_SharpCoreDB&issues=AaBrZlBQhda6KO8M_ys5&open=AaBrZlBQhda6KO8M_ys5&pullRequest=378
{
if (kvp.Value is not null)
{
_index.Remove(normalizedKey);
CompactPositionList(kvp.Key, kvp.Value);
}
}
}
Expand All @@ -326,6 +350,36 @@
}
}

/// <summary>
/// Removes every position in <paramref name="removed"/> from the key's position list with one
/// O(list) compaction pass (value-based membership, so ordering is irrelevant).
/// </summary>
private void CompactPositionList(object key, HashSet<long> removed)
{
if (!_index.TryGetValue(key, out var list))
{
return;
}

int write = 0;
for (int read = 0; read < list.Count; read++)
{
if (!removed.Contains(list[read]))
{
list[write++] = list[read];
}
}

if (write == 0)
{
_index.Remove(key);
}
else if (write < list.Count)
{
list.RemoveRange(write, list.Count - write);
}
}

/// <summary>
/// Key-based overload of <see cref="AddBatch"/>: callers that already know each indexed key
/// (e.g. an in-place UPDATE re-point) add all rows with one lock acquisition per index.
Expand Down
116 changes: 116 additions & 0 deletions tests/SharpCoreDB.Tests/HashIndexDuplicateKeyBatchDeleteTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
// <copyright file="HashIndexDuplicateKeyBatchDeleteTests.cs" company="MPCoreDeveloper">
// Copyright (c) 2026 MPCoreDeveloper. All rights reserved.
// Licensed under the MIT License. See LICENSE file in the project root for full license information.
// </copyright>
namespace SharpCoreDB.Tests;

using Microsoft.Extensions.DependencyInjection;
using SharpCoreDB.Interfaces;
using System;
using System.Collections.Generic;
using System.Globalization;
using System.IO;
using Xunit;

/// <summary>
/// Regression coverage for duplicate-key hash-index removal: batch-DELETEs that hit a non-unique
/// indexed column with large position groups exercise HashIndex.RemoveBatchKeys's deferred
/// duplicate-key compaction (previously one O(list) List.Remove shift per duplicate).
/// </summary>
public sealed class HashIndexDuplicateKeyBatchDeleteTests : IDisposable
{
private readonly DatabaseFactory _factory;
private readonly string _dirPath;

public HashIndexDuplicateKeyBatchDeleteTests()
{
var services = new ServiceCollection();
services.AddSharpCoreDB();
_factory = services.BuildServiceProvider().GetRequiredService<DatabaseFactory>();
_dirPath = Path.Combine(Path.GetTempPath(), $"SCDB_HashDup_{Guid.NewGuid():N}");
}

public void Dispose()
{
try { if (Directory.Exists(_dirPath)) Directory.Delete(_dirPath, true); } catch { }
}

[Theory]
[InlineData(false)]
[InlineData(true)]
public void BatchDelete_DuplicatedNameGroups_RemovesOnlyThoseKeys_AcrossReopen(bool useUnsafeEqualityIndex)
{
IDatabase? db = _factory.Create(_dirPath, "pw", isReadOnly: false,
config: new DatabaseConfig
{
NoEncryptMode = true,
AutoFixedWidthRecords = false,
EnableUnsafeEqualityIndex = useUnsafeEqualityIndex,
});
try
{
db.ExecuteSQL("CREATE TABLE docs (id INTEGER PRIMARY KEY, name TEXT, score REAL)");
db.ExecuteSQL("CREATE INDEX idx_docs_name ON docs(name)");

// 2000 rows, 200 rows per duplicated name group (dup0..dup9).
var stmts = new List<string>(2000);
for (int i = 1; i <= 2000; i++)
{
stmts.Add(string.Format(CultureInfo.InvariantCulture,
"INSERT INTO docs VALUES ({0}, 'dup{1}', {2})", i, i % 10, i * 0.5));
}

db.ExecuteBatchSQL(stmts);
db.Flush();

Assert.Equal(2000, db.ExecuteQuery("SELECT id FROM docs").Count);

// Delete three full duplicate groups in one batch: dup1, dup5, dup9 (600 rows).
db.ExecuteBatchSQL(
[
"DELETE FROM docs WHERE name = 'dup1'",
"DELETE FROM docs WHERE name = 'dup5'",
"DELETE FROM docs WHERE name = 'dup9'",
]);
db.Flush();

Assert.Equal(1400, db.ExecuteQuery("SELECT id FROM docs").Count);
Assert.Empty(db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup1'"));
Assert.Empty(db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup5'"));
Assert.Empty(db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup9'"));
Assert.Equal(200, db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup2'").Count);
Assert.Equal(200, db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup0'").Count);

// Partial group delete: remove half of dup2 by id, keeping the rest reachable.
var partial = new List<string>(100);
for (int i = 2; i <= 2000; i += 20)
{
partial.Add($"DELETE FROM docs WHERE id = {i}");
}

db.ExecuteBatchSQL(partial);
db.Flush();
Assert.Equal(100, db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup2'").Count);
}
finally { (db as IDisposable)?.Dispose(); }

// Reopen: tombstoned rows stay gone, live duplicated-key groups stay fully reachable.
db = _factory.Create(_dirPath, "pw", isReadOnly: false,
config: new DatabaseConfig
{
NoEncryptMode = true,
AutoFixedWidthRecords = false,
EnableUnsafeEqualityIndex = useUnsafeEqualityIndex,
});
try
{
Assert.Equal(1300, db.ExecuteQuery("SELECT id FROM docs").Count);
Assert.Empty(db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup1'"));
Assert.Empty(db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup5'"));
Assert.Empty(db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup9'"));
Assert.Equal(100, db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup2'").Count);
Assert.Equal(200, db.ExecuteQuery("SELECT id FROM docs WHERE name = 'dup0'").Count);
}
finally { (db as IDisposable)?.Dispose(); }
}
}
Loading