perf(collections): size bulk constructors for load factor so they never resize (#27) - #168
Merged
Merged
Conversation
…er resize (#27) The `IEnumerable<…>`-source constructors of every hash-table collection document that the source's `Count` is used to size the backing storage "so inserts do not resize". That promise was not kept: 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 (a 100-pair source at the default 0.75 load factor sized to 128, threshold 96, and rehashed into 256 near the end). Each collection's `InitialCapacityForSource` helper now scales the requested capacity up by `1 / loadFactor`, 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 IntDictionary, LongDictionary, CelerityDictionary, IntSet, LongSet, CeleritySet, and CelerityMultiMap. The plain `(capacity, loadFactor)` constructor is unchanged (capacity still rounds to the next power of two). SmallDictionary is exempt (no load factor, no hasher). The null-source-before-loadFactor-validation ordering (#94) is preserved: the sizing math runs only after the null check and is skipped for an out-of-range load factor, which the primary ctor still rejects. Addresses issue #27 (Optimize resize operations / Reduce allocations in critical paths). - src/Celerity/Collections/*: load-factor headroom in the 7 source ctors - BulkConstructorNoResizeTests: hash-call-counting regression suite pinning exactly N hashes (no resize) per collection, a load-factor-scaling theory, and the non-collection fallback. 1978 tests green on net8.0 (10 new). - MemoryAllocationBenchmark: new FromCollection category (Dictionary vs IntDictionary vs CelerityDictionary) showing the saved allocation - docs/performance.md: clarify the source ctor includes load-factor headroom - CHANGELOG: Fixed section bullets for the fix, tests, and benchmark Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Coverage
|
Benchmarks1 regression Highlights
Collections (104)
Hashers (90)
Same-runner A/B: main ( |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Tier-(a) pick: issue #27 (Performance optimizations,
priority:high, milestone 1.2.0) — specifically its Optimize resize operations / Reduce allocations in critical paths tasks. (All three 1.2.0 collections —FrozenCelerityDictionary,CelerityMultiMap,SmallDictionary— are already shipped, so this advances the milestone's remaining Performance lane. #27 is an umbrella and stays open; this is one focused, complete unit within it.)Every hash-table collection's
IEnumerable<…>-source constructor documents that the source'sCountis used to size the backing storage "so inserts do not resize." That promise was not met: the resize threshold issize × loadFactor, so a table sized to the raw count still tripped a full rehash-and-copy on the last inserts of the bulk fill. Example at the default0.75load factor: a 100-pair source sized to128, threshold96, then rehashed into256near the end — an extraint[]/TValue[]allocation pair and ~95 redundant hashes thrown away.The fix scales the requested capacity up by
1 / loadFactorin each collection'sInitialCapacityForSourcehelper, so a known-count source lands below the threshold and the bulk build is a single allocation with one hash per entry and zero rehashes.Scope & safety
IntDictionary,LongDictionary,CelerityDictionary,IntSet,LongSet,CeleritySet,CelerityMultiMap(distinct-key fills; duplicate-heavy sources just leave slack, never resize).(capacity, loadFactor)constructor unchanged —capacitystill rounds to the next power of two as documented. Only the source constructor gains the headroom, so the two ctors stay consistent for the samecapacityvalue.SmallDictionaryis exempt — it has no load factor and no hasher (it linear-scans), so its capacity-verbatim sizing already fills without resizing.ArgumentNullException.ThrowIfNull(source)and is skipped for an out-of-range load factor, which the primary ctor still rejects withArgumentOutOfRangeException. A non-collection source (unknown count) falls through to the plain capacity exactly as before.Parity facets
This is a perf fix to existing behaviour, not a new type, so the new-collection parity facets (dedicated test files, new shared-test type rows, dashboard
COLLECTIONS/ship-card entries, new docs sections, ROADMAP status flip) do not apply. The facets that do:InitialCapacityForSourcehelpers + accurate XML-doc<param>textBulkConstructorNoResizeTests— hash-call-counting regression suite (10 facts/theory cases)MemoryAllocationBenchmarkgains aFromCollectioncategorydocs/performance.mdclarified;docs/api/collections.mdclaim now actually holds[Unreleased] → Fixedbullets for fix / tests / benchmarkBulkConstructorNoResizeTestspins the contract the wayTryAddProbeCountTestsdoes — by countingIHashProvider.Hashcalls. A no-resize build ofNdistinct keys costs exactlyNhashes; a mid-build resize re-hashes placed entries and pushes the count aboveN. The assertion fails on the pre-fix sizing and passes after it, with a load-factor-scaling theory (0.25/0.5/0.75/0.95) and a non-collection fallback case.Test plan
dotnet build(whole solution) — 0 warnings, 0 errorsdotnet test— 1978 passed, 0 failed onnet8.0(10 new)dotnet build -c ReleaseofCelerity.Benchmarks— cleanmain: the weekly extended-benchmark run refreshes thedev/bench-extendedgh-pages dashboard with the newFromCollectionrows (core per-PR dashboard unaffected —MemoryAllocationBenchmarkis in the extended suite)Addresses #27.
🤖 Generated with Claude Code