Skip to content

rand: Use stdlib crypto/rand.Read on OpenBSD + Go 1.24#3464

Merged
davecgh merged 1 commit intodecred:masterfrom
jrick:arc4random
Dec 17, 2024
Merged

rand: Use stdlib crypto/rand.Read on OpenBSD + Go 1.24#3464
davecgh merged 1 commit intodecred:masterfrom
jrick:arc4random

Conversation

@jrick
Copy link
Member

@jrick jrick commented Dec 17, 2024

On Go 1.24, the standard library crypto/rand.Read method will read cryptographically secure bytes using arc4random_buf(3) instead of getentropy(2). This avoids a context switch to kernel for a system call and is much faster on small reads than both the previous stdlib crypto/rand reader, and our custom implemented userspace PRNG. It also avoids the need to provide (additional) locking to the dcrd's package global userspace PRNG.

goos: openbsd
goarch: amd64
pkg: github.com/decred/dcrd/crypto/rand
cpu: AMD Ryzen 7 5800X3D 8-Core Processor
                    │   old.txt   │               new.txt                │
                    │   sec/op    │    sec/op     vs base                │
DcrdRead/4b-8         149.7n ± 1%   124.3n ±  1%  -16.91% (p=0.000 n=10)
DcrdRead/8b-8         163.8n ± 0%   137.8n ±  1%  -15.84% (p=0.000 n=10)
DcrdRead/32b-8        243.9n ± 1%   232.2n ±  2%   -4.82% (p=0.001 n=10)
DcrdRead/512b-8       1.460µ ± 0%   1.814µ ±  0%  +24.25% (p=0.000 n=10)
DcrdRead/1KiB-8       2.770µ ± 0%   3.501µ ±  3%  +26.39% (p=0.000 n=10)
DcrdRead/4KiB-8       10.50µ ± 1%   13.55µ ±  4%  +29.06% (p=0.000 n=10)
StdlibRead/4b-8       519.5n ± 1%   124.1n ±  0%  -76.11% (p=0.000 n=10)
StdlibRead/8b-8       534.5n ± 1%   137.9n ±  1%  -74.20% (p=0.000 n=10)
StdlibRead/32b-8      624.3n ± 2%   231.9n ±  1%  -62.86% (p=0.000 n=10)
StdlibRead/512b-8     2.631µ ± 0%   1.816µ ±  0%  -30.98% (p=0.000 n=10)
StdlibRead/1KiB-8     5.196µ ± 0%   3.494µ ±  0%  -32.76% (p=0.000 n=10)
StdlibRead/4KiB-8     20.52µ ± 0%   13.52µ ±  0%  -34.12% (p=0.000 n=10)
DcrdReadPRNG/4b-8     140.6n ± 0%   124.1n ±  0%  -11.74% (p=0.000 n=10)
DcrdReadPRNG/8b-8     154.9n ± 0%   137.2n ±  1%  -11.43% (p=0.000 n=10)
DcrdReadPRNG/32b-8    228.8n ± 0%   232.0n ±  0%   +1.42% (p=0.001 n=10)
DcrdReadPRNG/512b-8   1.423µ ± 0%   1.816µ ±  0%  +27.54% (p=0.000 n=10)
DcrdReadPRNG/1KiB-8   2.721µ ± 0%   3.496µ ±  0%  +28.49% (p=0.000 n=10)
DcrdReadPRNG/4KiB-8   10.45µ ± 0%   13.52µ ±  0%  +29.35% (p=0.000 n=10)
Int32N-8              174.4n ± 1%   148.4n ±  0%  -14.88% (p=0.000 n=10)
Uint32N-8             173.6n ± 1%   147.0n ±  0%  -15.32% (p=0.000 n=10)
Int64N-8              170.9n ± 1%   146.1n ±  0%  -14.48% (p=0.000 n=10)
Uint64N-8             170.4n ± 0%   145.9n ±  1%  -14.40% (p=0.000 n=10)
Duration-8            191.1n ± 7%   159.0n ± 10%  -16.80% (p=0.000 n=10)
ShuffleSlice-8        161.5n ± 0%   144.6n ±  1%  -10.50% (p=0.000 n=10)
geomean               670.3n        542.5n        -19.07%

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

Doesn't build on go1.24rc1 !openbsd:

$ GOWORK=off go1.24rc1 test
# github.com/decred/dcrd/crypto/rand [github.com/decred/dcrd/crypto/rand.test]
.\default.go:20:17: undefined: lockingPRNG
.\uniform.go:47:10: undefined: PRNG
.\uniform.go:54:10: undefined: PRNG
.\uniform.go:61:10: undefined: PRNG
.\uniform.go:102:10: undefined: PRNG
.\uniform.go:152:10: undefined: PRNG
.\uniform.go:159:10: undefined: PRNG
.\uniform.go:168:10: undefined: PRNG
.\uniform.go:175:10: undefined: PRNG
.\uniform.go:183:10: undefined: PRNG
.\uniform.go:183:10: too many errors

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

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

I don't see any real issue with this, as the entire codebase prior to the addition of this module relied on crypto/rand.Read anyway, but I will note that this change makes the module rely on specific OS behavior now whereas it didn't before.

For example, since the interface contract for crypto/rand.Read does not dictate the blocking behavior or semantics related to gathering entropy, there is no guarantee that some future version of Go won't cease to use arc4random_buf on OpenBSD in favor of something else (e.g. perhaps not a userspace PRNG) that could cause performance regressions.

@jrick
Copy link
Member Author

jrick commented Dec 17, 2024

That would be a stupid change if they reverted it back, and not one I would anticipate, but it wouldn't break the correctness (it is still safe to concurrently call stdlib crypto/rand.Read)

@davecgh
Copy link
Member

davecgh commented Dec 17, 2024

That would be a stupid change if they reverted it back, and not one I would anticipate, but it wouldn't break the correctness (it is still safe to concurrently call stdlib crypto/rand.Read)

Agreed. It's an unlikely change and correctness is the important part, particularly that crypto/rand.Read is now guaranteed not to error. That why I said I don't really see any issue with the change. I just wanted to point out the semantics change to being more dependent on the undocumented aspects in regards to performance.

@davecgh davecgh added this to the 2.1.0 milestone Dec 17, 2024
On Go 1.24, the standard library crypto/rand.Read method will read
cryptographically secure bytes using arc4random_buf(3) instead of
getentropy(2).  This avoids a context switch to kernel for a system call and
is much faster on small reads than both the previous stdlib crypto/rand
reader, and our custom implemented userspace PRNG.  It also avoids the need to
provide (additional) locking to the dcrd's package global userspace PRNG.

goos: openbsd
goarch: amd64
pkg: github.com/decred/dcrd/crypto/rand
cpu: AMD Ryzen 7 5800X3D 8-Core Processor
                    │   old.txt   │               new.txt                │
                    │   sec/op    │    sec/op     vs base                │
DcrdRead/4b-8         149.7n ± 1%   124.3n ±  1%  -16.91% (p=0.000 n=10)
DcrdRead/8b-8         163.8n ± 0%   137.8n ±  1%  -15.84% (p=0.000 n=10)
DcrdRead/32b-8        243.9n ± 1%   232.2n ±  2%   -4.82% (p=0.001 n=10)
DcrdRead/512b-8       1.460µ ± 0%   1.814µ ±  0%  +24.25% (p=0.000 n=10)
DcrdRead/1KiB-8       2.770µ ± 0%   3.501µ ±  3%  +26.39% (p=0.000 n=10)
DcrdRead/4KiB-8       10.50µ ± 1%   13.55µ ±  4%  +29.06% (p=0.000 n=10)
StdlibRead/4b-8       519.5n ± 1%   124.1n ±  0%  -76.11% (p=0.000 n=10)
StdlibRead/8b-8       534.5n ± 1%   137.9n ±  1%  -74.20% (p=0.000 n=10)
StdlibRead/32b-8      624.3n ± 2%   231.9n ±  1%  -62.86% (p=0.000 n=10)
StdlibRead/512b-8     2.631µ ± 0%   1.816µ ±  0%  -30.98% (p=0.000 n=10)
StdlibRead/1KiB-8     5.196µ ± 0%   3.494µ ±  0%  -32.76% (p=0.000 n=10)
StdlibRead/4KiB-8     20.52µ ± 0%   13.52µ ±  0%  -34.12% (p=0.000 n=10)
DcrdReadPRNG/4b-8     140.6n ± 0%   124.1n ±  0%  -11.74% (p=0.000 n=10)
DcrdReadPRNG/8b-8     154.9n ± 0%   137.2n ±  1%  -11.43% (p=0.000 n=10)
DcrdReadPRNG/32b-8    228.8n ± 0%   232.0n ±  0%   +1.42% (p=0.001 n=10)
DcrdReadPRNG/512b-8   1.423µ ± 0%   1.816µ ±  0%  +27.54% (p=0.000 n=10)
DcrdReadPRNG/1KiB-8   2.721µ ± 0%   3.496µ ±  0%  +28.49% (p=0.000 n=10)
DcrdReadPRNG/4KiB-8   10.45µ ± 0%   13.52µ ±  0%  +29.35% (p=0.000 n=10)
Int32N-8              174.4n ± 1%   148.4n ±  0%  -14.88% (p=0.000 n=10)
Uint32N-8             173.6n ± 1%   147.0n ±  0%  -15.32% (p=0.000 n=10)
Int64N-8              170.9n ± 1%   146.1n ±  0%  -14.48% (p=0.000 n=10)
Uint64N-8             170.4n ± 0%   145.9n ±  1%  -14.40% (p=0.000 n=10)
Duration-8            191.1n ± 7%   159.0n ± 10%  -16.80% (p=0.000 n=10)
ShuffleSlice-8        161.5n ± 0%   144.6n ±  1%  -10.50% (p=0.000 n=10)
geomean               670.3n        542.5n        -19.07%
@davecgh davecgh merged commit cad8fef into decred:master Dec 17, 2024
@jrick jrick deleted the arc4random branch December 17, 2024 21:29
@davecgh davecgh modified the milestones: 2.1.0, 2.0.6 Feb 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants