]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: don't acquirem() in vgetrandom unless necessary
authorJason A. Donenfeld <Jason@zx2c4.com>
Tue, 1 Oct 2024 20:19:23 +0000 (22:19 +0200)
committerJason Donenfeld <Jason@zx2c4.com>
Wed, 2 Oct 2024 17:00:39 +0000 (17:00 +0000)
I noticed in pprof that acquirem() was a bit of a hotspot. It turns out
that we can use the same trick that runtime.rand() does, and only
acquirem if we're doing something non-nosplit -- in this case, getting a
new state -- but otherwise just do getg().m, which is safe because we're
inside runtime and don't call split functions.

cpu: 11th Gen Intel(R) Core(TM) i7-11850H @ 2.50GHz
                     │   sec/op    │   sec/op     vs base               │
ParallelGetRandom-16   2.651n ± 4%   2.416n ± 7%  -8.87% (p=0.001 n=10)
                     │     B/s      │     B/s       vs base               │
ParallelGetRandom-16   1.406Gi ± 4%   1.542Gi ± 6%  +9.72% (p=0.001 n=10)

Change-Id: Iae075f4e298b923e499cd01adfabacab725a8684
Reviewed-on: https://go-review.googlesource.com/c/go/+/616738
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/runtime/vgetrandom_linux.go

index c938909503dd464a18fdea60e8317b4e779ea072..a6ec4b701c1a8fcbf94a79b08fa30012bc8f856d 100644 (file)
@@ -87,19 +87,26 @@ func vgetrandom(p []byte, flags uint32) (ret int, supported bool) {
                return -1, false
        }
 
-       mp := acquirem()
+       // We use getg().m instead of acquirem() here, because always taking
+       // the lock is slightly more expensive than not always taking the lock.
+       // However, we *do* require that m doesn't migrate elsewhere during the
+       // execution of the vDSO. So, we exploit two details:
+       //   1) Asynchronous preemption is aborted when PC is in the runtime.
+       //   2) Most of the time, this function only calls vgetrandom1(), which
+       //      does not have a preamble that synchronously preempts.
+       // We do need to take the lock when getting a new state for m, but this
+       // is very much the slow path, in the sense that it only ever happens
+       // once over the entire lifetime of an m. So, a simple getg().m suffices.
+       mp := getg().m
+
        if mp.vgetrandomState == 0 {
+               mp.locks++
                state := vgetrandomGetState()
+               mp.locks--
                if state == 0 {
-                       releasem(mp)
                        return -1, false
                }
                mp.vgetrandomState = state
        }
-
-       ret = vgetrandom1(unsafe.SliceData(p), uintptr(len(p)), flags, mp.vgetrandomState, vgetrandomAlloc.stateSize)
-       supported = true
-
-       releasem(mp)
-       return
+       return vgetrandom1(unsafe.SliceData(p), uintptr(len(p)), flags, mp.vgetrandomState, vgetrandomAlloc.stateSize), true
 }