runtime: don't call lockOSThread for every syscall call on Windows

Windows syscall.SyscallN currently calls lockOSThread for every syscall.
This can be expensive and produce unnecessary context switches,
especially when the syscall is called frequently under high contention.

The lockOSThread was necessary to ensure that cgocall wouldn't
reschedule the goroutine to a different M, as the syscall return values
are reported back in the M struct.

This CL instructs cgocall to copy the syscall return values into the
the M that will see the caller on return, so the caller no longer needs
to call lockOSThread.

Updates #58336.

Cq-Include-Trybots: luci.golang.try:gotip-windows-arm64,gotip-windows-amd64-longtest
Change-Id: If6644fd111dbacab74e7dcee2afa18ca146735da
Reviewed-on: https://go-review.googlesource.com/c/go/+/562915
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Auto-Submit: Emmanuel Odeke <emmanuel@orijtech.com>
Reviewed-by: Than McIntosh <thanm@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Pratt <mpratt@google.com>
Run-TryBot: Emmanuel Odeke <emmanuel@orijtech.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/runtime/cgocall.go b/src/runtime/cgocall.go
index 05fa47158afe53f8c6e7c08d7ea148530f344f1d..e81852f6074ffd9e8c8abe560e4accb9361336d8 100644 (file)
--- a/src/runtime/cgocall.go
+++ b/src/runtime/cgocall.go
@@ -181,8 +181,14 @@ func cgocall(fn, arg unsafe.Pointer) int32 {
 
        osPreemptExtExit(mp)
 
+       // Save current syscall parameters, so m.winsyscall can be
+       // used again if callback decide to make syscall.
+       winsyscall := mp.winsyscall
+
        exitsyscall()
 
+       getg().m.winsyscall = winsyscall
+
        // Note that raceacquire must be called only after exitsyscall has
        // wired this M to a P.
        if raceenabled {
@@ -297,9 +303,9 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) {
 
        checkm := gp.m
 
-       // Save current syscall parameters, so m.syscall can be
+       // Save current syscall parameters, so m.winsyscall can be
        // used again if callback decide to make syscall.
-       syscall := gp.m.syscall
+       winsyscall := gp.m.winsyscall
 
        // entersyscall saves the caller's SP to allow the GC to trace the Go
        // stack. However, since we're returning to an earlier stack frame and
@@ -340,7 +346,7 @@ func cgocallbackg(fn, frame unsafe.Pointer, ctxt uintptr) {
        // going back to cgo call
        reentersyscall(savedpc, uintptr(savedsp))
 
-       gp.m.syscall = syscall
+       gp.m.winsyscall = winsyscall
 }
 
 func cgocallbackg1(fn, frame unsafe.Pointer, ctxt uintptr) {

diff --git a/src/runtime/nonwindows_stub.go b/src/runtime/nonwindows_stub.go
index e420acf1852b28c3c3937ef5157625d55af7a3a2..859ae48004ff0692f53560fd280c0107ed5a4922 100644 (file)
--- a/src/runtime/nonwindows_stub.go
+++ b/src/runtime/nonwindows_stub.go
@@ -21,3 +21,8 @@ func osRelax(relax bool) {}
 // enableWER is called by setTraceback("wer").
 // Windows Error Reporting (WER) is only supported on Windows.
 func enableWER() {}
+
+// winlibcall is not implemented on non-Windows systems,
+// but it is used in non-OS-specific parts of the runtime.
+// Define it as an empty struct to avoid wasting stack space.
+type winlibcall struct{}

diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
index 0074b9358abdd8a7befd8ef8941c1215e6c8453e..d65e0c91f42a867b8a1bd777b125822fa1854327 100644 (file)
--- a/src/runtime/os_windows.go
+++ b/src/runtime/os_windows.go
@@ -214,6 +214,8 @@ func asmstdcall(fn unsafe.Pointer)
 
 var asmstdcallAddr unsafe.Pointer
 
+type winlibcall libcall
+
 func windowsFindfunc(lib uintptr, name []byte) stdFunction {
        if name[len(name)-1] != 0 {
                throw("usage")

diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go
index 4a7ad271726d8fecbca1339742e52785dfd802c1..c335f8c9d0db5573cf1f93d318b4973de7192bff 100644 (file)
--- a/src/runtime/runtime2.go
+++ b/src/runtime/runtime2.go
@@ -612,11 +612,11 @@ type m struct {
 
        // these are here because they are too large to be on the stack
        // of low-level NOSPLIT functions.
-       libcall   libcall
-       libcallpc uintptr // for cpu profiler
-       libcallsp uintptr
-       libcallg  guintptr
-       syscall   libcall // stores syscall parameters on windows
+       libcall    libcall
+       libcallpc  uintptr // for cpu profiler
+       libcallsp  uintptr
+       libcallg   guintptr
+       winsyscall winlibcall // stores syscall parameters on windows
 
        vdsoSP uintptr // SP for traceback while in VDSO call (0 if not in call)
        vdsoPC uintptr // PC for traceback while in VDSO call

diff --git a/src/runtime/syscall_windows.go b/src/runtime/syscall_windows.go
index 0b085835634284cdd2dc00e53fa8decc3016cdbc..09762aed51cf7c966207feee038cc0b9e1b1f2db 100644 (file)
--- a/src/runtime/syscall_windows.go
+++ b/src/runtime/syscall_windows.go
@@ -500,16 +500,18 @@ func syscall_SyscallN(fn uintptr, args ...uintptr) (r1, r2, err uintptr) {
        }
 
        // The cgocall parameters are stored in m instead of in
-       // the stack because the stack can move during if fn
+       // the stack because the stack can move during fn if it
        // calls back into Go.
-       lockOSThread()
-       defer unlockOSThread()
-       c := &getg().m.syscall
+       c := &getg().m.winsyscall
        c.fn = fn
        c.n = uintptr(len(args))
        if c.n != 0 {
                c.args = uintptr(noescape(unsafe.Pointer(&args[0])))
        }
        cgocall(asmstdcallAddr, unsafe.Pointer(c))
+       // cgocall may reschedule us on to a different M,
+       // but it copies the return values into the new M's
+       // so we can read them from there.
+       c = &getg().m.winsyscall
        return c.r1, c.r2, c.err
 }