Skip to content

Optimize ImmutableHashSet<T>.IsProperSubsetOf to avoid unnecessary allocations - #127368

Open
aw0lid wants to merge 5 commits into
dotnet:mainfrom
aw0lid:fix-immutablehashset-IsProperSubsetOf-allocs
Open

aw0lid wants to merge 5 commits into
dotnet:mainfrom
aw0lid:fix-immutablehashset-IsProperSubsetOf-allocs

Conversation

@aw0lid

@aw0lid aw0lid commented Apr 24, 2026 •

Copy link
Copy Markdown
Contributor

Part of #127279

Summary

ImmutableHashSet<T>.IsProperSubsetOf always creates a new intermediate HashSet<T> for the other collection, leading to avoidable allocations and GC pressure, especially for large datasets

Optimization Logic

  • O(1) Pre-Scan: Immediately returns false if other is an ICollection with a smaller or equal Count. By performing this validation upfront, the need for tracking variables like matches and extraFound is eliminated, as any complete match is now mathematically guaranteed to be a proper subset.

  • Fast-Path Pattern Matching: Detects ImmutableHashSet<T> and HashSet<T> to bypass intermediate allocations.

  • Comparer Guard: Validates EqualityComparer compatibility before triggering fast paths to ensure logical consistency.

  • Short-Circuit Validation: Re-validates Count within specialized paths for an immediate exit before $O(n)$ enumeration.

  • Don't repeat your self: reused SetEqualsWithHashset and SetEqualsWithImmutableHashset methods to avoid code duplication while ensuring we leverage the $O(1)$ lookup efficiency when other is a Hashset<T>.

  • Zero-Allocation Execution: Direct iteration over compatible collections, eliminating the costly new HashSet<T>(other) fallback.

  • Deferred fallback: Reserves the expensive allocation solely for general IEnumerable types.

Click to expand Benchmark Source Code
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Order;
using BenchmarkDotNet.Running;
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;

namespace ImmutableHashSetBenchmarks
{
    [MemoryDiagnoser]
    [Orderer(SummaryOrderPolicy.FastestToSlowest)]
    [RankColumn]
    public class ImmutableHashSetIsProperSubsetOfBenchmark
    {
        private ImmutableHashSet<int> _sourceSet = null!;
        private ImmutableHashSet<int> _immutableLarger = null!;
        private HashSet<int> _bclHashSetLarger = null!;
        private List<int> _listLarger = null!;
        private int[] _arrayLarger = null!;

        private ImmutableHashSet<int> _immutableSmaller = null!;
        private ImmutableHashSet<int> _immutableSameCount = null!;
        
        private HashSet<int> _bclHashSetLargerDiffComparer = null!;

        private List<int> _listWithDuplicatesButProper = null!;
        private ImmutableHashSet<int> _emptySource = null!;
        private List<int> _listSameElementsWithDuplicates = null!;

        [Params(100000)]
        public int Size { get; set; }

        [GlobalSetup]
        public void Setup()
        {
            var elements = Enumerable.Range(0, Size).ToList();
            var largerElements = Enumerable.Range(0, Size + 10).ToList();
            var smallerElements = Enumerable.Range(0, Size - 10).ToList();
            var reverseComparer = new ReverseComparer<int>();

            _sourceSet = ImmutableHashSet.CreateRange(elements);
            
            _immutableLarger = ImmutableHashSet.CreateRange(largerElements);
            _bclHashSetLarger = new HashSet<int>(largerElements);
            _listLarger = largerElements;
            _arrayLarger = largerElements.ToArray();

            _immutableSmaller = ImmutableHashSet.CreateRange(smallerElements);
            _immutableSameCount = ImmutableHashSet.CreateRange(elements);

            _bclHashSetLargerDiffComparer = new HashSet<int>(largerElements, reverseComparer);

            _listWithDuplicatesButProper = elements.Concat(new[] { Size + 1, Size + 1, Size + 1 }).ToList();
            _emptySource = ImmutableHashSet<int>.Empty;
            _listSameElementsWithDuplicates = elements.Concat(elements).ToList();
        }

        #region Fast Path: Same Type and Comparer (Optimized)

        [Benchmark(Description = "ImmutableHashSet (Proper Subset - O(N))")]
        public bool Case_ImmutableHashSet_Proper() => _sourceSet.IsProperSubsetOf(_immutableLarger);

        [Benchmark(Description = "BCL HashSet (Proper Subset - O(N))")]
        public bool Case_BclHashSet_Proper() => _sourceSet.IsProperSubsetOf(_bclHashSetLarger);

        #endregion

        #region Early Exit: Count Check (O(1))

        [Benchmark(Description = "Empty Source (O(1) Check)")]
        public bool Case_EmptySource_Proper() => _emptySource.IsProperSubsetOf(_bclHashSetLarger);

        [Benchmark(Description = "List (Same Elements with Duplicates - Not Proper)")]
        public bool Case_List_Duplicates_NotProper() => _sourceSet.IsProperSubsetOf(_listSameElementsWithDuplicates);

        [Benchmark(Description = "Early Exit (Other is Smaller)")]
        public bool Case_SmallerCount() => _sourceSet.IsProperSubsetOf(_immutableSmaller);

        [Benchmark(Description = "Early Exit (Same Count - Cannot be Proper)")]
        public bool Case_SameCount() => _sourceSet.IsProperSubsetOf(_immutableSameCount);

        #endregion

        #region Fallback Path: Non-Set or Different Comparer

        [Benchmark(Description = "List (Proper - Fallback to HashSet)")]
        public bool Case_List_Proper() => _sourceSet.IsProperSubsetOf(_listLarger);

        [Benchmark(Description = "Array (Proper - Fallback to HashSet)")]
        public bool Case_Array_Proper() => _sourceSet.IsProperSubsetOf(_arrayLarger);

        [Benchmark(Description = "HashSet (Different Comparer - Force Fallback)")]
        public bool Case_HashSet_DiffComparer() => _sourceSet.IsProperSubsetOf(_bclHashSetLargerDiffComparer);

        [Benchmark(Description = "List with Duplicates (Proper Subset)")]
        public bool Case_List_Duplicates_Proper() => _sourceSet.IsProperSubsetOf(_listWithDuplicatesButProper);

        #endregion
    }

    public class ReverseComparer<T> : IEqualityComparer<T> where T : IComparable<T>
    {
        public bool Equals(T? x, T? y) => x?.CompareTo(y) == 0;
        public int GetHashCode(T? obj) => obj?.GetHashCode() ?? 0;
    }

    public class Program
    {
        public static void Main(string[] args)
        {
            BenchmarkRunner.Run<ImmutableHashSetIsProperSubsetOfBenchmark>();
        }
    }
}
Click to expand Benchmark Results

Benchmark Results (Before Optimization)

Method Size Mean Error StdDev Rank Gen0 Gen1 Gen2 Allocated
'Empty Source (O(1) Check)' 100000 3.020 ns 0.0522 ns 0.0463 ns 1 - - - -
'List (Same Elements with Duplicates - Not Proper)' 100000 2,253,302.064 ns 43,996.1710 ns 83,707.2944 ns 2 85.9375 85.9375 85.9375 3605725 B
'BCL HashSet (Proper Subset - O(N))' 100000 7,143,590.897 ns 55,965.9984 ns 52,350.6297 ns 3 62.5000 62.5000 62.5000 1738869 B
'Array (Proper - Fallback to HashSet)' 100000 7,191,053.721 ns 60,956.8792 ns 54,036.6857 ns 3 70.3125 70.3125 70.3125 1738731 B
'Early Exit (Other is Smaller)' 100000 7,252,623.997 ns 127,108.7726 ns 112,678.6161 ns 3 70.3125 70.3125 70.3125 1738868 B
'Early Exit (Same Count - Cannot be Proper)' 100000 7,254,561.478 ns 73,899.0961 ns 57,695.5534 ns 3 78.1250 78.1250 78.1250 1738874 B
'List (Proper - Fallback to HashSet)' 100000 8,628,967.714 ns 103,114.2276 ns 96,453.1125 ns 4 78.1250 78.1250 78.1250 1738861 B
'List with Duplicates (Proper Subset)' 100000 8,979,029.530 ns 124,859.7911 ns 104,263.5805 ns 4 78.1250 78.1250 78.1250 1738861 B
'HashSet (Different Comparer - Force Fallback)' 100000 9,236,192.453 ns 94,804.2142 ns 88,679.9208 ns 4 78.1250 78.1250 78.1250 1738861 B
'ImmutableHashSet (Proper Subset - O(N))' 100000 14,980,671.116 ns 155,763.4001 ns 145,701.1812 ns 5 78.1250 78.1250 78.1250 1738897 B

Benchmark Results (After Optimization)

Method Size Mean Error StdDev Rank Gen0 Gen1 Gen2 Allocated
'Empty Source (O(1) Check)' 100000 1.769 ns 0.0372 ns 0.0348 ns 1 - - - -
'Early Exit (Same Count - Cannot be Proper)' 100000 2.261 ns 0.0409 ns 0.0363 ns 2 - - - -
'Early Exit (Other is Smaller)' 100000 2.401 ns 0.0800 ns 0.0748 ns 2 - - - -
'List (Same Elements with Duplicates - Not Proper)' 100000 2,226,632.494 ns 44,488.2292 ns 94,808.0507 ns 3 82.0313 82.0313 82.0313 3605636 B
'Array (Proper - Fallback to HashSet)' 100000 4,128,214.156 ns 40,633.6128 ns 36,020.6393 ns 4 62.5000 62.5000 62.5000 1738710 B
'HashSet (Different Comparer - Force Fallback)' 100000 4,310,472.716 ns 39,782.7924 ns 33,220.4335 ns 5 70.3125 70.3125 70.3125 1738810 B
'BCL HashSet (Proper Subset - O(N))' 100000 5,622,467.743 ns 43,428.7884 ns 36,265.0052 ns 6 - - - -
'List (Proper - Fallback to HashSet)' 100000 6,933,851.438 ns 51,688.7203 ns 43,162.4224 ns 7 62.5000 62.5000 62.5000 1738734 B
'List with Duplicates (Proper Subset)' 100000 7,455,423.480 ns 92,394.8283 ns 81,905.6087 ns 8 70.3125 70.3125 70.3125 1738817 B
'ImmutableHashSet (Proper Subset - O(N))' 100000 13,207,980.537 ns 130,270.0225 ns 115,480.9819 ns 9 - - - -

Performance Analysis Summary (100,000 Elements)

Case / Method Before (ns) After (ns) Speedup Ratio Memory Improvement
Early Exit (Other is Smaller) 7,252,623 2.401 ~3,020,667x -100% (Zero Alloc)
Early Exit (Same Count) 7,254,561 2.261 ~3,208,563x -100% (Zero Alloc)
Empty Source 3.020 1.769 1.71x Zero Alloc
BCL HashSet (Proper Subset) 7,143,590 5,622,467 1.27x -100% (Zero Alloc)
List (Duplicates - Not Proper) 2,253,302 2,226,632 1.01x Stable (3.6 MB)
Array (Fallback to HashSet) 7,191,053 4,128,214 1.74x Stable (1.7 MB)
List (Proper - Fallback) 8,628,967 6,933,851 1.24x Stable (1.7 MB)
List with Duplicates (Proper) 8,979,029 7,455,423 1.20x Stable (1.7 MB)
HashSet (Diff Comparer) 9,236,192 4,310,472 2.14x Stable (1.7 MB)
ImmutableHashSet (Proper) 14,980,671 13,207,980 1.13x -100% (Zero Alloc)

✅ Unit Tests Added

Added unit tests for IsProperSubsetOf to cover various edge cases and ensure the correctness of the new logic:

  • Mismatched Comparers: Validated behavior when comparing sets with different comparers (e.g., Ordinal vs. OrdinalIgnoreCase).
  • Duplicate Elements: Verified ICollection<T> logic to ensure that collections with duplicates are handled correctly by the early exit (Count <= origin.Count).
  • Empty Set Scenarios: Confirmed expected behavior when either the origin or the target collection is empty.
  • Equality Logic: Covered cases where logical equality might differ from reference equality in comparers.

@dotnet-policy-service dotnet-policy-service Bot added the community-contribution Indicates that the PR has been added by a community member label Apr 24, 2026
@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@aw0lid
aw0lid force-pushed the fix-immutablehashset-IsProperSubsetOf-allocs branch from e482a29 to dc4b4b2 Compare May 4, 2026 21:56
@aw0lid
aw0lid marked this pull request as ready for review May 5, 2026 08:55
@aw0lid

aw0lid commented Jun 22, 2026

Copy link
Copy Markdown
Contributor Author

Hi everyone, just a gentle follow-up on this PR
CC/ @eiriktsarpalis, @MihaZupan

@github-actions

github-actions Bot commented Jul 19, 2026 •

Copy link
Copy Markdown
Contributor

Workflow state for the Holistic Review Orchestrator.

{
  "version": 5,
  "last_dispatched_commit": "d5da4276084a2615ce50353e6cc19ec7e3ea30c6",
  "last_dispatched_base_ref": "main",
  "last_dispatched_base_sha": "b5d84ecbf4d92d63329e63025099a4a063a65285",
  "last_reviewed_commit": "d5da4276084a2615ce50353e6cc19ec7e3ea30c6",
  "last_reviewed_base_ref": "main",
  "last_reviewed_base_sha": "b5d84ecbf4d92d63329e63025099a4a063a65285",
  "last_recorded_worker_run_id": "29725956930",
  "review_attempt_commit": "",
  "review_attempt_base_ref": "",
  "review_attempt_count": 0,
  "max_review_attempts": 5,
  "review_history_format": "holistic-review-disclosure-v1",
  "review_history": [
    {
      "commit": "e76ce57566c99c85d2e6274e76509f57f9ee61d2",
      "review_id": 4730525611
    },
    {
      "commit": "d5da4276084a2615ce50353e6cc19ec7e3ea30c6",
      "review_id": 4733113020
    }
  ]
}

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holistic Review

Motivation: ImmutableHashSet<T>.IsProperSubsetOf always materialized a new HashSet<T> from other, incurring an O(n) allocation even in cases that could be resolved in O(1) (e.g. other has fewer or equal elements) or without allocation (e.g. other is already an ImmutableHashSet<T>/HashSet<T> with a compatible comparer). This adds avoidable GC pressure for large sets. This PR is part of #127279.

Approach: The public IsProperSubsetOf now short-circuits the empty-origin case (return other.Any()) up front. The private IsProperSubsetOf(other, origin) gains a switch (other) that mirrors the existing SetEquals fast-path structure: for ImmutableHashSet<T>, HashSet<T>, and ICollection<T> it performs an O(1) count check (Count <= origin.Count => false, correct because a proper subset requires the superset to be strictly larger, and this holds even for collections with duplicates since dedup only shrinks the count). When the comparer matches, it reuses SetEqualsWithImmutableHashset/SetEqualsWithHashset to verify containment without allocating. Only the general IEnumerable fallback still allocates a HashSet<T>. Tests cover mismatched comparers, duplicate ICollection elements, and empty-set scenarios.

Summary: The refactor is correct and consistent with the established SetEquals fast-path pattern in this file. The count checks are logically sound: because origin is a set, a proper-subset relationship requires other's distinct count to strictly exceed origin.Count, and Count <= origin.Count on the raw collection safely rejects (dedup can only reduce the count). The comparer guard via EqualityComparer<IEqualityComparer<T>>.Default.Equals correctly restricts the zero-alloc containment path to compatible comparers; mismatched-comparer cases fall through to the allocating new HashSet<T>(other, origin.EqualityComparer) path, preserving prior semantics. The empty-origin handling moved to the public entry point is equivalent, and the private method's fallback still behaves correctly for empty origin when reached via the Builder (Count <= 0 count checks and the subsequent containment/count logic). I found no correctness regressions. The only issue is a cosmetic indentation glitch on the switch (other) line (flagged inline), which formatting validation may reject. Verdict: LGTM once the formatting nit is addressed.

Detailed Findings

No functional issues found. One cosmetic finding is noted inline: the switch (other) statement at line 940 is mis-indented; run dotnet format to align it with the enclosing block so the C# formatting check passes.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 94.9 AIC · ⌖ 10.6 AIC · ⊞ 10K

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holistic Review

Motivation: ImmutableHashSet<T>.IsProperSubsetOf always materialized a new HashSet<T> from other, incurring an O(n) allocation even in cases resolvable in O(1) (e.g. other has fewer or equal elements) or without allocation (e.g. other is already an ImmutableHashSet<T>/HashSet<T> with a compatible comparer). This adds avoidable GC pressure for large sets. This PR is part of #127279.

Approach: The public IsProperSubsetOf short-circuits the empty-origin case (return other.Any()) up front. The private IsProperSubsetOf(other, origin) gains a switch (other) mirroring the existing SetEquals fast-path structure: for ImmutableHashSet<T>, HashSet<T>, and ICollection<T> it performs an O(1) count check (Count <= origin.Count => false, correct because a proper subset requires the superset to be strictly larger, and this holds even for collections with duplicates since dedup only shrinks the count). When the comparer matches, it reuses SetEqualsWithImmutableHashset/SetEqualsWithHashset to verify containment without allocating. Only the general IEnumerable fallback still allocates a HashSet<T>. Tests cover mismatched comparers, duplicate ICollection elements, and empty-set scenarios.

Summary: The latest commit makes a single change: it corrects the mis-indentation of the switch (other) statement that was flagged in the prior review. It is now aligned (12 spaces) with the enclosing block, so the C# formatting check should pass. No functional code changed in this increment, and the cumulative assessment is unchanged: the refactor is correct, consistent with the established SetEquals fast-path pattern, and introduces no correctness regressions. The count checks are sound, the comparer guard correctly restricts the zero-alloc containment path to compatible comparers, and mismatched-comparer cases fall through to the allocating path preserving prior semantics. The empty-origin handling moved to the public entry point is equivalent. Verdict: LGTM.

Detailed Findings

No functional issues found. The only outstanding item from the prior review—the mis-indented switch (other) line—has been fixed in this increment.

Assessment History

  • review 4730525611 reviewed commit e76ce57: verdict LGTM (once the formatting nit was addressed). Current verdict: LGTM. Assessment changed only in that the previously-flagged cosmetic indentation issue is now resolved by commit d5da427; motivation, approach, and risk are otherwise unchanged.

Note

This review was generated by this repository's Holistic Review agentic workflow to complement the built-in Copilot review.

Generated by Holistic Review · 44.7 AIC · ⌖ 9.67 AIC · ⊞ 10K

@jeffhandley jeffhandley added this to the 12.0.0 milestone Aug 17, 2026
@aw0lid

aw0lid commented Sep 22, 2026

Copy link
Copy Markdown
Contributor Author

Just a quick follow-up on this PR.
I’d appreciate any feedback when you get a chance. Thanks!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-System.Collections community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants