crypto: limit sha1 or sha512 blocks processed at once in assembly - #80917
Open
AskAlexSharov wants to merge 1 commit into
Open
crypto: limit sha1 or sha512 blocks processed at once in assembly#80917AskAlexSharov wants to merge 1 commit into
AskAlexSharov wants to merge 1 commit into
Conversation
Problem GC stop-the-world is slow if a large input is passed to sha1/sha512: worst stopping pause 67 ms for sha512 and 25 ms for sha1 on a 64 MiB buffer. Every goroutine in the process is frozen for that window. Root cause: Write passes unbounded input to block(), and assembly is not preemptible. CL 671098 bounded md5 and sha256 for golang#64417; sha1 and sha512 have the same problem and were not covered. Write now feeds block() at most maxAsmSize (64 KiB) per call, as md5 and sha256 already do. Nothing else changes: on the arches with assembly, block() is an ordinary Go function dispatching to blockAVX2/blockSHANI, so its prologue already carries the stack check the runtime poisons to request preemption. Calling it 1024 times instead of once is what lets the collector in. Stop-the-world (improved) Worst /sched/pauses/stopping/gc:seconds sample, which is the time the runtime spends waiting for every P to halt, while hashing a 64 MiB buffer. linux/amd64, EPYC 4344P, GOMAXPROCS=2. Values are histogram bucket upper bounds, so read them as "at most": before after pauses over 1ms crypto/sha1 25.166ms 197us 2 -> 0 crypto/sha512 67.109ms 1.049ms 2 -> 0 The pause is linear in what a single call is handed, about 0.4 ms of stop-the-world per MiB for sha1 and 1.0 ms per MiB for sha512, so the bound caps it rather than removing it. BenchmarkSTW, added here, reports the same effect as the wall time of one runtime.GC() that overlaps the hash: GOMAXPROCS=2 go test -run='^$' -bench=BenchmarkSTW -count=6 -benchtime=2s crypto/sha1 crypto/sha512 benchstat before.txt after.txt gcwait-sec/op gcwait-sec/op vs base sha1 24988.7µ ± 0% 319.3µ ± 13% -98.72% (p=0.002 n=6) sha512 61965.8µ ± 0% 475.8µ ± 60% -99.23% (p=0.002 n=6) The before column is flat at 0% because it is not measuring the collector at all, it is measuring the hash. The variance that appears afterwards is the collector's own work. The benchmark counts an iteration only if the hash had started before the collection finished, so a sample that overlapped nothing cannot skew the mean. Throughput (no degradations) The existing benchmarks stop at 8 KiB, below the bound, so Hash256K and Hash1M are added. That required benchmarkSize to allocate its own buffer rather than slice a fixed 8192-byte package var, matching crypto/sha256. Sampled across 8 separate process instances per side. Hash8Bytes is bimodal per-process, landing at either ~57.6ns or ~62ns depending on heap alignment in both the old and the new binary, so a single process per side attributes that coin flip to the change. for i in $(seq 8); do GOMAXPROCS=2 go test -run='^$' -bench=BenchmarkHash -count=3 -benchtime=200ms crypto/sha1 crypto/sha512 done benchstat before.txt after.txt sha1 sec/op sec/op vs base Hash8Bytes/New-2 57.70n ± 6% 57.78n ± 5% ~ (p=0.649 n=24) Hash8Bytes/Sum-2 52.50n ± 0% 52.46n ± 0% ~ (p=0.770 n=24) Hash320Bytes/New-2 174.3n ± 0% 174.3n ± 0% ~ (p=0.888 n=24) Hash320Bytes/Sum-2 171.6n ± 0% 171.7n ± 0% ~ (p=0.368 n=24) Hash1K/New-2 433.8n ± 0% 433.9n ± 0% ~ (p=0.922 n=24) Hash1K/Sum-2 432.1n ± 0% 432.0n ± 0% ~ (p=0.988 n=24) Hash8K/New-2 3.083µ ± 0% 3.083µ ± 0% ~ (p=0.896 n=24) Hash8K/Sum-2 3.083µ ± 0% 3.082µ ± 0% ~ (p=0.938 n=24) Hash256K/New-2 96.96µ ± 0% 96.94µ ± 0% ~ (p=0.657 n=24) Hash256K/Sum-2 96.90µ ± 0% 96.96µ ± 0% ~ (p=0.092 n=24) Hash1M/New-2 387.9µ ± 0% 388.0µ ± 0% ~ (p=0.214 n=24) Hash1M/Sum-2 387.8µ ± 0% 387.9µ ± 0% ~ (p=0.428 n=24) geomean 2.796µ 2.796µ +0.01% sha512 sec/op sec/op vs base Hash8Bytes/New-2 149.4n ± 0% 149.3n ± 0% ~ (p=0.849 n=24) Hash8Bytes/Sum384-2 157.6n ± 0% 158.0n ± 0% +0.25% (p=0.003 n=24) Hash8Bytes/Sum512-2 157.5n ± 0% 157.8n ± 0% ~ (p=0.106 n=24) Hash1K/New-2 1.082µ ± 0% 1.084µ ± 0% ~ (p=0.053 n=24) Hash1K/Sum384-2 1.089µ ± 0% 1.089µ ± 0% ~ (p=0.329 n=24) Hash1K/Sum512-2 1.093µ ± 0% 1.091µ ± 0% ~ (p=0.538 n=24) Hash8K/New-2 7.655µ ± 0% 7.650µ ± 0% -0.07% (p=0.045 n=24) Hash8K/Sum384-2 7.668µ ± 0% 7.652µ ± 0% -0.21% (p=0.000 n=24) Hash8K/Sum512-2 7.660µ ± 0% 7.649µ ± 0% -0.14% (p=0.000 n=24) Hash256K/New-2 240.5µ ± 0% 239.9µ ± 0% ~ (p=0.173 n=24) Hash256K/Sum384-2 240.7µ ± 0% 240.0µ ± 0% -0.32% (p=0.024 n=24) Hash256K/Sum512-2 240.2µ ± 0% 240.0µ ± 0% ~ (p=0.136 n=24) Hash1M/New-2 960.8µ ± 0% 962.4µ ± 0% ~ (p=0.519 n=24) Hash1M/Sum384-2 961.2µ ± 0% 961.0µ ± 0% ~ (p=0.814 n=24) Hash1M/Sum512-2 960.8µ ± 0% 960.5µ ± 0% ~ (p=0.878 n=24) geomean 12.44µ 12.44µ -0.04% Nothing moves outside noise, in either direction, at any size. Updates golang#64417
Contributor
|
This PR (HEAD: d4ceb67) has been imported to Gerrit for code review. Please visit Gerrit at https://go-review.googlesource.com/c/go/+/816420. Important tips:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
GC stop-the-world is slow if a large input is passed to sha1/sha512: worst
stopping pause 67 ms for sha512 and 25 ms for sha1 on a 64 MiB buffer. Every
goroutine in the process is frozen for that window.
Root cause: Write passes unbounded input to block(), and assembly is not
preemptible. CL 671098 bounded md5 and sha256 for #64417
Stop-the-world (improved)
Worst /sched/pauses/stopping/gc:seconds sample, which is the time the runtime
spends waiting for every P to halt, while hashing a 64 MiB buffer.
linux/amd64, EPYC 4344P, GOMAXPROCS=2. Values are histogram bucket upper
bounds, so read them as "at most":
The pause is linear in what a single call is handed, about 0.4 ms of
stop-the-world per MiB for sha1 and 1.0 ms per MiB for sha512, so the bound
caps it rather than removing it.
BenchmarkSTW, added here, reports the same effect as the wall time of one
runtime.GC() that overlaps the hash:
Throughput (no degradations)