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.

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

Use optimized hash creation methods on .NET 5+ #67999

Merged
merged 2 commits into from
Apr 28, 2023

Conversation

sharwell
Copy link
Member

@sharwell sharwell commented Apr 27, 2023

Implements the changes suggested in #67995 (comment)

Review commit-by-commit highly recommended.

Supersedes #67998
Fixes #67995

@sharwell sharwell marked this pull request as ready for review April 27, 2023 16:14
@sharwell sharwell requested a review from a team as a code owner April 27, 2023 16:14
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead labels Apr 27, 2023
@sharwell sharwell force-pushed the incremental-hash-2 branch from bb641e1 to d086859 Compare April 27, 2023 17:03
@sharwell sharwell force-pushed the incremental-hash-2 branch from d086859 to 48e9cc5 Compare April 27, 2023 19:05
return From(hash);
#elif NET5_0_OR_GREATER
using var pooledHash = s_incrementalHashPool.GetPooledObject();
Span<byte> buffer = stackalloc byte[SharedPools.ByteBufferSize];
Copy link
Member

Choose a reason for hiding this comment

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

4K is a much larger buffer than we typically like to stackalloc. It might be fine for your scenarios, just calling it out. We have a rough guideline of not stackalloc'ing more than 1K.

Copy link
Member

Choose a reason for hiding this comment

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

(Also, keep in mind this whole buffer will be zero'd unless you're using SkipLocalsInit.)

@stephentoub
Copy link
Member

Implements the changes suggested in #67995 (comment)

Glad it was helpful. Curious what the net impact looks like if there are any perf tests that surface it...

@sharwell sharwell merged commit ce2b2c7 into dotnet:main Apr 28, 2023
@sharwell sharwell deleted the incremental-hash-2 branch April 28, 2023 16:16
@ghost ghost added this to the Next milestone Apr 28, 2023
@Cosifne Cosifne modified the milestones: Next, 17.7 P2 May 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-IDE untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Checksum's usage of a native hash algorithm resulting in thread starvation
4 participants