Skip to content

Commit 01ee398

Browse files
Merge pull request #168 from marius-bughiu/fix/issue-27-bulk-ctor-resize
perf(collections): size bulk constructors for load factor so they never resize (#27)
2 parents 98a6426 + 2fca0c0 commit 01ee398

11 files changed

Lines changed: 428 additions & 37 deletions

File tree

CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,12 @@ All notable changes to Celerity are documented here. This project follows [Keep
113113

114114
- All remaining `xUnit2013` warnings in `Celerity.Tests` are gone: `Assert.Equal(0, x.Count)` / `Assert.Equal(1, x.Count)` against a Celerity collection now use the xUnit-idiomatic `Assert.Empty(x)` / `Assert.Single(x)`, which produce better failure messages (`"The collection was expected to contain a single element, but it was empty"` instead of `"Expected: 1 / Actual: 0"`) and are also a strictly stronger assertion — `Assert.Single` enumerates and counts, so a collection whose `Count` property and enumerator disagree would now fail at this site instead of silently passing. The replacement is mechanical and spans 10 files: `RemoveOutValueTests.cs` (44 sites), `AddAndTryAddTests.cs` (28), `LongDictionaryTests.cs` / `IntDictionaryTests.cs` / `CelerityDictionaryTests.cs` / `CelerityDictionaryCollisionTests.cs` (8 each), `IEnumerableConstructorTests.cs` (6), `ReadOnlyDictionaryInterfaceTests.cs` (4), and `LongDictionaryCollisionTests.cs` / `IntDictionaryCollisionTests.cs` (2 each) — exactly the file list and call counts the weekly automated code-review sweep flagged. A clean `dotnet build --no-incremental` now produces zero `xUnit2013` warnings, so genuinely new warnings on a touched test file aren't drowned in 64 pre-existing ones. No test was added, removed, or weakened, and the full `Celerity.Tests` suite (818 tests) still passes locally on `net8.0`. Closes #123.
115115

116+
### Fixed
117+
118+
- Bulk `IEnumerable<…>`-source constructors now size the backing table to hold the whole source **without resizing**, honouring the documented "the source's `Count` is used to size the backing storage so inserts do not resize" contract that was previously not met (issue #27 — *Optimize resize operations* / *Reduce allocations in critical paths*). The resize threshold is `size × loadFactor`, so a table sized to the raw source count still tripped a full rehash-and-copy on the last inserts of the bulk fill (e.g. a 100-pair source at the default `0.75` load factor sized to `128`, threshold `96`, and rehashed into `256` near the end). The fix scales the requested capacity up by `1 / loadFactor` in each collection's `InitialCapacityForSource` helper, so a known-count source fits below the threshold and the bulk build is a single allocation with one hash per entry and zero rehashes. Applied uniformly to all seven hash-table collections — `IntDictionary`, `LongDictionary`, `CelerityDictionary`, `IntSet`, `LongSet`, `CeleritySet`, and `CelerityMultiMap` (distinct-key fills; duplicate-heavy sources simply leave slack, never resize). The plain `(capacity, loadFactor)` constructor is unchanged — `capacity` still rounds to the next power of two as documented — and `SmallDictionary` is exempt (no load factor, no hasher: its capacity-verbatim sizing already fills without resizing). The null-source-before-loadFactor-validation ordering (issue #94) is preserved: the new sizing math runs only after the `ArgumentNullException.ThrowIfNull(source)` check and is skipped for an out-of-range load factor, which the primary constructor still rejects with `ArgumentOutOfRangeException`. A non-collection source (unknown count) falls through to the plain capacity exactly as before.
119+
- `BulkConstructorNoResizeTests` — a new cross-collection shared regression suite pinning the fix the same way `TryAddProbeCountTests` pins the single-probe contract: by counting `IHashProvider<T>.Hash` calls during construction. Building a table of `N = 100` distinct keys with no resize costs exactly `N` hash calls (one probe-chain walk per insert); a resize re-hashes every entry already placed, so a mid-build resize would push the count above `N`. Each of the seven hash-table collections gets a from-collection no-resize fact, plus a load-factor-scaling theory (`0.25` / `0.5` / `0.75` / `0.95`) asserting the headroom tracks a non-default load factor, and a non-collection-enumerable fallback case asserting correctness when the count is unknown. `SmallDictionary` is intentionally excluded (no hasher to count). The assertion fails on the pre-fix sizing and passes after it. Full `Celerity.Tests` suite is green at 1978 tests on `net8.0` (10 new).
120+
- `MemoryAllocationBenchmark` extended with a `FromCollection` category — `Dictionary<int, int>`, `IntDictionary<int>`, and `CelerityDictionary<int, int, Int32WangNaiveHasher>` each built from a known-count `KeyValuePair<int, int>[]` source via the `IEnumerable` constructor, with the full `MemoryDiagnoser` columns. It sits alongside the existing `Grow` / `Presized` cases and makes the saved rehash-and-copy allocation visible. Part of the extended (weekly / on-demand) suite published to the `dev/bench-extended` dashboard; no `Program.cs` change needed (`MemoryAllocationBenchmark` is already registered).
121+
116122
## [1.4.0] - 2026-05-31
117123

118124
### Added

docs/performance.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,10 @@ for (int i = 0; i < 1000; i++) d[i] = Work(i);
6666

6767
Account for the load factor when sizing: at the default `0.75`, a `capacity` of 1024 resizes once you pass 768 entries. To hold 1,000 entries without any resize, you need `capacity`~1365 (which rounds to 2048), or a higher load factor.
6868

69-
**Building from an existing collection sizes itself.** The `IEnumerable<KeyValuePair<TKey, TValue>>` constructor reads `ICollection<T>.Count` when the source exposes it and pre-sizes the backing storage, so bulk-filling from a `List<>`, array, or BCL `Dictionary<,>` avoids resize work for free:
69+
**Building from an existing collection sizes itself.** The `IEnumerable<KeyValuePair<TKey, TValue>>` constructor reads `ICollection<T>.Count` when the source exposes it and pre-sizes the backing storage**including the load-factor headroom**so bulk-filling from a `List<>`, array, or BCL `Dictionary<,>` avoids resize work for free. Unlike the plain `capacity` constructor (where you account for the load factor yourself, per the note above), the source constructor scales the count up by `1 / loadFactor` for you, so the whole source lands below the resize threshold in a single allocation:
7070

7171
```csharp
72-
var fast = new IntDictionary<string>(existingList); // pre-sized from Count
72+
var fast = new IntDictionary<string>(existingList); // pre-sized from Count, no resize
7373
```
7474

7575
For a non-`ICollection` enumerable (a LINQ query, a generator), `Count` is unknown — pass an explicit `capacity` if you can estimate the size.

src/Celerity.Benchmarks/MemoryAllocationBenchmark.cs

Lines changed: 32 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,35 @@
1919
/// </list>
2020
/// The gap between the two rows is exactly what a caller saves by passing a
2121
/// capacity to the constructor.
22+
///
23+
/// A third <c>FromCollection</c> case builds each dictionary from a known-count
24+
/// <see cref="ICollection{T}"/> source via the <c>IEnumerable</c> constructor.
25+
/// That constructor sizes the backing store from the source's <c>Count</c>; the
26+
/// fix for issue #27 adds the load-factor headroom so the whole source fits below
27+
/// the resize threshold, eliminating the one rehash-and-copy a count-sized table
28+
/// would otherwise pay near the end of the bulk fill.
2229
/// </remarks>
2330
[MemoryDiagnoser]
2431
[CategoriesColumn]
2532
[GroupBenchmarksBy(BenchmarkLogicalGroupRule.ByCategory)]
2633
public class MemoryAllocationBenchmark
2734
{
2835
private int[] keys = null!;
36+
private KeyValuePair<int, int>[] pairs = null!;
2937

3038
[Params(100_000)]
3139
public int ItemCount;
3240

3341
[GlobalSetup]
34-
public void Setup() => keys = KeyDistributions.Int32(Distribution.Uniform, ItemCount);
42+
public void Setup()
43+
{
44+
keys = KeyDistributions.Int32(Distribution.Uniform, ItemCount);
45+
// Distinct sequential keys: the IEnumerable constructor uses Add, which
46+
// rejects duplicates, so the source must be collision-free.
47+
pairs = new KeyValuePair<int, int>[ItemCount];
48+
for (int i = 0; i < ItemCount; i++)
49+
pairs[i] = new KeyValuePair<int, int>(i, i);
50+
}
3551

3652
// ── Grow from default capacity (resize churn included) ──────────────────────
3753

@@ -108,4 +124,19 @@ public CelerityDictionary<int, int, Int32WangNaiveHasher> CelerityDictionary_Pre
108124
}
109125
return map;
110126
}
127+
128+
// ── Bulk-built from a known-count collection (issue #27 sizing fix) ──────────
129+
130+
[Benchmark]
131+
[BenchmarkCategory("FromCollection")]
132+
public Dictionary<int, int> Dictionary_FromCollection() => new Dictionary<int, int>(pairs);
133+
134+
[Benchmark]
135+
[BenchmarkCategory("FromCollection")]
136+
public IntDictionary<int> IntDictionary_FromCollection() => new IntDictionary<int>(pairs);
137+
138+
[Benchmark]
139+
[BenchmarkCategory("FromCollection")]
140+
public CelerityDictionary<int, int, Int32WangNaiveHasher> CelerityDictionary_FromCollection()
141+
=> new CelerityDictionary<int, int, Int32WangNaiveHasher>(pairs);
111142
}
Lines changed: 222 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,222 @@
1+
using Celerity.Collections;
2+
using Celerity.Hashing;
3+
4+
namespace Celerity.Tests.Collections;
5+
6+
/// <summary>
7+
/// Regression tests for the bulk-constructor sizing fix (issue #27). The
8+
/// <c>IEnumerable&lt;…&gt;</c> source constructors of every hash-table collection
9+
/// document that the source's <c>Count</c> is used to size the backing storage
10+
/// "so inserts do not resize" (or "so the initial fill avoids resize work").
11+
/// Before the fix the table was sized to the raw count, but the resize threshold
12+
/// is <c>size × loadFactor</c>, so the last few inserts of the bulk fill still
13+
/// tripped a full rehash. The fix scales the requested capacity up by
14+
/// <c>1 / loadFactor</c> so the whole source fits below the threshold.
15+
///
16+
/// These tests pin the contract the same way <see cref="TryAddProbeCountTests"/>
17+
/// does: by counting <see cref="IHashProvider{T}.Hash"/> calls. Building a table
18+
/// of <c>N</c> distinct keys with no resize costs exactly <c>N</c> hash calls
19+
/// (one probe-chain walk per insert). A resize re-hashes every entry already in
20+
/// the table, so a resize during construction would push the count above <c>N</c>.
21+
/// Asserting <c>== N</c> therefore fails on the pre-fix sizing and passes after it.
22+
///
23+
/// <c>SmallDictionary</c> is intentionally excluded: it has no load factor and no
24+
/// hasher (it linear-scans), so its capacity-verbatim sizing already fills without
25+
/// resizing and there is nothing to count.
26+
/// </summary>
27+
public class BulkConstructorNoResizeTests
28+
{
29+
private const int N = 100;
30+
31+
private static int _hashCallCount;
32+
33+
private struct CountingIntHasher : IHashProvider<int>
34+
{
35+
public int Hash(int key)
36+
{
37+
_hashCallCount++;
38+
unchecked
39+
{
40+
uint x = (uint)key;
41+
x = ((x >> 16) ^ x) * 0x45d9f3b;
42+
x = ((x >> 16) ^ x) * 0x45d9f3b;
43+
x = (x >> 16) ^ x;
44+
return (int)x;
45+
}
46+
}
47+
}
48+
49+
private struct CountingLongHasher : IHashProvider<long>
50+
{
51+
public int Hash(long key)
52+
{
53+
_hashCallCount++;
54+
unchecked
55+
{
56+
ulong x = (ulong)key;
57+
x = (x ^ (x >> 30)) * 0xbf58476d1ce4e5b9UL;
58+
x = (x ^ (x >> 27)) * 0x94d049bb133111ebUL;
59+
x = x ^ (x >> 31);
60+
return (int)x;
61+
}
62+
}
63+
}
64+
65+
private struct CountingStringHasher : IHashProvider<string>
66+
{
67+
public int Hash(string key)
68+
{
69+
_hashCallCount++;
70+
return key.GetHashCode();
71+
}
72+
}
73+
74+
private static KeyValuePair<int, int>[] IntPairs() =>
75+
Enumerable.Range(1, N).Select(i => new KeyValuePair<int, int>(i, i * 10)).ToArray();
76+
77+
private static KeyValuePair<long, int>[] LongPairs() =>
78+
Enumerable.Range(1, N).Select(i => new KeyValuePair<long, int>(i, i * 10)).ToArray();
79+
80+
private static KeyValuePair<string, int>[] StringPairs() =>
81+
Enumerable.Range(1, N).Select(i => new KeyValuePair<string, int>($"k{i}", i)).ToArray();
82+
83+
// ── Dictionaries ────────────────────────────────────────────────────────────
84+
85+
[Fact]
86+
public void IntDictionary_BulkConstruct_FromKnownCount_DoesNotResize()
87+
{
88+
KeyValuePair<int, int>[] src = IntPairs();
89+
_hashCallCount = 0;
90+
91+
var map = new IntDictionary<int, CountingIntHasher>(src);
92+
93+
Assert.Equal(N, _hashCallCount);
94+
Assert.Equal(N, map.Count);
95+
for (int i = 1; i <= N; i++)
96+
Assert.Equal(i * 10, map[i]);
97+
}
98+
99+
[Fact]
100+
public void LongDictionary_BulkConstruct_FromKnownCount_DoesNotResize()
101+
{
102+
KeyValuePair<long, int>[] src = LongPairs();
103+
_hashCallCount = 0;
104+
105+
var map = new LongDictionary<int, CountingLongHasher>(src);
106+
107+
Assert.Equal(N, _hashCallCount);
108+
Assert.Equal(N, map.Count);
109+
for (int i = 1; i <= N; i++)
110+
Assert.Equal(i * 10, map[i]);
111+
}
112+
113+
[Fact]
114+
public void CelerityDictionary_BulkConstruct_FromKnownCount_DoesNotResize()
115+
{
116+
KeyValuePair<string, int>[] src = StringPairs();
117+
_hashCallCount = 0;
118+
119+
var map = new CelerityDictionary<string, int, CountingStringHasher>(src);
120+
121+
Assert.Equal(N, _hashCallCount);
122+
Assert.Equal(N, map.Count);
123+
for (int i = 1; i <= N; i++)
124+
Assert.Equal(i, map[$"k{i}"]);
125+
}
126+
127+
// ── Sets ──────────────────────────────────────────────────────────────────────
128+
129+
[Fact]
130+
public void IntSet_BulkConstruct_FromKnownCount_DoesNotResize()
131+
{
132+
int[] src = Enumerable.Range(1, N).ToArray();
133+
_hashCallCount = 0;
134+
135+
var set = new IntSet<CountingIntHasher>(src);
136+
137+
Assert.Equal(N, _hashCallCount);
138+
Assert.Equal(N, set.Count);
139+
for (int i = 1; i <= N; i++)
140+
Assert.True(set.Contains(i));
141+
}
142+
143+
[Fact]
144+
public void LongSet_BulkConstruct_FromKnownCount_DoesNotResize()
145+
{
146+
long[] src = Enumerable.Range(1, N).Select(i => (long)i).ToArray();
147+
_hashCallCount = 0;
148+
149+
var set = new LongSet<CountingLongHasher>(src);
150+
151+
Assert.Equal(N, _hashCallCount);
152+
Assert.Equal(N, set.Count);
153+
for (int i = 1; i <= N; i++)
154+
Assert.True(set.Contains(i));
155+
}
156+
157+
[Fact]
158+
public void CeleritySet_BulkConstruct_FromKnownCount_DoesNotResize()
159+
{
160+
string[] src = Enumerable.Range(1, N).Select(i => $"v{i}").ToArray();
161+
_hashCallCount = 0;
162+
163+
var set = new CeleritySet<string, CountingStringHasher>(src);
164+
165+
Assert.Equal(N, _hashCallCount);
166+
Assert.Equal(N, set.Count);
167+
for (int i = 1; i <= N; i++)
168+
Assert.True(set.Contains($"v{i}"));
169+
}
170+
171+
// ── MultiMap ────────────────────────────────────────────────────────────────
172+
173+
[Fact]
174+
public void CelerityMultiMap_BulkConstruct_FromKnownDistinctCount_DoesNotResize()
175+
{
176+
KeyValuePair<int, int>[] src = IntPairs();
177+
_hashCallCount = 0;
178+
179+
var map = new CelerityMultiMap<int, int, CountingIntHasher>(src);
180+
181+
Assert.Equal(N, _hashCallCount);
182+
Assert.Equal(N, map.Count);
183+
for (int i = 1; i <= N; i++)
184+
Assert.Equal(new[] { i * 10 }, map[i].ToArray());
185+
}
186+
187+
// ── Load-factor scaling: the headroom must track a non-default load factor ────
188+
189+
[Theory]
190+
[InlineData(0.25f)]
191+
[InlineData(0.5f)]
192+
[InlineData(0.75f)]
193+
[InlineData(0.95f)]
194+
public void IntDictionary_BulkConstruct_HoldsSourceWithoutResize_AcrossLoadFactors(float loadFactor)
195+
{
196+
KeyValuePair<int, int>[] src = IntPairs();
197+
_hashCallCount = 0;
198+
199+
var map = new IntDictionary<int, CountingIntHasher>(src, loadFactor: loadFactor);
200+
201+
Assert.Equal(N, _hashCallCount);
202+
Assert.Equal(N, map.Count);
203+
for (int i = 1; i <= N; i++)
204+
Assert.Equal(i * 10, map[i]);
205+
}
206+
207+
// ── Fallback: a non-collection source (unknown count) still builds correctly ──
208+
// (No resize guarantee is possible without a count; only correctness is pinned.)
209+
210+
[Fact]
211+
public void IntDictionary_BulkConstruct_FromNonCollectionEnumerable_IsCorrect()
212+
{
213+
IEnumerable<KeyValuePair<int, int>> src =
214+
Enumerable.Range(1, N).Select(i => new KeyValuePair<int, int>(i, i * 10));
215+
216+
var map = new IntDictionary<int, CountingIntHasher>(src);
217+
218+
Assert.Equal(N, map.Count);
219+
for (int i = 1; i <= N; i++)
220+
Assert.Equal(i * 10, map[i]);
221+
}
222+
}

src/Celerity/Collections/CelerityDictionary.cs

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,8 +86,9 @@ public CelerityDictionary(
8686
/// <c>Count</c> is used to size the backing storage so inserts do not resize.
8787
/// </param>
8888
/// <param name="capacity">
89-
/// The minimum initial capacity. The final capacity is the larger of this
90-
/// value and the source's count, rounded up to the next power of two.
89+
/// The minimum initial capacity, rounded up to the next power of two. When
90+
/// the source's count is larger, the backing store is sized — including
91+
/// load-factor headroom — to hold the whole source without resizing.
9192
/// </param>
9293
/// <param name="loadFactor">
9394
/// The fraction of the dictionary's size that can be filled before resizing.
@@ -102,7 +103,7 @@ public CelerityDictionary(
102103
IEnumerable<KeyValuePair<TKey, TValue>> source,
103104
int capacity = DEFAULT_CAPACITY,
104105
float loadFactor = DEFAULT_LOAD_FACTOR)
105-
: this(InitialCapacityForSource(source, capacity), loadFactor)
106+
: this(InitialCapacityForSource(source, capacity, loadFactor), loadFactor)
106107
{
107108
foreach (KeyValuePair<TKey, TValue> kvp in source)
108109
{
@@ -116,10 +117,28 @@ public CelerityDictionary(
116117
// when the user also passed an invalid loadFactor.
117118
private static int InitialCapacityForSource(
118119
IEnumerable<KeyValuePair<TKey, TValue>> source,
119-
int capacity)
120+
int capacity,
121+
float loadFactor)
120122
{
121123
ArgumentNullException.ThrowIfNull(source);
122-
return Math.Max(capacity, (source as ICollection<KeyValuePair<TKey, TValue>>)?.Count ?? 0);
124+
int count = (source as ICollection<KeyValuePair<TKey, TValue>>)?.Count ?? 0;
125+
126+
// Size for the source count *including* load-factor headroom: the resize
127+
// threshold is size*loadFactor, so a table sized to the raw count would
128+
// still rehash on the last inserts of the bulk fill. Scaling the count up
129+
// by 1/loadFactor makes the "Count is used to size the backing storage so
130+
// inserts do not resize" contract actually hold (issue #27). A
131+
// non-collection source (count 0) or an out-of-range loadFactor — left for
132+
// the primary ctor to reject, so null-source-beats-bad-loadFactor ordering
133+
// is preserved — falls through to the plain capacity.
134+
if (count > 0 && loadFactor > 0f && loadFactor < 1f)
135+
{
136+
int withHeadroom = (int)Math.Ceiling(count / (double)loadFactor);
137+
if (withHeadroom > count)
138+
count = withHeadroom;
139+
}
140+
141+
return Math.Max(capacity, count);
123142
}
124143

125144
/// <summary>

0 commit comments

Comments
 (0)