From 1cfacfbe8ae7ed12eb798c84b53e1ebd0e3ed0b0 Mon Sep 17 00:00:00 2001 From: Garet Halliday Date: Thu, 13 Oct 2022 19:35:18 -0500 Subject: [PATCH] wasm: remove redundant calls to setTimeout and clearTimeout The existing implementation clears and recreates Javascript timeouts when Go is called from js, leading to excessive load on the js scheduler. Instead, we should remove redundant calls to clearTimeout and refrain from creating new timeouts if the previous event's timestamp is within 1 millisecond of our target (the js scheduler's max precision) Fixes #56100 Change-Id: I42bbed4c2f1fa6579c1f3aa519b6ed8fc003a20c Reviewed-on: https://go-review.googlesource.com/c/go/+/442995 Reviewed-by: Michael Knyszek TryBot-Result: Gopher Robot Reviewed-by: Heschi Kreinick Run-TryBot: Michael Knyszek --- misc/wasm/wasm_exec.js | 2 +- src/runtime/lock_js.go | 78 +++++++++++++++++++++++++++++------------- src/syscall/js/func.go | 17 ++++++--- 3 files changed, 68 insertions(+), 29 deletions(-) diff --git a/misc/wasm/wasm_exec.js b/misc/wasm/wasm_exec.js index 7f72bee005..bc6f210242 100644 --- a/misc/wasm/wasm_exec.js +++ b/misc/wasm/wasm_exec.js @@ -276,7 +276,7 @@ this._resume(); } }, - getInt64(sp + 8) + 1, // setTimeout has been seen to fire up to 1 millisecond early + getInt64(sp + 8), )); this.mem.setInt32(sp + 16, id, true); }, diff --git a/src/runtime/lock_js.go b/src/runtime/lock_js.go index ae2bb3db47..fd2abee7c4 100644 --- a/src/runtime/lock_js.go +++ b/src/runtime/lock_js.go @@ -117,7 +117,6 @@ func notetsleepg(n *note, ns int64) bool { gopark(nil, nil, waitReasonSleep, traceBlockSleep, 1) clearTimeoutEvent(id) // note might have woken early, clear timeout - clearIdleID() mp = acquirem() delete(notes, n) @@ -169,8 +168,36 @@ type event struct { returned bool } +type timeoutEvent struct { + id int32 + // The time when this timeout will be triggered. + time int64 +} + +// diff calculates the difference of the event's trigger time and x. +func (e *timeoutEvent) diff(x int64) int64 { + if e == nil { + return 0 + } + + diff := x - idleTimeout.time + if diff < 0 { + diff = -diff + } + return diff +} + +// clear cancels this timeout event. +func (e *timeoutEvent) clear() { + if e == nil { + return + } + + clearTimeoutEvent(e.id) +} + // The timeout event started by beforeIdle. -var idleID int32 +var idleTimeout *timeoutEvent // beforeIdle gets called by the scheduler if no goroutine is awake. // If we are not already handling an event, then we pause for an async event. @@ -183,21 +210,23 @@ var idleID int32 func beforeIdle(now, pollUntil int64) (gp *g, otherReady bool) { delay := int64(-1) if pollUntil != 0 { - delay = pollUntil - now - } - - if delay > 0 { - clearIdleID() - if delay < 1e6 { - delay = 1 - } else if delay < 1e15 { - delay = delay / 1e6 - } else { + // round up to prevent setTimeout being called early + delay = (pollUntil-now-1)/1e6 + 1 + if delay > 1e9 { // An arbitrary cap on how long to wait for a timer. // 1e9 ms == ~11.5 days. delay = 1e9 } - idleID = scheduleTimeoutEvent(delay) + } + + if delay > 0 && (idleTimeout == nil || idleTimeout.diff(pollUntil) > 1e6) { + // If the difference is larger than 1 ms, we should reschedule the timeout. + idleTimeout.clear() + + idleTimeout = &timeoutEvent{ + id: scheduleTimeoutEvent(delay), + time: pollUntil, + } } if len(events) == 0 { @@ -217,12 +246,10 @@ func handleAsyncEvent() { pause(getcallersp() - 16) } -// clearIdleID clears our record of the timeout started by beforeIdle. -func clearIdleID() { - if idleID != 0 { - clearTimeoutEvent(idleID) - idleID = 0 - } +// clearIdleTimeout clears our record of the timeout started by beforeIdle. +func clearIdleTimeout() { + idleTimeout.clear() + idleTimeout = nil } // pause sets SP to newsp and pauses the execution of Go's WebAssembly code until an event is triggered. @@ -250,9 +277,10 @@ func handleEvent() { } events = append(events, e) - eventHandler() - - clearIdleID() + if !eventHandler() { + // If we did not handle a window event, the idle timeout was triggered, so we can clear it. + clearIdleTimeout() + } // wait until all goroutines are idle e.returned = true @@ -265,9 +293,11 @@ func handleEvent() { pause(getcallersp() - 16) } -var eventHandler func() +// eventHandler retrieves and executes handlers for pending JavaScript events. +// It returns true if an event was handled. +var eventHandler func() bool //go:linkname setEventHandler syscall/js.setEventHandler -func setEventHandler(fn func()) { +func setEventHandler(fn func() bool) { eventHandler = fn } diff --git a/src/syscall/js/func.go b/src/syscall/js/func.go index cc94972364..53a4d79a95 100644 --- a/src/syscall/js/func.go +++ b/src/syscall/js/func.go @@ -60,16 +60,19 @@ func (c Func) Release() { } // setEventHandler is defined in the runtime package. -func setEventHandler(fn func()) +func setEventHandler(fn func() bool) func init() { setEventHandler(handleEvent) } -func handleEvent() { +// handleEvent retrieves the pending event (window._pendingEvent) and calls the js.Func on it. +// It returns true if an event was handled. +func handleEvent() bool { + // Retrieve the event from js cb := jsGo.Get("_pendingEvent") if cb.IsNull() { - return + return false } jsGo.Set("_pendingEvent", Null()) @@ -77,14 +80,17 @@ func handleEvent() { if id == 0 { // zero indicates deadlock select {} } + + // Retrieve the associated js.Func funcsMu.Lock() f, ok := funcs[id] funcsMu.Unlock() if !ok { Global().Get("console").Call("error", "call to released function") - return + return true } + // Call the js.Func with arguments this := cb.Get("this") argsObj := cb.Get("args") args := make([]Value, argsObj.Length()) @@ -92,5 +98,8 @@ func handleEvent() { args[i] = argsObj.Index(i) } result := f(this, args) + + // Return the result to js cb.Set("result", result) + return true } -- 2.44.0