Skip to content

Commit

Permalink
runtime: don't call lockOSThread for every cgo call
Browse files Browse the repository at this point in the history
For a trivial benchmark with a do-nothing cgo call:

name    old time/op  new time/op  delta
Call-4  64.5ns ± 7%  63.0ns ± 6%  -2.25%  (p=0.027 n=20+16)

Because Windows uses the cgocall mechanism to make system calls,
and passes arguments in a struct held in the m,
we need to do the lockOSThread/unlockOSThread in that code.

Because deferreturn was getting a nosplit stack overflow error,
change it to avoid calling typedmemmove.

Updates #21827.

Change-Id: I9b1d61434c44faeb29805b46b409c812c9acadc2
Reviewed-on: https://go-review.googlesource.com/64070
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Crawshaw <crawshaw@golang.org>
  • Loading branch information
ianlancetaylor committed Sep 22, 2017
1 parent 9daee93 commit 332719f
Show file tree
Hide file tree
Showing 5 changed files with 82 additions and 35 deletions.
80 changes: 47 additions & 33 deletions src/runtime/cgocall.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
// runtime.cgocall(_cgo_Cfunc_f, frame), where _cgo_Cfunc_f is a
// gcc-compiled function written by cgo.
//
// runtime.cgocall (below) locks g to m, calls entersyscall
// so as not to block other goroutines or the garbage collector,
// and then calls runtime.asmcgocall(_cgo_Cfunc_f, frame).
// runtime.cgocall (below) calls entersyscall so as not to block
// other goroutines or the garbage collector, and then calls
// runtime.asmcgocall(_cgo_Cfunc_f, frame).
//
// runtime.asmcgocall (in asm_$GOARCH.s) switches to the m->g0 stack
// (assumed to be an operating system-allocated stack, so safe to run
Expand Down Expand Up @@ -104,13 +104,9 @@ func cgocall(fn, arg unsafe.Pointer) int32 {
racereleasemerge(unsafe.Pointer(&racecgosync))
}

// Lock g to m to ensure we stay on the same stack if we do a
// cgo callback. In case of panic, unwindm calls endcgo.
lockOSThread()
mp := getg().m
mp.ncgocall++
mp.ncgo++
mp.incgo = true

// Reset traceback.
mp.cgoCallers[0] = 0
Expand All @@ -130,7 +126,14 @@ func cgocall(fn, arg unsafe.Pointer) int32 {
// and then re-enter the "system call" reusing the PC and SP
// saved by entersyscall here.
entersyscall(0)

mp.incgo = true
errno := asmcgocall(fn, arg)

// Call endcgo before exitsyscall because exitsyscall may
// reschedule us on to a different M.
endcgo(mp)

exitsyscall(0)

// From the garbage collector's perspective, time can move
Expand All @@ -145,8 +148,8 @@ func cgocall(fn, arg unsafe.Pointer) int32 {
// GC by forcing them to stay live across this time warp.
KeepAlive(fn)
KeepAlive(arg)
KeepAlive(mp)

endcgo(mp)
return errno
}

Expand All @@ -158,8 +161,6 @@ func endcgo(mp *m) {
if raceenabled {
raceacquire(unsafe.Pointer(&racecgosync))
}

unlockOSThread() // invalidates mp
}

// Call from C back to Go.
Expand All @@ -171,6 +172,12 @@ func cgocallbackg(ctxt uintptr) {
exit(2)
}

// The call from C is on gp.m's g0 stack, so we must ensure
// that we stay on that M. We have to do this before calling
// exitsyscall, since it would otherwise be free to move us to
// a different M. The call to unlockOSThread is in unwindm.
lockOSThread()

// Save current syscall parameters, so m.syscall can be
// used again if callback decide to make syscall.
syscall := gp.m.syscall
Expand All @@ -186,6 +193,10 @@ func cgocallbackg(ctxt uintptr) {

cgocallbackg1(ctxt)

// At this point unlockOSThread has been called.
// The following code must not change to a different m.
// This is enforced by checking incgo in the schedule function.

gp.m.incgo = true
// going back to cgo call
reentersyscall(savedpc, uintptr(savedsp))
Expand Down Expand Up @@ -321,32 +332,35 @@ func cgocallbackg1(ctxt uintptr) {
}

func unwindm(restore *bool) {
if !*restore {
return
}
// Restore sp saved by cgocallback during
// unwind of g's stack (see comment at top of file).
mp := acquirem()
sched := &mp.g0.sched
switch GOARCH {
default:
throw("unwindm not implemented")
case "386", "amd64", "arm", "ppc64", "ppc64le", "mips64", "mips64le", "s390x", "mips", "mipsle":
sched.sp = *(*uintptr)(unsafe.Pointer(sched.sp + sys.MinFrameSize))
case "arm64":
sched.sp = *(*uintptr)(unsafe.Pointer(sched.sp + 16))
}
if *restore {
// Restore sp saved by cgocallback during
// unwind of g's stack (see comment at top of file).
mp := acquirem()
sched := &mp.g0.sched
switch GOARCH {
default:
throw("unwindm not implemented")
case "386", "amd64", "arm", "ppc64", "ppc64le", "mips64", "mips64le", "s390x", "mips", "mipsle":
sched.sp = *(*uintptr)(unsafe.Pointer(sched.sp + sys.MinFrameSize))
case "arm64":
sched.sp = *(*uintptr)(unsafe.Pointer(sched.sp + 16))
}

// Call endcgo to do the accounting that cgocall will not have a
// chance to do during an unwind.
//
// In the case where a Go call originates from C, ncgo is 0
// and there is no matching cgocall to end.
if mp.ncgo > 0 {
endcgo(mp)
// Call endcgo to do the accounting that cgocall will not have a
// chance to do during an unwind.
//
// In the case where a Go call originates from C, ncgo is 0
// and there is no matching cgocall to end.
if mp.ncgo > 0 {
endcgo(mp)
}

releasem(mp)
}

releasem(mp)
// Undo the call to lockOSThread in cgocallbackg.
// We must still stay on the same m.
unlockOSThread()
}

// called from assembly
Expand Down
12 changes: 11 additions & 1 deletion src/runtime/panic.go
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,17 @@ func freedefer(d *_defer) {
unlock(&sched.deferlock)
})
}
*d = _defer{}

// These lines used to be simply `*d = _defer{}` but that
// started causing a nosplit stack overflow via typedmemmove.
d.siz = 0
d.started = false
d.sp = 0
d.pc = 0
d.fn = nil
d._panic = nil
d.link = nil

pp.deferpool[sc] = append(pp.deferpool[sc], d)
}

Expand Down
6 changes: 6 additions & 0 deletions src/runtime/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -2221,6 +2221,12 @@ func schedule() {
execute(_g_.m.lockedg.ptr(), false) // Never returns.
}

// We should not schedule away from a g that is executing a cgo call,
// since the cgo call is using the m's g0 stack.
if _g_.m.incgo {
throw("schedule: in cgo")
}

top:
if sched.gcwaiting != 0 {
gcstopm()
Expand Down
3 changes: 2 additions & 1 deletion src/runtime/runtime2.go
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,8 @@ func extendRandom(r []byte, n int) {
}
}

// deferred subroutine calls
// A _defer holds an entry on the list of deferred calls.
// If you add a field here, add code to clear it in freedefer.
type _defer struct {
siz int32
started bool
Expand Down
16 changes: 16 additions & 0 deletions src/runtime/syscall_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,8 @@ const _LOAD_LIBRARY_SEARCH_SYSTEM32 = 0x00000800
//go:linkname syscall_loadsystemlibrary syscall.loadsystemlibrary
//go:nosplit
func syscall_loadsystemlibrary(filename *uint16) (handle, err uintptr) {
lockOSThread()
defer unlockOSThread()
c := &getg().m.syscall

if useLoadLibraryEx {
Expand Down Expand Up @@ -126,6 +128,8 @@ func syscall_loadsystemlibrary(filename *uint16) (handle, err uintptr) {
//go:linkname syscall_loadlibrary syscall.loadlibrary
//go:nosplit
func syscall_loadlibrary(filename *uint16) (handle, err uintptr) {
lockOSThread()
defer unlockOSThread()
c := &getg().m.syscall
c.fn = getLoadLibrary()
c.n = 1
Expand All @@ -141,6 +145,8 @@ func syscall_loadlibrary(filename *uint16) (handle, err uintptr) {
//go:linkname syscall_getprocaddress syscall.getprocaddress
//go:nosplit
func syscall_getprocaddress(handle uintptr, procname *byte) (outhandle, err uintptr) {
lockOSThread()
defer unlockOSThread()
c := &getg().m.syscall
c.fn = getGetProcAddress()
c.n = 2
Expand All @@ -156,6 +162,8 @@ func syscall_getprocaddress(handle uintptr, procname *byte) (outhandle, err uint
//go:linkname syscall_Syscall syscall.Syscall
//go:nosplit
func syscall_Syscall(fn, nargs, a1, a2, a3 uintptr) (r1, r2, err uintptr) {
lockOSThread()
defer unlockOSThread()
c := &getg().m.syscall
c.fn = fn
c.n = nargs
Expand All @@ -167,6 +175,8 @@ func syscall_Syscall(fn, nargs, a1, a2, a3 uintptr) (r1, r2, err uintptr) {
//go:linkname syscall_Syscall6 syscall.Syscall6
//go:nosplit
func syscall_Syscall6(fn, nargs, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2, err uintptr) {
lockOSThread()
defer unlockOSThread()
c := &getg().m.syscall
c.fn = fn
c.n = nargs
Expand All @@ -178,6 +188,8 @@ func syscall_Syscall6(fn, nargs, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2, err ui
//go:linkname syscall_Syscall9 syscall.Syscall9
//go:nosplit
func syscall_Syscall9(fn, nargs, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1, r2, err uintptr) {
lockOSThread()
defer unlockOSThread()
c := &getg().m.syscall
c.fn = fn
c.n = nargs
Expand All @@ -189,6 +201,8 @@ func syscall_Syscall9(fn, nargs, a1, a2, a3, a4, a5, a6, a7, a8, a9 uintptr) (r1
//go:linkname syscall_Syscall12 syscall.Syscall12
//go:nosplit
func syscall_Syscall12(fn, nargs, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12 uintptr) (r1, r2, err uintptr) {
lockOSThread()
defer unlockOSThread()
c := &getg().m.syscall
c.fn = fn
c.n = nargs
Expand All @@ -200,6 +214,8 @@ func syscall_Syscall12(fn, nargs, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11,
//go:linkname syscall_Syscall15 syscall.Syscall15
//go:nosplit
func syscall_Syscall15(fn, nargs, a1, a2, a3, a4, a5, a6, a7, a8, a9, a10, a11, a12, a13, a14, a15 uintptr) (r1, r2, err uintptr) {
lockOSThread()
defer unlockOSThread()
c := &getg().m.syscall
c.fn = fn
c.n = nargs
Expand Down

0 comments on commit 332719f

Please sign in to comment.