Refactor small int optimization in IdentityMap

Clarified and restricted the "small int" optimization path to tracking (serialization) only, not value storage. Made _useSmallInt a constant, removed unnecessary _smallValues initialization, and improved comments for clarity. Updated code to safely handle uninitialized _smallValues and documented that hash table is preferred for value storage.
This commit is contained in:
Loretta 2026-02-01 09:15:41 +01:00
parent c7f44906e7
commit 056a66d713
1 changed files with 46 additions and 61 deletions

View File

@ -46,8 +46,9 @@ public interface IIdentityMap
/// <typeparam name="TId">The ID type (int, long, Guid, string)</typeparam> /// <typeparam name="TId">The ID type (int, long, Guid, string)</typeparam>
public sealed class IdentityMap<TId> : IIdentityMap where TId : notnull public sealed class IdentityMap<TId> : IIdentityMap where TId : notnull
{ {
private bool _useSmallInt = false; private const bool _useSmallInt = true;
// Small int optimization (TId = int only, 0-4095), 512 bytes (fits in L1 cache!)
private const int SmallBitmapSize = 64; private const int SmallBitmapSize = 64;
private const int SmallSize = 4096; private const int SmallSize = 4096;
@ -55,10 +56,6 @@ public sealed class IdentityMap<TId> : IIdentityMap where TId : notnull
//private const int SmallBitmapSize = 1024; //private const int SmallBitmapSize = 1024;
//private const int SmallSize = SmallBitmapSize * 64; //private const int SmallSize = SmallBitmapSize * 64;
// Small int optimization (TId = int only, 0-4095)
//private const int SmallSize = 4096;
//private const int SmallBitmapSize = SmallSize / 64; // 64 ulongs = 512 bytes (fits in L1 cache!)
// Slot for hash table entries (generation needed for hash table validity) // Slot for hash table entries (generation needed for hash table validity)
private struct HashSlot private struct HashSlot
{ {
@ -90,8 +87,7 @@ public sealed class IdentityMap<TId> : IIdentityMap where TId : notnull
public IdentityMap() public IdentityMap()
{ {
} }
/// <summary> /// <summary>
/// Tries to add a key to tracking (serialization). /// Tries to add a key to tracking (serialization).
/// Returns true if first occurrence (key was added). /// Returns true if first occurrence (key was added).
@ -311,16 +307,16 @@ public sealed class IdentityMap<TId> : IIdentityMap where TId : notnull
// NOTE: SmallInt path DISABLED for deserializer. // NOTE: SmallInt path DISABLED for deserializer.
// The 512KB _smallValues sparse array causes cache misses. // The 512KB _smallValues sparse array causes cache misses.
// Hash table has better cache locality. // Hash table has better cache locality.
// // Small int fast path //// Small int fast path
// if (_useSmallInt && IsInt32) //if (_useSmallInt && IsInt32)
// { //{
// var intKey = Unsafe.As<TId, int>(ref key); // var intKey = Unsafe.As<TId, int>(ref key);
// if ((uint)intKey < SmallSize) // if ((uint)intKey < SmallSize)
// { // {
// return TryGetOrAddSmallInt(intKey, out existing); // return TryGetOrAddSmallInt(intKey, out existing);
// } // }
// } //}
// Hash table path // Hash table path
if (!TryAddHash(key, out var slotIndex)) if (!TryAddHash(key, out var slotIndex))
@ -335,19 +331,13 @@ public sealed class IdentityMap<TId> : IIdentityMap where TId : notnull
[MethodImpl(MethodImplOptions.AggressiveInlining)] [MethodImpl(MethodImplOptions.AggressiveInlining)]
private bool TryGetOrAddSmallInt(int key, out object? existing) private bool TryGetOrAddSmallInt(int key, out object? existing)
{ {
// Lazy init bitmap // Lazy init bitmap only - NOT _smallValues!
// This method is for tracking (key seen?), not value storage.
if (_smallBitmap == null) if (_smallBitmap == null)
{ {
_smallBitmap = ArrayPool<ulong>.Shared.Rent(SmallBitmapSize); _smallBitmap = ArrayPool<ulong>.Shared.Rent(SmallBitmapSize);
Array.Clear(_smallBitmap, 0, SmallBitmapSize); Array.Clear(_smallBitmap, 0, SmallBitmapSize);
} }
// Lazy init values array (only when needed for deserialization)
if (_smallValues == null)
{
_smallValues = ArrayPool<object?>.Shared.Rent(SmallSize);
Array.Clear(_smallValues, 0, SmallSize);
}
var wordIdx = key >> 6; var wordIdx = key >> 6;
var bit = 1UL << (key & 63); var bit = 1UL << (key & 63);
@ -355,14 +345,13 @@ public sealed class IdentityMap<TId> : IIdentityMap where TId : notnull
ref var word = ref _smallBitmap[wordIdx]; ref var word = ref _smallBitmap[wordIdx];
if ((word & bit) != 0) if ((word & bit) != 0)
{ {
// Bitmap says seen - value is valid // Bitmap says seen - return stored value if any
existing = _smallValues![key]; existing = _smallValues?[key];
return false; // Already seen return false; // Already seen
} }
// First occurrence - mark as seen // First occurrence - mark as seen (don't touch _smallValues!)
word |= bit; word |= bit;
_smallValues![key] = null;
existing = null; existing = null;
return true; return true;
} }
@ -378,16 +367,16 @@ public sealed class IdentityMap<TId> : IIdentityMap where TId : notnull
// The 512KB _smallValues sparse array causes cache misses. // The 512KB _smallValues sparse array causes cache misses.
// Hash table has better cache locality (only stores actual entries). // Hash table has better cache locality (only stores actual entries).
// Bitmap is only useful for tracking (serializer) - not for value storage. // Bitmap is only useful for tracking (serializer) - not for value storage.
// // Small int fast path //// Small int fast path
// if (_useSmallInt && IsInt32) //if (_useSmallInt && IsInt32)
// { //{
// var intKey = Unsafe.As<TId, int>(ref key); // var intKey = Unsafe.As<TId, int>(ref key);
// if ((uint)intKey < SmallSize) // if ((uint)intKey < SmallSize)
// { // {
// return TryGetOrAddSmallIntValue(intKey, newValue); // return TryGetOrAddSmallIntValue(intKey, newValue);
// } // }
// } //}
// Hash table path // Hash table path
if (!TryAddHash(key, out var slotIndex)) if (!TryAddHash(key, out var slotIndex))
@ -441,28 +430,24 @@ public sealed class IdentityMap<TId> : IIdentityMap where TId : notnull
[MethodImpl(MethodImplOptions.AggressiveInlining)] [MethodImpl(MethodImplOptions.AggressiveInlining)]
public bool TryGetValue(TId key, out object? value) public bool TryGetValue(TId key, out object? value)
{ {
// NOTE: SmallInt path DISABLED for deserializer value lookup. //// Small int fast path - bitmap check first (cache-friendly, 512 bytes)
// The 512KB _smallValues sparse array causes cache misses. //if (_useSmallInt && IsInt32)
// Hash table has better cache locality. //{
// var intKey = Unsafe.As<TId, int>(ref key);
// // Small int fast path - bitmap check first (cache-friendly, 512 bytes) // if ((uint)intKey < SmallSize && _smallBitmap != null)
// if (_useSmallInt && IsInt32) // {
// { // var wordIdx = intKey >> 6;
// var intKey = Unsafe.As<TId, int>(ref key); // var bit = 1UL << (intKey & 63);
// if ((uint)intKey < SmallSize && _smallBitmap != null) // if ((_smallBitmap[wordIdx] & bit) != 0)
// { // {
// var wordIdx = intKey >> 6; // // Bitmap says seen - return value if exists
// var bit = 1UL << (intKey & 63); // value = _smallValues?[intKey];
// if ((_smallBitmap[wordIdx] & bit) != 0) // return value != null;
// { // }
// // Bitmap says seen - value is valid // value = null;
// value = _smallValues![intKey]; // return false;
// return true; // }
// } //}
// value = null;
// return false;
// }
// }
// Hash table path // Hash table path
if (_buckets == null) if (_buckets == null)