runtime: respect timeout in semasleep on Darwin

semasleep on Darwin was refactored in https://golang.org/cl/118736 to
use the pthread_cond_timedwait function from libc. The new code
incorrectly assumed that pthread_cond_timedwait took a timeout relative
to the current time, when it in fact it takes a timeout specified in
absolute time. semasleep thus specified a timeout well in the past,
causing it to immediately exceed the timeout and spin hot. This was the
source of a large performance hit to CockroachDB (#26019).

Adjust semasleep to instead call pthread_cond_timedwait_relative_np,
which properly interprets its timeout parameter as relative to the
current time.

pthread_cond_timedwait_relative_np is non-portable, but using
pthread_cond_timedwait correctly would require two calls to
gettimeofday: one in the runtime package to convert the relative timeout
to absolute time, then another in the pthread library to convert back to
a relative offset [0], as the Darwin kernel expects a relative offset.

[0]: https://opensource.apple.com/source/libpthread/libpthread-301.30.1/src/pthread_cond.c.auto.html

Fix #26019.

Change-Id: I1a8c2429f79513b43d2b256365cd9166d235af8b
Reviewed-on: https://go-review.googlesource.com/120635
Reviewed-by: Keith Randall <khr@golang.org>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>

diff --git a/src/runtime/os_darwin.go b/src/runtime/os_darwin.go
index 5019b9423c81084aaf97353e9f59e85bdab7c2dc..ff375004a38052612fea675cb86565006f231250 100644 (file)
--- a/src/runtime/os_darwin.go
+++ b/src/runtime/os_darwin.go
@@ -45,7 +45,7 @@ func semasleep(ns int64) int32 {
                if ns >= 0 {
                        var t timespec
                        t.set_nsec(ns)
-                       err := pthread_cond_timedwait(&mp.cond, &mp.mutex, &t)
+                       err := pthread_cond_timedwait_relative_np(&mp.cond, &mp.mutex, &t)
                        if err == _ETIMEDOUT {
                                pthread_mutex_unlock(&mp.mutex)
                                return -1

diff --git a/src/runtime/sys_darwin.go b/src/runtime/sys_darwin.go
index ef5aef19299370f6f9051b76020c2ccf6308a23f..f0d0815903fe9b6057af514d56891fcd3c4c5485 100644 (file)
--- a/src/runtime/sys_darwin.go
+++ b/src/runtime/sys_darwin.go
@@ -287,10 +287,10 @@ func pthread_cond_wait_trampoline()
 
 //go:nosplit
 //go:cgo_unsafe_args
-func pthread_cond_timedwait(c *pthreadcond, m *pthreadmutex, t *timespec) int32 {
-       return libcCall(unsafe.Pointer(funcPC(pthread_cond_timedwait_trampoline)), unsafe.Pointer(&c))
+func pthread_cond_timedwait_relative_np(c *pthreadcond, m *pthreadmutex, t *timespec) int32 {
+       return libcCall(unsafe.Pointer(funcPC(pthread_cond_timedwait_relative_np_trampoline)), unsafe.Pointer(&c))
 }
-func pthread_cond_timedwait_trampoline()
+func pthread_cond_timedwait_relative_np_trampoline()
 
 //go:nosplit
 //go:cgo_unsafe_args
@@ -348,7 +348,7 @@ func closeonexec(fd int32) {
 //go:cgo_import_dynamic libc_pthread_mutex_unlock pthread_mutex_unlock "/usr/lib/libSystem.B.dylib"
 //go:cgo_import_dynamic libc_pthread_cond_init pthread_cond_init "/usr/lib/libSystem.B.dylib"
 //go:cgo_import_dynamic libc_pthread_cond_wait pthread_cond_wait "/usr/lib/libSystem.B.dylib"
-//go:cgo_import_dynamic libc_pthread_cond_timedwait pthread_cond_timedwait "/usr/lib/libSystem.B.dylib"
+//go:cgo_import_dynamic libc_pthread_cond_timedwait_relative_np pthread_cond_timedwait_relative_np "/usr/lib/libSystem.B.dylib"
 //go:cgo_import_dynamic libc_pthread_cond_signal pthread_cond_signal "/usr/lib/libSystem.B.dylib"
 
 // Magic incantation to get libSystem actually dynamically linked.

diff --git a/src/runtime/sys_darwin_386.s b/src/runtime/sys_darwin_386.s
index c0903e7b6035a4f45a5f0ef143683cf60ac2c35b..09f12283a18749315be5be0c953e6fc65098a698 100644 (file)
--- a/src/runtime/sys_darwin_386.s
+++ b/src/runtime/sys_darwin_386.s
@@ -595,7 +595,7 @@ TEXT runtime·pthread_cond_wait_trampoline(SB),NOSPLIT,$0
        POPL    BP
        RET
 
-TEXT runtime·pthread_cond_timedwait_trampoline(SB),NOSPLIT,$0
+TEXT runtime·pthread_cond_timedwait_relative_np_trampoline(SB),NOSPLIT,$0
        PUSHL   BP
        MOVL    SP, BP
        SUBL    $24, SP
@@ -606,7 +606,7 @@ TEXT runtime·pthread_cond_timedwait_trampoline(SB),NOSPLIT,$0
        MOVL    AX, 4(SP)
        MOVL    8(CX), AX       // arg 3 timeout
        MOVL    AX, 8(SP)
-       CALL    libc_pthread_cond_timedwait(SB)
+       CALL    libc_pthread_cond_timedwait_relative_np(SB)
        MOVL    BP, SP
        POPL    BP
        RET

diff --git a/src/runtime/sys_darwin_amd64.s b/src/runtime/sys_darwin_amd64.s
index 5522a86a1f8206dc724311be07e7b1c254e3b7bf..142933585db8f97f03557bbda74f14b957b5c4e3 100644 (file)
--- a/src/runtime/sys_darwin_amd64.s
+++ b/src/runtime/sys_darwin_amd64.s
@@ -447,13 +447,13 @@ TEXT runtime·pthread_cond_wait_trampoline(SB),NOSPLIT,$0
        POPQ    BP
        RET
 
-TEXT runtime·pthread_cond_timedwait_trampoline(SB),NOSPLIT,$0
+TEXT runtime·pthread_cond_timedwait_relative_np_trampoline(SB),NOSPLIT,$0
        PUSHQ   BP
        MOVQ    SP, BP
        MOVQ    8(DI), SI       // arg 2 mutex
        MOVQ    16(DI), DX      // arg 3 timeout
        MOVQ    0(DI), DI       // arg 1 cond
-       CALL    libc_pthread_cond_timedwait(SB)
+       CALL    libc_pthread_cond_timedwait_relative_np(SB)
        POPQ    BP
        RET
 

diff --git a/src/runtime/sys_darwin_arm.s b/src/runtime/sys_darwin_arm.s
index 5b3f553ff98450dec29728e27afcd5965f4a1d23..9b5c667f45198aa03522676e3b08f80970ddc216 100644 (file)
--- a/src/runtime/sys_darwin_arm.s
+++ b/src/runtime/sys_darwin_arm.s
@@ -368,11 +368,11 @@ TEXT runtime·pthread_cond_wait_trampoline(SB),NOSPLIT,$0
        BL      libc_pthread_cond_wait(SB)
        RET
 
-TEXT runtime·pthread_cond_timedwait_trampoline(SB),NOSPLIT,$0
+TEXT runtime·pthread_cond_timedwait_relative_np_trampoline(SB),NOSPLIT,$0
        MOVW    4(R0), R1       // arg 2 mutex
        MOVW    8(R0), R2       // arg 3 timeout
        MOVW    0(R0), R0       // arg 1 cond
-       BL      libc_pthread_cond_timedwait(SB)
+       BL      libc_pthread_cond_timedwait_relative_np(SB)
        RET
 
 TEXT runtime·pthread_cond_signal_trampoline(SB),NOSPLIT,$0

diff --git a/src/runtime/sys_darwin_arm64.s b/src/runtime/sys_darwin_arm64.s
index eb01774d8dbfe28150fcb5bc5f9fc586acc4194d..c324994d26f5aaf496ae476bd030843fb255a5ca 100644 (file)
--- a/src/runtime/sys_darwin_arm64.s
+++ b/src/runtime/sys_darwin_arm64.s
@@ -357,11 +357,11 @@ TEXT runtime·pthread_cond_wait_trampoline(SB),NOSPLIT,$0
        BL      libc_pthread_cond_wait(SB)
        RET
 
-TEXT runtime·pthread_cond_timedwait_trampoline(SB),NOSPLIT,$0
+TEXT runtime·pthread_cond_timedwait_relative_np_trampoline(SB),NOSPLIT,$0
        MOVD    8(R0), R1       // arg 2 mutex
        MOVD    16(R0), R2      // arg 3 timeout
        MOVD    0(R0), R0       // arg 1 cond
-       BL      libc_pthread_cond_timedwait(SB)
+       BL      libc_pthread_cond_timedwait_relative_np(SB)
        RET
 
 TEXT runtime·pthread_cond_signal_trampoline(SB),NOSPLIT,$0