From: Jason A. Donenfeld <Jason@zx2c4.com>
Date: Tue, 1 Oct 2024 20:19:23 +0000 (+0200)
Subject: runtime: don't acquirem() in vgetrandom unless necessary
X-Git-Tag: go1.24rc1~767
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=8c269479eddb8a620e4f4581a520fdf5a931d648;p=gostls13.git

runtime: don't acquirem() in vgetrandom unless necessary

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>
---

diff --git a/src/runtime/vgetrandom_linux.go b/src/runtime/vgetrandom_linux.go
index c938909503..a6ec4b701c 100644
--- a/src/runtime/vgetrandom_linux.go
+++ b/src/runtime/vgetrandom_linux.go
@@ -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
 }