search

dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.24k stars 1.35k forks source link

Clean up HashSet.cs copy #7340

Open danmoseley opened 2 years ago

danmoseley commented 2 years ago

We have a partial copy of HashSet as it existed in the .NET Framework circa 2008 or something, with the following comment: https://github.com/dotnet/msbuild/blob/6a79376cc50e5f0d829475adc418e7078776ccb7/src/Build/Collections/RetrievableEntryHashSet/HashSet.cs#L19-L39

The rationale for this odd pattern of fragmented dead code was to diff for future updates to HashSet -- but the live copy of HashSet here has since changed so substantially that it can likely hardly be diffed at all.

As a matter of healthy cleanup, I suggest to delete all the dead code.

There have also been several improvements to HashSet since the copy was done, for perf reasons - either obvious ones or to produce better codegen. It would be nice to copy them (again, without bringing dead code) for some small perf wins. I expect this will be fairly easy to do by eyeballing them side by side.

elachlan commented 2 years ago

I have been attempting this. The CORE version was re-written in June of 2020. I think it might be best to take that version and refactor it with the changes from MSBUILD.

I will let you know how I go.

rainersigwald commented 2 years ago

I wouldn't spend a lot of time on this: we don't have any indication that the current implementation is causing a bottleneck today, and changes are likely to be hard to review due to the overall complexity of the class.