Bug 221124 - Not clamp setTimeout(..., 0) to 1ms
Summary: Not clamp setTimeout(..., 0) to 1ms
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Cameron McCormack (:heycam)
URL:
Keywords: InRadar
Depends on: 215271 238098
Blocks:
  Show dependency treegraph
 
Reported: 2021-01-29 00:23 PST by Wanming Lin
Modified: 2022-03-28 15:36 PDT (History)
19 users (show)

See Also:


Attachments
Patch (11.96 KB, patch)
2022-03-15 14:53 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff
Patch (11.87 KB, patch)
2022-03-17 14:19 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff
Patch v2 (15.42 KB, patch)
2022-03-20 17:29 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff
Patch v2.1 (15.63 KB, patch)
2022-03-20 20:40 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff
Patch (15.95 KB, patch)
2022-03-23 14:01 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff
Patch (17.38 KB, patch)
2022-03-27 15:13 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff
Patch (26.27 KB, patch)
2022-03-27 19:29 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff
Patch (25.40 KB, patch)
2022-03-27 20:52 PDT, Cameron McCormack (:heycam)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Wanming Lin 2021-01-29 00:23:43 PST
Calls to setTimeout(..., 0) is clamped to a 1 ms timeout in WebKit(https://github.com/WebKit/WebKit/blob/main/Source/WebCore/page/DOMTimer.cpp#L384), this is actually a bug since there's no 1ms clamp in the spec(https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#timer-initialisation-steps), and which will cause scheduling error, e.g.

setTimeout(()=> console.log('1ms timeout'), 1);
setTimeout(()=> console.log('0ms timeout'), 0);

Outputs:
Safari:  1ms timeout, 0ms timeout

Moreover, 1ms clamp may bring performance penalty.

Compatibility:
Firefox: no 1ms clamp
Safari: 1ms clamp
Chrome: 1ms clamp

Chrome is planning to remove this 1ms clamp, see discussion at https://groups.google.com/a/chromium.org/g/blink-dev/c/HKPTp7C1LwY/m/kIbTxKSPAwAJ
Comment 1 Alexey Proskuryakov 2021-01-31 12:42:08 PST
What is the motivation for changing the behavior, as opposed to correcting the spec? Seems like only bad things can possibly happen with regards to battery life; and there are other reasons why timers would be clamped anyway (nesters timers, backgrounds tabs).

If this is desired to correct ordering, there are other ways to do it, but also unsure if it's a particularly likely case to run into.
Comment 2 Wanming Lin 2021-02-01 05:32:40 PST
Thanks Alexey! One more motivation of this change is performance impact, we can see about 1% beneficial to Speedometer 2 on MiniBrowser on M1 with this change.
Comment 3 Radar WebKit Bug Importer 2021-02-01 16:22:57 PST
<rdar://problem/73852354>
Comment 4 Ryosuke Niwa 2021-02-04 17:50:03 PST
As far as we've tested, there were no improvements on Intel Macs.
Comment 5 Cameron McCormack (:heycam) 2022-03-14 17:39:42 PDT
Removing the 1ms setTimeout minimum, and leaving the existing nested throttling behavior, currently shows a 1.7% improvement on Speedometer locally for me, on an M1 MacBook Air.
Comment 6 Cameron McCormack (:heycam) 2022-03-15 00:43:02 PDT
And 1.2% on my Intel MBP.
Comment 7 Cameron McCormack (:heycam) 2022-03-15 14:53:36 PDT
Created attachment 454760 [details]
Patch
Comment 8 EWS Watchlist 2022-03-15 14:55:46 PDT
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess
Comment 9 Cameron McCormack (:heycam) 2022-03-17 14:19:57 PDT
Created attachment 455027 [details]
Patch
Comment 10 EWS 2022-03-18 04:59:17 PDT
Committed r291476 (248592@main): <https://commits.webkit.org/248592@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455027 [details].
Comment 11 WebKit Commit Bot 2022-03-18 15:18:30 PDT
Re-opened since this is blocked by bug 238098
Comment 13 Cameron McCormack (:heycam) 2022-03-18 15:41:39 PDT
I shouldn't have cq+ed this last night.

Thinking about that test failure more this morning, I'm wondering if setTimeout(..., 0) callbacks are preempting whatever else is in the event queue.  The HTML spec says that when the timeout fires, it should post a task to the event queue to run the timeout callback.
Comment 14 Cameron McCormack (:heycam) 2022-03-20 17:29:40 PDT
Created attachment 455210 [details]
Patch v2
Comment 15 Cameron McCormack (:heycam) 2022-03-20 20:40:14 PDT
Created attachment 455212 [details]
Patch v2.1
Comment 16 Cameron McCormack (:heycam) 2022-03-23 14:01:40 PDT
Created attachment 455550 [details]
Patch
Comment 17 Cameron McCormack (:heycam) 2022-03-27 15:13:23 PDT
Created attachment 455868 [details]
Patch
Comment 18 Cameron McCormack (:heycam) 2022-03-27 19:29:47 PDT
Created attachment 455880 [details]
Patch
Comment 19 Cameron McCormack (:heycam) 2022-03-27 20:52:23 PDT
Created attachment 455881 [details]
Patch
Comment 20 EWS 2022-03-28 15:36:37 PDT
Committed r291998 (248952@main): <https://commits.webkit.org/248952@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 455881 [details].