From 1f354a60ff7e8e047e4dfecb1033a20ee8c266dc Mon Sep 17 00:00:00 2001 From: qmuntal Date: Mon, 11 Mar 2024 10:49:44 +0100 Subject: [PATCH] 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 Auto-Submit: Emmanuel Odeke Reviewed-by: Than McIntosh TryBot-Result: Gopher Robot Reviewed-by: Michael Pratt Run-TryBot: Emmanuel Odeke LUCI-TryBot-Result: Go LUCI --- src/runtime/cgocall.go | 12 +++++++++--- src/runtime/nonwindows_stub.go | 5 +++++ src/runtime/os_windows.go | 2 ++ src/runtime/runtime2.go | 10 +++++----- src/runtime/syscall_windows.go | 10 ++++++---- 5 files changed, 27 insertions(+), 12 deletions(-) diff --git a/src/runtime/cgocall.go b/src/runtime/cgocall.go index 05fa47158a..e81852f607 100644 --- 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 e420acf185..859ae48004 100644 --- 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 0074b9358a..d65e0c91f4 100644 --- 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 4a7ad27172..c335f8c9d0 100644 --- 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 0b08583563..09762aed51 100644 --- 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 }