Skip to content

Commit

Permalink
Revert "runtime: disable use of runnext on wasm"
Browse files Browse the repository at this point in the history
This reverts CL 557437.

Reason for revert: Appears to have broken wasip1 builders.

For golang#65178.

Change-Id: I59c1a310eb56589c768536fe444c1efaf862f8b0
Reviewed-on: https://go-review.googlesource.com/c/go/+/559237
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Michael Pratt <mpratt@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
  • Loading branch information
prattmic authored and gopherbot committed Jan 30, 2024
1 parent f96d9a6 commit 4afb155
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 21 deletions.
17 changes: 17 additions & 0 deletions src/net/net_fake.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"errors"
"io"
"os"
"runtime"
"sync"
"sync/atomic"
"syscall"
Expand Down Expand Up @@ -516,6 +517,14 @@ func (pq *packetQueue) send(dt *deadlineTimer, b []byte, from sockaddr, block bo
full = pq.full
}

// Before we check dt.expired, yield to other goroutines.
// This may help to prevent starvation of the goroutine that runs the
// deadlineTimer's time.After callback.
//
// TODO(#65178): Remove this when the runtime scheduler no longer starves
// runnable goroutines.
runtime.Gosched()

select {
case <-dt.expired:
return 0, os.ErrDeadlineExceeded
Expand Down Expand Up @@ -567,6 +576,14 @@ func (pq *packetQueue) recvfrom(dt *deadlineTimer, b []byte, wholePacket bool, c
empty = pq.empty
}

// Before we check dt.expired, yield to other goroutines.
// This may help to prevent starvation of the goroutine that runs the
// deadlineTimer's time.After callback.
//
// TODO(#65178): Remove this when the runtime scheduler no longer starves
// runnable goroutines.
runtime.Gosched()

select {
case <-dt.expired:
return 0, nil, os.ErrDeadlineExceeded
Expand Down
23 changes: 2 additions & 21 deletions src/runtime/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -167,7 +167,7 @@ func main() {
// Allow newproc to start new Ms.
mainStarted = true

if haveSysmon {
if GOARCH != "wasm" { // no threads on wasm yet, so no sysmon
systemstack(func() {
newm(sysmon, nil, -1)
})
Expand Down Expand Up @@ -5933,11 +5933,6 @@ var forcegcperiod int64 = 2 * 60 * 1e9
// golang.org/issue/42515 is needed on NetBSD.
var needSysmonWorkaround bool = false

// haveSysmon indicates whether there is sysmon thread support.
//
// No threads on wasm yet, so no sysmon.
const haveSysmon = GOARCH != "wasm"

// Always runs without a P, so write barriers are not allowed.
//
//go:nowritebarrierrec
Expand Down Expand Up @@ -6118,10 +6113,7 @@ func retake(now int64) uint32 {
s := pp.status
sysretake := false
if s == _Prunning || s == _Psyscall {
// Preempt G if it's running on the same schedtick for
// too long. This could be from a single long-running
// goroutine or a sequence of goroutines run via
// runnext, which share a single schedtick time slice.
// Preempt G if it's running for too long.
t := int64(pp.schedtick)
if int64(pd.schedtick) != t {
pd.schedtick = uint32(t)
Expand Down Expand Up @@ -6632,17 +6624,6 @@ const randomizeScheduler = raceenabled
// If the run queue is full, runnext puts g on the global queue.
// Executed only by the owner P.
func runqput(pp *p, gp *g, next bool) {
if !haveSysmon && next {
// A runnext goroutine shares the same time slice as the
// current goroutine (inheritTime from runqget). To prevent a
// ping-pong pair of goroutines from starving all others, we
// depend on sysmon to preempt "long-running goroutines". That
// is, any set of goroutines sharing the same time slice.
//
// If there is no sysmon, we must avoid runnext entirely or
// risk starvation.
next = false
}
if randomizeScheduler && next && randn(2) == 0 {
next = false
}
Expand Down
5 changes: 5 additions & 0 deletions src/time/sleep_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,11 @@ func TestAfterFuncStarvation(t *testing.T) {
// the AfterFunc goroutine instead of the runnable channel goroutine.
// However, in https://go.dev/issue/65178 this was observed to live-lock
// on wasip1/wasm and js/wasm after <10000 runs.

if runtime.GOARCH == "wasm" {
testenv.SkipFlaky(t, 65178)
}

defer runtime.GOMAXPROCS(runtime.GOMAXPROCS(1))

var (
Expand Down

0 comments on commit 4afb155

Please sign in to comment.