Skip to content

Commit

Permalink
runtime: don't call lockOSThread for every syscall call on Windows
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
qmuntal authored and gopherbot committed Mar 26, 2024
1 parent 3659b87 commit 1f354a6
Show file tree
Hide file tree
Showing 5 changed files with 27 additions and 12 deletions.
12 changes: 9 additions & 3 deletions src/runtime/cgocall.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 5 additions & 0 deletions src/runtime/nonwindows_stub.go
Expand Up @@ -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{}
2 changes: 2 additions & 0 deletions src/runtime/os_windows.go
Expand Up @@ -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")
Expand Down
10 changes: 5 additions & 5 deletions src/runtime/runtime2.go
Expand Up @@ -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
Expand Down
10 changes: 6 additions & 4 deletions src/runtime/syscall_windows.go
Expand Up @@ -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
}

0 comments on commit 1f354a6

Please sign in to comment.