Rewrite HashSet's implementation based on Dictionary's by stephentoub · Pull Request #37180 · dotnet/runtime (original) (raw)

Fixes #37111
Contributes to #1989

This moves HashSet into corelib, and then effectively deletes HashSet's data structure and replaces it with the one used by Dictionary, then updated for the differences (e.g. just a value rather than a key and a value). HashSet used to have basically the same implementation, but Dictionary has evolved significantly and HashSet hasn't; this brings them to basic parity on implementation.

Based on perf tests, I veered away from Dictionary's implementation in a few places (e.g. a goto-based implementation in the core find method led to a significant regression for Int32-based Contains operations), and we should follow-up to understand whether Dictionary should be changed as well, or why there's a difference between the two. @benaadams, if you have some time, it'd probably worth looking at this again; maybe you'll get different numbers than I did.

Functionally, bringing over Dictionary's implementation yields a few notable changes, namely that Remove and Clear no longer invalidate enumerations. The tests have been updated accordingly.

With HashSet now in corelib, I also updated two Dictionary uses I found in corelib that were using Dictionary as a set and switched them to use HashSet.

Running the dotnet/performance perf tests:

dotnet run -c Release -f net5.0 --filter System.Collections.*.HashSet --corerun d:\coreclrtest\master\corerun.exe d:\coreclrtest\pr\corerun.exe --join

cc: @benaadams, @danmosemsft, @GrabYourPitchforks, @eiriktsarpalis, @layomia