Open nvanbenschoten opened 9 months ago
Your code could be simplified but it sounds reasonable. A third alternative could be to have assembly check some magic preemption boolean and return early if it is preempted. Something along the lines of:
func block(state *[8]uint32, buf []byte) (hashed uint)
// ...
for h := p[:len(p) &^ (chunk - 1)]; len(h) >= chunk; h = h[block(d, h):] {}
Proposal Details
As discussed in https://github.com/golang/go/issues/24543#issuecomment-376581064, assembly code is non-preemptible. Non-preemptible goroutines can delay stop-the-world pauses (after all other goroutines have been paused), meaning that they can directly contribute to workload tail latency if timed poorly. This makes libraries which perform unbounded amounts of work in assembly loops risky for use in latency sensitive applications.
One way to mitigate this risk would be to bound the amount of work that any single call into an assembly routine performs on a case-by-case basis.
The
crypto
libraries provide a handful of examples where this technique could be applied. For example, incrypto/sha256
, the following diff limits the non-preemptible section of a sha256 hash to somewhere around 250µs:We've seen in CockroachDB that this change combined with an almost identical one in
crypto/md5
halves database p99 latency during backup uploads to AWS S3.Alternative 1
One simple alternative is to document this risk and push the responsibility to bound the amount of non-preemptible to the users of these libraries.
However, that's not a great solution, as the direct callers of these hashing libraries are often buried deep inside of other libraries which applications cannot control (examples in the aws sdk: here, here, and here).
Alternative 2
The
Safe-points everywhere for non-cooperative goroutine preemption
design proposed a method to annotate safe preemption points in assembly code.To my knowledge, this has never been implemented. If it was, it would provide an alternative to this proposal, though it would still require changes around each relevant use of assembly.
cc. @aclements you've thought about this problem more than anyone else. Does this seem like a reasonable proposal?