Skip to content

Conversation

@sharwell
Copy link
Contributor

@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
@ghost ghost 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