Skip to content

Commit 85f86c6

Browse files
committed
perf: minor improvements for AddHashElement (elide extra bounds check, remove the need for IDictionary devirtualization, reduce branching graph depth), this adds about 13% on .NET 8
1 parent 2864935 commit 85f86c6

File tree

1 file changed

+13
-8
lines changed

1 file changed

+13
-8
lines changed

CardinalityEstimation/CardinalityEstimator.cs

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@ public class CardinalityEstimator : ICardinalityEstimator<string>, ICardinalityE
111111
/// <summary>
112112
/// Lookup dictionary for the sparse representation of HLL buckets
113113
/// </summary>
114-
private IDictionary<ushort, byte> lookupSparse;
114+
private Dictionary<ushort, byte> lookupSparse;
115115

116116
/// <summary>
117117
/// Max number of elements to hold in the sparse representation before switching to dense
@@ -787,19 +787,24 @@ private bool AddElementHash(ulong hashCode)
787787
if (isSparse)
788788
{
789789
lookupSparse.TryGetValue(substream, out byte prevRank);
790-
lookupSparse[substream] = Math.Max(prevRank, sigma);
791-
changed = changed || (prevRank != sigma && lookupSparse[substream] == sigma);
792-
if (lookupSparse.Count > sparseMaxElements)
790+
if (sigma > prevRank)
793791
{
794-
SwitchToDenseRepresentation();
792+
lookupSparse[substream] = sigma;
793+
if (lookupSparse.Count > sparseMaxElements)
794+
{
795+
SwitchToDenseRepresentation();
796+
}
795797
changed = true;
796798
}
797799
}
798800
else
799801
{
800-
var prevMax = lookupDense[substream];
801-
lookupDense[substream] = Math.Max(prevMax, sigma);
802-
changed = changed || (prevMax != sigma && lookupDense[substream] == sigma);
802+
ref var value = ref lookupDense[substream];
803+
if (sigma > value)
804+
{
805+
value = sigma;
806+
changed = true;
807+
}
803808
}
804809
return changed;
805810
}

0 commit comments

Comments
 (0)