Use nearest prime sizing for collection trimming

[tannergooding]

Dictionary<TKey, TValue> and HashSet<T> currently use the growth-oriented cached prime table for TrimExcess, which can retain nearly 20% more storage than requested. Select the nearest valid prime for compaction while preserving the existing cached-prime behavior for construction, EnsureCapacity, and automatic growth.

The shared prime-selection implementation now lives under libraries/Common rather than being duplicated by MetadataLoadContext.

Internal prime search

These are microbenchmarks of the computed-prime fallback beyond the precomputed cache. They measure the helper directly, not end-user TrimExcess performance. Visiting only 6k +/- 1 candidates and reusing the square-root limit improved representative searches by 33-36% locally:

Minimum	Before	After
1,000	67.51 ns	43.22 ns
5,001,234	2.185 us	1.458 us
1,000,000,000	30.04 us	19.85 us

TrimExcess

To isolate destination sizing from other implementation differences, these benchmarks use the PR implementation for both cases and compact to either the previous cached-prime target or the new nearest-prime target. Copy throughput is mixed within a few percent while allocation falls with the retained capacity:

Collection	Count	Cached target	Nearest target	Time ratio	Allocated before	Allocated after
Dictionary<int, int>	100,000	108,631	100,003	1.02x	2.07 MB	1.91 MB
Dictionary<int, int>	1,000,000	1,162,687	1,000,003	1.03x	22.18 MB	19.07 MB
Dictionary<int, int>	5,001,234	5,999,471	5,001,251	0.98x	114.43 MB	95.39 MB
HashSet<int>	100,000	108,631	100,003	1.06x	1.66 MB	1.53 MB
HashSet<int>	1,000,000	1,162,687	1,000,003	0.98x	17.74 MB	15.26 MB
HashSet<int>	5,001,234	5,999,471	5,001,251	0.99x	91.54 MB	76.31 MB

At 5,001,234 entries, this avoids 998,220 slots -- approximately 19.04 MiB for Dictionary<int, int> or 15.23 MiB for HashSet<int> across the bucket and entry arrays.

Addresses the TrimExcess sizing problem discussed in #132051.

Note

This pull request description was drafted with GitHub Copilot.

[azure-pipelines]

Azure Pipelines:
Successfully started running 3 pipeline(s).
13 pipeline(s) were filtered out due to trigger conditions.
There may be pipelines that require an authorized user to comment /azp run to run.

[dotnet-policy-service]

Tagging subscribers to this area: @dotnet/area-system-collections
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

Updates internal hash-table sizing helpers so Dictionary<TKey, TValue>.TrimExcess(int) and HashSet<T>.TrimExcess(int) choose a tighter “nearest valid prime” capacity (rather than the growth-oriented cached prime table), reducing retained storage after trimming while keeping the existing cached-prime behavior for construction/growth paths.

Changes:

• Added a new prime-sizing path (GetPrimeAtLeast) and optimized prime search in System.Collections.HashHelpers.
• Switched Dictionary/HashSet TrimExcess(int) to use the tighter prime sizing and refactored trim rehashing into compacting resize helpers.
• Added regression tests asserting the new TrimExcess capacity selection behavior.

Show a summary per file

File	Description
src/libraries/System.Reflection.MetadataLoadContext/src/System/Reflection/TypeLoading/General/HashHelpers.cs	Optimizes prime selection in MetadataLoadContext’s local HashHelpers.
src/libraries/System.Private.CoreLib/src/System/Collections/HashHelpers.cs	Adds GetPrimeAtLeast and updates prime-testing/search logic used by core collections.
src/libraries/System.Private.CoreLib/src/System/Collections/Generic/HashSet.cs	Uses tighter prime sizing for TrimExcess and introduces compacting resize/copy helpers.
src/libraries/System.Private.CoreLib/src/System/Collections/Generic/Dictionary.cs	Uses tighter prime sizing for TrimExcess and introduces compacting resize helper.
src/libraries/System.Collections/tests/Generic/HashSet/HashSet.Generic.Tests.cs	Adds test coverage for the new HashSet.TrimExcess sizing behavior.
src/libraries/System.Collections/tests/Generic/Dictionary/Dictionary.Generic.Tests.cs	Adds test coverage for the new Dictionary.TrimExcess sizing behavior.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 17/17 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 18/18 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 18/18 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 18/18 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

• Files reviewed: 18/18 changed files
• Comments generated: 0 new
• Review effort level: Lite

[verelpode]

Unfortunately, there are still major problems with the current version of this PR:

• It is a semi-breaking change. A complex change in performance characteristics. Two different performance models (switches when TrimExcess(int) is invoked - an unexpected change in the behavior of TrimExcess(int)).
• It causes performance regression. Increased (worsened) collision rate. Increased average chain length.
• It introduces unjustifiable complexity in exchange for performance regression instead of improvement. It makes Dictionary slower and more complex. It uses a special prime number calculation algorithm for no benefit in return (or what benefit it does deliver can be far more easily obtained without using this special new algorithm).
• This PR changes the API, not in terms of the method signature, but in terms of what the method does. This PR causes TrimExcess(int) to trigger a switch to a different performance model, and this is unexpected and undocumented and not part of the original definition of TrimExcess(int), therefore it is effectively a change to the API.
• The API is confusing or counterintuitive. Two overloads of TrimExcess where one was expected (TrimExcess(int) is most likely defective or at least difficult to understand what it is supposed to do, and this PR makes it even more confusing).
• It is risky. The added complexity + speed reduction make the ramifications difficult to safely approve, especially considering the broad impact that this PR has, because it modifies HashHelpers. It impacts multiple classes - all classes that use HashHelpers are potentially impacted or at least in need of review/checking. Furthermore, all preexisting callers of Dictionary.TrimExcess are potentially impacted in a somewhat complex manner.
• There is also the risk of people complaining about the performance regression that this PR causes.
• It is controversial because it increases the collision rate and average chain length and introduces two different performance models depending on whether TrimExcess(int) is invoked or not.
• Non-solution: This PR does not solve the original problem that prompted the creation of this PR. The new TrimExcess(int) in this PR does nothing (returns silently) when invoked in the original scenario (converting an ICollection<KeyValuePair> to Dictionary with little or no wastage in Dictionary.Capacity). It is unsuitable for the task.

As the situation stands today, the current version of this PR is clearly and definitely not something that should be approved.

[Copilot]

Review details

• Files reviewed: 19/19 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

Review details

Suppressed comments (1)

src/libraries/Common/src/System/Collections/HashHelpers.cs:146

• GetPrimeAtLeastCore uses an unbounded while (true) and increments candidate without any overflow/termination check. For large min values (e.g., int.MaxValue from public Dictionary.TrimExcess(int) / HashSet.TrimExcess(int)), candidate += increment can overflow to a negative value and the loop will never terminate, potentially hanging the caller.

• Files reviewed: 18/18 changed files
• Comments generated: 0 new
• Review effort level: Lite

[vojtechgadurek]

Hi all, I was just wondering, why we do not make the prime array denser? If we increase the size 10 times, we should lower the overhead to 1/10 of the current.

The performance hit for searching the correct prime should be neglible to the overall cost of copy of all elements (if using linear search).

It may be noticable, if someone is allocating large dictionaries and then not using them.

[tannergooding]

If we increase the size 10 times, we should lower the overhead to 1/10 of the current.

This comes at other cost, including pessimizing the common case of automatic growth in favor of the less common case of explicit user sizing.

There's a lot of primes, with the largest gap being (when accounting for the HashPrime of 101) being roughly 318 between any two primes between [0, int.MaxValue] (there's roughly 465m total primes in that range, not accounting for the hash prime filtering). On the other hand, finding the next largest prime via computation is also not terribly expensive and the more expensive cost is actually allocating+copying, particularly for larger counts (somewhere around 1500x Int128 is where I measured it start to be non-impactful).

It may be noticable, if someone is allocating large dictionaries and then not using them.

Users intentionally over-allocating isn't a concern. That's on them to fix if its problematic.

[tannergooding]

Unfortunately, there are still major problems with the current version of this PR:

I appreciate the feedback and while I've already addressed many of these statements on the other thread, I'll summarize the general response here as well.

Given effectively random hashes (which is expected for most inputs and is how most GetHashCode implementations are built), any given dictionary follows the standard 1 + ((count / capacity) / 2) algorithm (where count / capacity is effectively the load factor) for the expected average number of lookups required. So when count is approx equal to capacity, you get an expected 1.5 comparisons per lookup. When capacity is roughly double count, then it becomes 1.25; then 1.125 at 4x the count, and so on. You need capacity to be roughly 10x the count to hit 1.05.

The existing lookup table entries averages about a 1.2 growth between prime entries and so the best case scenario when using exclusively the lookup table is around 1.417 average lookups (in contrast to the 1.5 base quoted above). We could have the algorithm always pick capacity * 1.2 but then that is misleading to users and what they request explicitly and so its safer and simpler to just do the "nearest prime" and let them scale it themselves if desired.

Then, while the 1 vs 1.4 vs 1.5 vs ... average lookups follow big-O notation, that isn't necessarily meaningful and there are other aspects to be considered. This includes that at smaller n, you're actually making memory accesses more random and so likely hurting perf because you're less likely to hit the cache. There is also the aspect of GC pressure, cost of allocating, zeroing, and copying that memory, etc. So the real world consideration is very different from just looking at the raw theoretical math.

Using Guid as an example (and noting that other types will have different measurements/costs), we save 4.96 MiB at 1m entries while at 4.2m entries we see 24.4 MiB in allocating savings. The perf for these however ends up being about net neutral with it being 2-4% faster in optimal cases and around 3-6% slower in the worst case scenarios; while distinct runs fluctuate around 5% on average just due to things like loop alignment and memory alignment that you happen to be given by the GC. Showcasing that this isn't likely a meaningful scenario and the winnings are really the size savings with little impact to real world perf for that theoretical 0.08 savings on average lookups done.

This then changes nothing with regards to what users could already see/experience. That is, the perf characteristics here are entirely dependent on the count to capacity ratio. Even with the prime table picking larger than necessary primes, the actual count could end up being a load capacity that is close to 1.0 and therefore could already see that 1.5 average. If a user actually cared, they'd need to use FrozenDictionary (to try and get 1.05x-1.25x, which is its target range based on count) or to explicitly specify a larger capacity; in which case this PR improves their ability to optimize for their scenario as it gives them what they ask for.

---

There are few things that are true wins or true losses, most are balanced based on many factors.

This PR in particular has one easy win in improving the prime lookup perf, at the cost of more code and algorithmic complexity.

It then has a different win in allowing TrimExcess to respect the user specified size (rather than choosing something that is in up to 1.2x larger), but at the cost of meaning users explicitly sizing their dictionaries need to better ensure they're picking an appropriate size -- which again is something they already had to consider for many specified capacities, just not all of them.

[tannergooding]

This is also notably not something for .NET 11, it'd be for .NET 12; and so it's something we have essentially a full 15 months to get feedback around and to tune if needed, including if we simply want to always choose capacity * 1.2x to maintain the existing lookup table distancing.