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

crypto/rand: Read performance improvements #33256

Closed
CAFxX opened this issue Jul 24, 2019 · 3 comments
Closed

crypto/rand: Read performance improvements #33256

CAFxX opened this issue Jul 24, 2019 · 3 comments
Labels
FrozenDueToAge Performance WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@CAFxX
Copy link
Contributor

CAFxX commented Jul 24, 2019

Wrapping crypto/rand.Reader in a bufio.Reader amortizes the syscall overhead significantly, especially for smaller reads (number of bytes per read after the /). This is obviously a quick and dirty test - I don't think it's a good idea to bluntly wrap it like this, but it's just to show that there are potential gains to be had by buffering reads from getrandom:

name                 old time/op  new time/op  delta
CryptoRandRead/1-4    921ns ± 0%    23ns ± 0%  -97.51%  (p=0.008 n=5+5)
CryptoRandRead/4-4    922ns ± 0%    40ns ± 0%  -95.64%  (p=0.008 n=5+5)
CryptoRandRead/8-4    922ns ± 0%    64ns ± 0%  -93.05%  (p=0.008 n=5+5)
CryptoRandRead/16-4   923ns ± 0%   111ns ± 0%  -87.97%  (p=0.008 n=5+5)
CryptoRandRead/32-4   923ns ± 0%   205ns ± 0%  -77.76%  (p=0.008 n=5+5)
CryptoRandRead/64-4  1.27µs ± 0%  0.39µs ± 0%  -68.95%  (p=0.016 n=5+4)

While on topic, it may be worthwhile to point out that short reads from getrandom don't need to be discarded, as they can be still used:

func getRandomBatch(p []byte) (ok bool) {
n, err := unix.GetRandom(p, 0)
return n == len(p) && err == nil
}

Environment:

cafxx@instance-1:~$ uname -a
Linux instance-1 5.0.0-1010-gcp #10-Ubuntu SMP Tue Jun 25 10:29:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux
cafxx@instance-1:~$ go version
go version go1.12.7 linux/amd64

Benchmark

package main

import (
    "crypto/rand"
    "io"
    "testing"
    "strconv"
    "bufio"
)

var sz = []int{1, 4, 8, 16, 32, 64}
var cr, bcr io.Reader

func init() {
    cr = rand.Reader
    bcr = bufio.NewReader(rand.Reader)
}

func BenchmarkCryptoRandRead(b *testing.B) {
    rand.Reader = cr
    b.Run("raw", benchmark)
    rand.Reader = bcr
    b.Run("buffered", benchmark)
}

func benchmark(b *testing.B) {
    defer func() {
        rand.Reader = cr
    }()
    for _, s := range sz {
        buf := make([]byte, s)
        b.Run(strconv.Itoa(s), func (b *testing.B) {
            for i := 0; i < b.N; i++ {
                if n, err := rand.Read(buf); n != s || err != nil {
                    b.Fatal("error in rand.Read()")
                }
            }
        })
        _ = buf
    }
}
@takeyourhatoff
Copy link

See: #16593

@julieqiu
Copy link
Member

Is this a dupe of #16593?

@julieqiu julieqiu added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 29, 2019
@CAFxX
Copy link
Contributor Author

CAFxX commented Aug 2, 2019

Yup will add a comment there.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge Performance WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

5 participants