runtime: refactor finalizer goroutine status

Use an atomic.Uint32 to represent the state of finalizer goroutine.
fingStatus will only be changed to fingWake in non fingWait state,
so it is safe to set fingRunningFinalizer status in runfinq.

name            old time/op  new time/op  delta
Finalizer-8      592µs ± 4%   561µs ± 1%  -5.22%  (p=0.000 n=10+10)
FinalizerRun-8   694ns ± 6%   675ns ± 7%    ~     (p=0.059 n=9+8)

Change-Id: I7e4da30cec98ce99f7d8cf4c97f933a8a2d1cae1
Reviewed-on: https://go-review.googlesource.com/c/go/+/400134
Reviewed-by: Joedian Reid <joedian@golang.org>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
Reviewed-by: Michael Pratt <mpratt@google.com>

1 files changed, 28 insertions, 21 deletions

diff --git a/src/runtime/mfinal.go b/src/runtime/mfinal.go
index 9de364c260..ef11b7df96 100644
--- a/src/runtime/mfinal.go
+++ b/src/runtime/mfinal.go
@@ -29,13 +29,23 @@ type finblock struct {
 	fin     [(_FinBlockSize - 2*goarch.PtrSize - 2*4) / unsafe.Sizeof(finalizer{})]finalizer
 }
 
+var fingStatus atomic.Uint32
+
+// finalizer goroutine status.
+const (
+	fingUninitialized uint32 = iota
+	fingCreated       uint32 = 1 << (iota - 1)
+	fingRunningFinalizer
+	fingWait
+	fingWake
+)
+
 var finlock mutex  // protects the following variables
 var fing *g        // goroutine that runs finalizers
 var finq *finblock // list of finalizers that are to be executed
 var finc *finblock // cache of free blocks
 var finptrmask [_FinBlockSize / goarch.PtrSize / 8]byte
-var fingwait bool
-var fingwake bool
+
 var allfin *finblock // list of all blocks
 
 // NOTE: Layout known to queuefinalizer.
@@ -126,8 +136,8 @@ func queuefinalizer(p unsafe.Pointer, fn *funcval, nret uintptr, fint *_type, ot
 	f.fint = fint
 	f.ot = ot
 	f.arg = p
-	fingwake = true
 	unlock(&finlock)
+	fingStatus.Or(fingWake)
 }
 
 //go:nowritebarrier
@@ -141,29 +151,27 @@ func iterate_finq(callback func(*funcval, unsafe.Pointer, uintptr, *_type, *ptrt
 }
 
 func wakefing() *g {
-	var res *g
-	lock(&finlock)
-	if fingwait && fingwake {
-		fingwait = false
-		fingwake = false
-		res = fing
+	if ok := fingStatus.CompareAndSwap(fingCreated|fingWait|fingWake, fingCreated); ok {
+		return fing
 	}
-	unlock(&finlock)
-	return res
+	return nil
 }
 
-var (
-	fingCreate  uint32
-	fingRunning bool
-)
-
 func createfing() {
 	// start the finalizer goroutine exactly once
-	if fingCreate == 0 && atomic.Cas(&fingCreate, 0, 1) {
+	if fingStatus.Load() == fingUninitialized && fingStatus.CompareAndSwap(fingUninitialized, fingCreated) {
 		go runfinq()
 	}
 }
 
+func finalizercommit(gp *g, lock unsafe.Pointer) bool {
+	unlock((*mutex)(lock))
+	// fingStatus should be modified after fing is put into a waiting state
+	// to avoid waking fing in running state, even if it is about to be parked.
+	fingStatus.Or(fingWait)
+	return true
+}
+
 // This is the goroutine that runs all of the finalizers
 func runfinq() {
 	var (
@@ -182,8 +190,7 @@ func runfinq() {
 		fb := finq
 		finq = nil
 		if fb == nil {
-			fingwait = true
-			goparkunlock(&finlock, waitReasonFinalizerWait, traceEvGoBlock, 1)
+			gopark(finalizercommit, unsafe.Pointer(&finlock), waitReasonFinalizerWait, traceEvGoBlock, 1)
 			continue
 		}
 		argRegs = intArgRegs
@@ -244,9 +251,9 @@ func runfinq() {
 				default:
 					throw("bad kind in runfinq")
 				}
-				fingRunning = true
+				fingStatus.Or(fingRunningFinalizer)
 				reflectcall(nil, unsafe.Pointer(f.fn), frame, uint32(framesz), uint32(framesz), uint32(framesz), &regs)
-				fingRunning = false
+				fingStatus.And(^fingRunningFinalizer)
 
 				// Drop finalizer queue heap references
 				// before hiding them from markroot.