From 056a66d713bbd6417f6e399a11d0561c316e0e5f Mon Sep 17 00:00:00 2001 From: Loretta Date: Sun, 1 Feb 2026 09:15:41 +0100 Subject: [PATCH] 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. --- AyCode.Core/Serializers/IdentityMap.cs | 107 +++++++++++-------------- 1 file changed, 46 insertions(+), 61 deletions(-) diff --git a/AyCode.Core/Serializers/IdentityMap.cs b/AyCode.Core/Serializers/IdentityMap.cs index 8d251a4..68256cf 100644 --- a/AyCode.Core/Serializers/IdentityMap.cs +++ b/AyCode.Core/Serializers/IdentityMap.cs @@ -46,8 +46,9 @@ public interface IIdentityMap /// The ID type (int, long, Guid, string) public sealed class IdentityMap : 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 SmallSize = 4096; @@ -55,10 +56,6 @@ public sealed class IdentityMap : IIdentityMap where TId : notnull //private const int SmallBitmapSize = 1024; //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) private struct HashSlot { @@ -90,8 +87,7 @@ public sealed class IdentityMap : IIdentityMap where TId : notnull public IdentityMap() { } - - + /// /// Tries to add a key to tracking (serialization). /// Returns true if first occurrence (key was added). @@ -311,16 +307,16 @@ public sealed class IdentityMap : IIdentityMap where TId : notnull // NOTE: SmallInt path DISABLED for deserializer. // The 512KB _smallValues sparse array causes cache misses. // Hash table has better cache locality. - - // // Small int fast path - // if (_useSmallInt && IsInt32) - // { - // var intKey = Unsafe.As(ref key); - // if ((uint)intKey < SmallSize) - // { - // return TryGetOrAddSmallInt(intKey, out existing); - // } - // } + + //// Small int fast path + //if (_useSmallInt && IsInt32) + //{ + // var intKey = Unsafe.As(ref key); + // if ((uint)intKey < SmallSize) + // { + // return TryGetOrAddSmallInt(intKey, out existing); + // } + //} // Hash table path if (!TryAddHash(key, out var slotIndex)) @@ -335,19 +331,13 @@ public sealed class IdentityMap : IIdentityMap where TId : notnull [MethodImpl(MethodImplOptions.AggressiveInlining)] 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) { _smallBitmap = ArrayPool.Shared.Rent(SmallBitmapSize); Array.Clear(_smallBitmap, 0, SmallBitmapSize); } - - // Lazy init values array (only when needed for deserialization) - if (_smallValues == null) - { - _smallValues = ArrayPool.Shared.Rent(SmallSize); - Array.Clear(_smallValues, 0, SmallSize); - } var wordIdx = key >> 6; var bit = 1UL << (key & 63); @@ -355,14 +345,13 @@ public sealed class IdentityMap : IIdentityMap where TId : notnull ref var word = ref _smallBitmap[wordIdx]; if ((word & bit) != 0) { - // Bitmap says seen - value is valid - existing = _smallValues![key]; + // Bitmap says seen - return stored value if any + existing = _smallValues?[key]; return false; // Already seen } - // First occurrence - mark as seen + // First occurrence - mark as seen (don't touch _smallValues!) word |= bit; - _smallValues![key] = null; existing = null; return true; } @@ -378,16 +367,16 @@ public sealed class IdentityMap : IIdentityMap where TId : notnull // The 512KB _smallValues sparse array causes cache misses. // Hash table has better cache locality (only stores actual entries). // Bitmap is only useful for tracking (serializer) - not for value storage. - - // // Small int fast path - // if (_useSmallInt && IsInt32) - // { - // var intKey = Unsafe.As(ref key); - // if ((uint)intKey < SmallSize) - // { - // return TryGetOrAddSmallIntValue(intKey, newValue); - // } - // } + + //// Small int fast path + //if (_useSmallInt && IsInt32) + //{ + // var intKey = Unsafe.As(ref key); + // if ((uint)intKey < SmallSize) + // { + // return TryGetOrAddSmallIntValue(intKey, newValue); + // } + //} // Hash table path if (!TryAddHash(key, out var slotIndex)) @@ -441,28 +430,24 @@ public sealed class IdentityMap : IIdentityMap where TId : notnull [MethodImpl(MethodImplOptions.AggressiveInlining)] public bool TryGetValue(TId key, out object? value) { - // NOTE: SmallInt path DISABLED for deserializer value lookup. - // The 512KB _smallValues sparse array causes cache misses. - // Hash table has better cache locality. - - // // Small int fast path - bitmap check first (cache-friendly, 512 bytes) - // if (_useSmallInt && IsInt32) - // { - // var intKey = Unsafe.As(ref key); - // if ((uint)intKey < SmallSize && _smallBitmap != null) - // { - // var wordIdx = intKey >> 6; - // var bit = 1UL << (intKey & 63); - // if ((_smallBitmap[wordIdx] & bit) != 0) - // { - // // Bitmap says seen - value is valid - // value = _smallValues![intKey]; - // return true; - // } - // value = null; - // return false; - // } - // } + //// Small int fast path - bitmap check first (cache-friendly, 512 bytes) + //if (_useSmallInt && IsInt32) + //{ + // var intKey = Unsafe.As(ref key); + // if ((uint)intKey < SmallSize && _smallBitmap != null) + // { + // var wordIdx = intKey >> 6; + // var bit = 1UL << (intKey & 63); + // if ((_smallBitmap[wordIdx] & bit) != 0) + // { + // // Bitmap says seen - return value if exists + // value = _smallValues?[intKey]; + // return value != null; + // } + // value = null; + // return false; + // } + //} // Hash table path if (_buckets == null)