Skip to content
Open
Show file tree
Hide file tree
Changes from 3 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
106 changes: 89 additions & 17 deletions src/Nethermind/Nethermind.Db.Rocks/ColumnsDb.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,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 +163,98 @@ 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,
() =>
{
ReadOptions options = new ReadOptions();
options.SetVerifyChecksums(columnsDb.VerifyChecksum);
options.SetSnapshot(snapshot);
return options;
},
columnFamily: columnsDb._columnDbs[k]._columnFamily));
private readonly Snapshot _snapshot;
private readonly ReadOptions _sharedReadOptions;
private readonly ReadOptions _sharedCacheMissReadOptions;
private bool _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;

// 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 = new ReadOptions();
_sharedReadOptions.SetVerifyChecksums(columnsDb.VerifyChecksum);
_sharedReadOptions.SetSnapshot(snapshot);
Copy link
Member

Choose a reason for hiding this comment

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

Helper method


_sharedCacheMissReadOptions = new ReadOptions();
_sharedCacheMissReadOptions.SetVerifyChecksums(columnsDb.VerifyChecksum);
_sharedCacheMissReadOptions.SetSnapshot(snapshot);
_sharedCacheMissReadOptions.SetFillCache(false);
Copy link
Member

Choose a reason for hiding this comment

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

Helper method


// 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 = () =>
{
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.
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;
}
Copy link
Member

Choose a reason for hiding this comment

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

helper method


if (columnsDb._cachedMaxOrdinal < 0)
{
int max = 0;
for (int i = 0; i < keys.Length; i++)
max = Math.Max(max, Convert.ToInt32(keys[i]));
columnsDb._cachedMaxOrdinal = max;
}
Copy link
Member

Choose a reason for hiding this comment

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

helper method


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

Choose a reason for hiding this comment

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

Helper method

}

public IReadOnlyKeyValueStore GetColumn(T key)
{
return _columnDbs[key];
return _readers[Convert.ToInt32(key)];
Copy link
Member

Choose a reason for hiding this comment

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

can be single line with =>

}

public void Dispose()
{
snapshot.Dispose();
if (_disposed) return;
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't need to be threadsafe?

We could do - although would need to by byte not bool

Suggested change
if (_disposed) return;
bool disposed = Interlocked.Exchange(ref _disposed, true);
if (disposed) return;

_disposed = true;

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

_snapshot.Dispose();
}
}
}
20 changes: 20 additions & 0 deletions src/Nethermind/Nethermind.Db.Rocks/RocksDbReader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,26 @@ public RocksDbReader(DbOnTheRocks mainDb,
_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)
Comment on lines +42 to +47
Copy link
Member

Choose a reason for hiding this comment

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

Can this constructor be called by the other constructor with : this()? I think so

{
_mainDb = mainDb;
_readOptionsFactory = readOptionsFactory;
_iteratorManager = iteratorManager;
_columnFamily = columnFamily;
_options = options;
_hintCacheMissOptions = hintCacheMissOptions;
}

public byte[]? Get(scoped ReadOnlySpan<byte> key, ReadFlags flags = ReadFlags.None)
{
if ((flags & ReadFlags.HintReadAhead) != 0 && _iteratorManager is not null)
Expand Down
Loading