Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Replace modulo with fastmodulo in HashSet and Hashtable #1989

Closed
1 of 2 tasks
danmoseley opened this issue Jan 22, 2020 · 7 comments
Closed
1 of 2 tasks

Replace modulo with fastmodulo in HashSet and Hashtable #1989

danmoseley opened this issue Jan 22, 2020 · 7 comments
Labels
area-System.Collections help wanted [up-for-grabs] Good issue for external contributors tenet-performance Performance related issue
Milestone
Future

Comments

@danmoseley
Copy link
Member

danmoseley commented Jan 22, 2020 •
edited by stephentoub
Loading

The performance optimization by @benaadams in Dictionary<K,V> in dotnet/coreclr#27299 should be applied to

  • Hashset<T>
  • Hashtable
@danmoseley danmoseley added tenet-performance Performance related issue help wanted [up-for-grabs] Good issue for external contributors labels Jan 22, 2020
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Collections untriaged New issue has not been triaged by the area owner labels Jan 22, 2020
@GrabYourPitchforks
Copy link
Member

HashSet<T> should also get the non-randomized string hash code optimization that Dictionary<TKey, TValue> enjoys. That optimization should pay healthy dividends.

@eanova
Copy link
Contributor

eanova commented Jan 23, 2020

I took a peek at this last night. Few test errors but I think I can resolve those and push a PR by tomorrow for HashSet.

@danmoseley
Copy link
Member Author

Sounds good. It would be good to share perf numbers before/after in your PR: https://github.com/dotnet/performance/blob/master/docs/benchmarking-workflow-dotnet-runtime.md

@danmoseley
Copy link
Member Author

That doc just got updated for 50 and this new repo, so if you see any glitch in it, let us know!

@eanova
Copy link
Contributor

eanova commented Jan 23, 2020

I'll post back here if I get stuck on the performance tests.

@danmoseley
Copy link
Member Author

OK! @adamsitnik owns that excellent doc.

@eanova eanova mentioned this issue Jan 24, 2020
Closed
This was referenced May 27, 2020
Merged
Closed
@stephentoub stephentoub assigned stephentoub and unassigned eanova May 29, 2020
@stephentoub stephentoub mentioned this issue May 29, 2020
Merged
@stephentoub stephentoub removed their assignment Jun 1, 2020
@eiriktsarpalis eiriktsarpalis added this to the Future milestone Jun 24, 2020
@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Jun 24, 2020
@jkotas
Copy link
Member

jkotas commented Jul 15, 2021

#55467 was unsuccessful attempt to update Hashtable to use fastmod.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area-System.Collections help wanted [up-for-grabs] Good issue for external contributors tenet-performance Performance related issue
Projects
None yet
Milestone
Future
Development

No branches or pull requests
8 participants
@GrabYourPitchforks @stephentoub @eiriktsarpalis @danmoseley @jkotas @layomia @Dotnet-GitSync-Bot @eanova