diff options
author | Bartek IwaĆczuk <biwanczuk@gmail.com> | 2023-05-22 14:09:31 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-05-22 14:09:31 +0200 |
commit | ffd3ed9b8ad34471a0ba30727b7321f86c0a30b5 (patch) | |
tree | 00b1abc7a673102b61bd2ebf4dec30839eb167fb | |
parent | 40bda07ff5751cbb2665a2d134f64826fe3790a8 (diff) |
fix(ext/web): improve timers resolution for 0ms timeouts (#19212)
This commit changes the implementation of `ext/web` timers, by using
"op_void_async_deferred" for timeouts of 0ms.
0ms timeout is meant to be run at the end of the event loop tick and
currently Tokio timers that we use to back timeouts have at least 1ms
resolution. That means that 0ms timeout actually take >1ms. This
commit changes that and runs 0ms timeout at the end of the event
loop tick.
One consequence is that "unrefing" a 0ms timer will actually keep
the event loop alive (which I believe actually makes sense, the test
we had only worked because the timeout took more than 1ms).
Ref https://github.com/denoland/deno/issues/19034
-rw-r--r-- | cli/js/40_testing.js | 2 | ||||
-rw-r--r-- | cli/tests/unit/timers_test.ts | 2 | ||||
-rw-r--r-- | ext/web/02_timers.js | 19 |
3 files changed, 18 insertions, 5 deletions
diff --git a/cli/js/40_testing.js b/cli/js/40_testing.js index e269b9c9f..ec83ce370 100644 --- a/cli/js/40_testing.js +++ b/cli/js/40_testing.js @@ -50,7 +50,7 @@ function opSanitizerDelay() { return new Promise((resolve) => { setTimeout(() => { ArrayPrototypePush(opSanitizerDelayResolveQueue, resolve); - }, 0); + }, 1); }); } diff --git a/cli/tests/unit/timers_test.ts b/cli/tests/unit/timers_test.ts index 8de756516..c50cb779c 100644 --- a/cli/tests/unit/timers_test.ts +++ b/cli/tests/unit/timers_test.ts @@ -557,7 +557,7 @@ Deno.test({ permissions: { run: true, read: true }, fn: async () => { const [statusCode, output] = await execCode(` - const timer = setTimeout(() => console.log("1")); + const timer = setTimeout(() => console.log("1"), 1); Deno.unrefTimer(timer); `); assertEquals(statusCode, 0); diff --git a/ext/web/02_timers.js b/ext/web/02_timers.js index cfd85a055..ed9f1c6fb 100644 --- a/ext/web/02_timers.js +++ b/ext/web/02_timers.js @@ -27,7 +27,10 @@ const { import * as webidl from "ext:deno_webidl/00_webidl.js"; import { reportException } from "ext:deno_web/02_event.js"; import { assert } from "ext:deno_web/00_infra.js"; -const { op_sleep } = core.generateAsyncOpHandler("op_sleep"); +const { op_sleep, op_void_async_deferred } = core.generateAsyncOpHandler( + "op_sleep", + "op_void_async_deferred", +); const hrU8 = new Uint8Array(8); const hr = new Uint32Array(TypedArrayPrototypeGetBuffer(hrU8)); @@ -218,7 +221,16 @@ const scheduledTimers = { head: null, tail: null }; */ function runAfterTimeout(cb, millis, timerInfo) { const cancelRid = timerInfo.cancelRid; - const sleepPromise = op_sleep(millis, cancelRid); + let sleepPromise; + // If this timeout is scheduled for 0ms it means we want it to run at the + // end of the event loop turn. There's no point in setting up a Tokio timer, + // since its lowest resolution is 1ms. Firing of a "void async" op is better + // in this case, because the timer will take closer to 0ms instead of >1ms. + if (millis === 0) { + sleepPromise = op_void_async_deferred(); + } else { + sleepPromise = op_sleep(millis, cancelRid); + } timerInfo.promiseId = sleepPromise[SymbolFor("Deno.core.internalPromiseId")]; if (!timerInfo.isRef) { core.unrefOp(timerInfo.promiseId); @@ -246,7 +258,8 @@ function runAfterTimeout(cb, millis, timerInfo) { PromisePrototypeThen( sleepPromise, (cancelled) => { - if (!cancelled) { + // "op_void_async_deferred" returns null + if (cancelled !== null && !cancelled) { // The timer was cancelled. removeFromScheduledTimers(timerObject); return; |