Skip to content
Merged
146 changes: 130 additions & 16 deletions src/Nethermind/Nethermind.Db.Rocks/ColumnsDb.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.CompilerServices;
using System.Threading;
using FastEnumUtility;
using Nethermind.Core;
using Nethermind.Db.Rocks.Config;
Expand All @@ -17,6 +19,11 @@ public class ColumnsDb<T> : DbOnTheRocks, IColumnsDb<T> where T : struct, Enum
{
private readonly IDictionary<T, ColumnDb> _columnDbs = new Dictionary<T, ColumnDb>();

// Cached for ColumnDbSnapshot to avoid per-snapshot recomputation.
// Initialized once on first CreateSnapshot call; both fields are idempotent (same result from any thread).
private volatile T[]? _cachedColumnKeys;
private volatile int _cachedMaxOrdinal = -1;

public ColumnsDb(string basePath, DbSettings settings, IDbConfig dbConfig, IRocksDbConfigFactory rocksDbConfigFactory, ILogManager logManager, IReadOnlyList<T> keys, IntPtr? sharedCache = null)
: this(basePath, settings, dbConfig, rocksDbConfigFactory, logManager, ResolveKeys(keys), sharedCache)
{
Expand Down Expand Up @@ -158,31 +165,138 @@ IColumnDbSnapshot<T> IColumnsDb<T>.CreateSnapshot()
return new ColumnDbSnapshot(this, snapshot);
}

private class ColumnDbSnapshot(
ColumnsDb<T> columnsDb,
Snapshot snapshot
) : IColumnDbSnapshot<T>
private class ColumnDbSnapshot : IColumnDbSnapshot<T>
{
private readonly Dictionary<T, IReadOnlyKeyValueStore> _columnDbs = columnsDb.ColumnKeys.ToDictionary(k => k, k =>
(IReadOnlyKeyValueStore)new RocksDbReader(
columnsDb,
() =>
private readonly Snapshot _snapshot;
private readonly ReadOptions _sharedReadOptions;
private readonly ReadOptions _sharedCacheMissReadOptions;
private readonly RocksDbSharp.Native _rocksDbNative;
private int _disposed;

// Use a flat array indexed by enum ordinal instead of Dictionary<T, IReadOnlyKeyValueStore>.
// This eliminates the dictionary + backing array allocation per snapshot.
private readonly RocksDbReader[] _readers;

public ColumnDbSnapshot(ColumnsDb<T> columnsDb, Snapshot snapshot)
{
_snapshot = snapshot;
_rocksDbNative = columnsDb._rocksDbNative;

// Create two shared ReadOptions for all column readers instead of 2 per reader.
// ReadOptions in RocksDbSharp has a finalizer but no IDisposable — creating many
// short-lived instances causes Gen1/Gen2 GC pressure from finalizer queue buildup.
_sharedReadOptions = CreateReadOptions(columnsDb, snapshot);
_sharedCacheMissReadOptions = CreateReadOptions(columnsDb, snapshot);
_sharedCacheMissReadOptions.SetFillCache(false);

// Single shared delegate for GetViewBetween — avoids per-reader closure allocation.
// Note: each GetViewBetween call still creates a new ReadOptions with a finalizer;
// that is pre-existing behavior not addressed by this PR.
Func<ReadOptions> readOptionsFactory = () => CreateReadOptions(columnsDb, snapshot);
T[] keys = CreateKeyCache(columnsDb);
GetCachedMaxOrdinal(columnsDb, keys);
_readers = CreateReaders();

static ReadOptions CreateReadOptions(ColumnsDb<T> columnsDb, Snapshot snapshot)
{
ReadOptions options = new ReadOptions();
options.SetVerifyChecksums(columnsDb.VerifyChecksum);
options.SetSnapshot(snapshot);
return options;
}

// Cache column keys and max ordinal on the parent ColumnsDb to avoid per-snapshot
// recomputation. The race is benign (both threads compute identical results) and
// volatile ensures visibility across cores.
static T[] CreateKeyCache(ColumnsDb<T> columnsDb)
{
T[]? keys = columnsDb._cachedColumnKeys;
if (keys is null)
{
IDictionary<T, ColumnDb> columnDbs = columnsDb._columnDbs;
keys = new T[columnDbs.Count];
int idx = 0;
foreach (T key in columnDbs.Keys)
{
keys[idx++] = key;
}

columnsDb._cachedColumnKeys = keys;
}

return keys;
}
static void GetCachedMaxOrdinal(ColumnsDb<T> columnsDb, T[] keys)
{
if (columnsDb._cachedMaxOrdinal >= 0) return;

int max = 0;
for (int i = 0; i < keys.Length; i++)
{
ReadOptions options = new ReadOptions();
options.SetVerifyChecksums(columnsDb.VerifyChecksum);
options.SetSnapshot(snapshot);
return options;
},
columnFamily: columnsDb._columnDbs[k]._columnFamily));
max = Math.Max(max, EnumToInt(keys[i]));
}

columnsDb._cachedMaxOrdinal = max;
}

// Build flat array of readers indexed by column ordinal
RocksDbReader[] CreateReaders()
{
RocksDbReader[] readers = new RocksDbReader[columnsDb._cachedMaxOrdinal + 1];
for (int i = 0; i < keys.Length; i++)
{
T k = keys[i];
readers[EnumToInt(k)] = new RocksDbReader(
columnsDb,
_sharedReadOptions,
_sharedCacheMissReadOptions,
readOptionsFactory,
columnFamily: columnsDb._columnDbs[k]._columnFamily);
}

return readers;
}
}

public IReadOnlyKeyValueStore GetColumn(T key)
{
return _columnDbs[key];
ObjectDisposedException.ThrowIf(Volatile.Read(ref _disposed) != 0, this);

int ordinal = EnumToInt(key);
if ((uint)ordinal >= (uint)_readers.Length || _readers[ordinal] is null)
{
throw new KeyNotFoundException($"Column '{key}' is not configured.");
}

return _readers[ordinal];
}

public void Dispose()
{
snapshot.Dispose();
if (Interlocked.Exchange(ref _disposed, 1) != 0) return;

// Explicitly destroy native ReadOptions handles to prevent finalizer queue buildup.
// GC.SuppressFinalize prevents the finalizer from running on already-destroyed handles.
DestroyReadOptions(_sharedReadOptions);
DestroyReadOptions(_sharedCacheMissReadOptions);

_snapshot.Dispose();
}

private void DestroyReadOptions(ReadOptions options)
{
_rocksDbNative.rocksdb_readoptions_destroy(options.Handle);
GC.SuppressFinalize(options);
}

// Non-boxing enum-to-int conversion. JIT eliminates dead branches at
// instantiation time, so this is zero-cost for any underlying type.
private static int EnumToInt(T value)
{
if (Unsafe.SizeOf<T>() == sizeof(int)) return Unsafe.As<T, int>(ref value);
if (Unsafe.SizeOf<T>() == sizeof(byte)) return Unsafe.As<T, byte>(ref value);
if (Unsafe.SizeOf<T>() == sizeof(short)) return Unsafe.As<T, short>(ref value);
return Convert.ToInt32(value); // fallback for long-backed enums
}
Comment on lines +292 to 300
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment above EnumToInt says “Non-boxing enum-to-int conversion”, but the Convert.ToInt32(value) fallback will box for most non-(byte/short/int)-sized enum underlying types due to overload resolution in generics. Consider tightening the comment (or adding explicit long/uint/ushort unsafe branches if truly aiming for non-boxing).

Copilot uses AI. Check for mistakes.
}
}
26 changes: 20 additions & 6 deletions src/Nethermind/Nethermind.Db.Rocks/RocksDbReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,22 +22,36 @@ public class RocksDbReader : ISortedKeyValueStore
private readonly DbOnTheRocks.IteratorManager? _iteratorManager;
private readonly ColumnFamilyHandle? _columnFamily;

readonly ReadOptions _options;
readonly ReadOptions _hintCacheMissOptions;
private readonly ReadOptions _options;
private readonly ReadOptions _hintCacheMissOptions;

public RocksDbReader(DbOnTheRocks mainDb,
Func<ReadOptions> readOptionsFactory,
DbOnTheRocks.IteratorManager? iteratorManager = null,
ColumnFamilyHandle? columnFamily = null)
: this(mainDb, readOptionsFactory(), readOptionsFactory(), readOptionsFactory, iteratorManager, columnFamily)
{
_hintCacheMissOptions.SetFillCache(false);
}

/// <summary>
/// Constructor that accepts pre-created <see cref="ReadOptions"/> instead of a factory.
/// Used by <see cref="ColumnsDb{T}.ColumnDbSnapshot"/> to share a single pair of ReadOptions
/// across all column readers, avoiding per-reader native handle allocation and finalizer pressure.
/// </summary>
public RocksDbReader(DbOnTheRocks mainDb,
ReadOptions options,
ReadOptions hintCacheMissOptions,
Func<ReadOptions> readOptionsFactory,
DbOnTheRocks.IteratorManager? iteratorManager = null,
ColumnFamilyHandle? columnFamily = null)
{
_mainDb = mainDb;
_readOptionsFactory = readOptionsFactory;
_iteratorManager = iteratorManager;
_columnFamily = columnFamily;

_options = readOptionsFactory();
_hintCacheMissOptions = readOptionsFactory();
_hintCacheMissOptions.SetFillCache(false);
_options = options;
_hintCacheMissOptions = hintCacheMissOptions;
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new constructor that accepts pre-created ReadOptions doesn’t enforce the invariant that hintCacheMissOptions should have FillCache=false (the factory-based ctor always sets this). Since this ctor is public, it’s easy for a future caller to accidentally pass an options instance with FillCache=true, changing cache behavior. Consider setting hintCacheMissOptions.SetFillCache(false) inside this ctor (or validating/asserting it) so both constructors preserve the same semantics by default.

Suggested change
_hintCacheMissOptions = hintCacheMissOptions;
_hintCacheMissOptions = hintCacheMissOptions;
_hintCacheMissOptions.SetFillCache(false);

Copilot uses AI. Check for mistakes.
}

public byte[]? Get(scoped ReadOnlySpan<byte> key, ReadFlags flags = ReadFlags.None)
Expand Down
23 changes: 23 additions & 0 deletions src/Nethermind/Nethermind.Db.Test/ColumnsDbTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,27 @@ public void SmokeTest_Snapshot()
snapshot.GetColumn(ReceiptsColumns.Blocks)
.Get(TestItem.KeccakA).Should().BeEquivalentTo(TestItem.KeccakA.BytesToArray());
}

[Test]
public void Snapshot_DoubleDispose_DoesNotThrow()
{
IColumnsDb<ReceiptsColumns> asColumnsDb = _db;
IColumnDbSnapshot<ReceiptsColumns> snapshot = asColumnsDb.CreateSnapshot();

snapshot.Dispose();

FluentActions.Invoking(() => snapshot.Dispose()).Should().NotThrow();
}

[Test]
public void Snapshot_GetColumn_AfterDispose_ThrowsObjectDisposedException()
{
IColumnsDb<ReceiptsColumns> asColumnsDb = _db;
IColumnDbSnapshot<ReceiptsColumns> snapshot = asColumnsDb.CreateSnapshot();

snapshot.Dispose();

FluentActions.Invoking(() => snapshot.GetColumn(ReceiptsColumns.Blocks))
.Should().Throw<ObjectDisposedException>();
}
}
Loading