Bug 221124

Summary: Not clamp setTimeout(..., 0) to 1ms
Product: WebKit Reporter: Wanming Lin <wanming.lin>
Component: DOMAssignee: Cameron McCormack (:heycam) <heycam>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, cdumez, clopez, commit-queue, dean_johnson, esprehn+autocc, ews-watchlist, fpizlo, ggaren, heycam, kangil.han, mjs, rniwa, saam, sam, simon.fraser, webkit-bug-importer, youennf, ysuzuki
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=253423
Bug Depends on: 215271, 238098    
Bug Blocks:    
Attachments:
Description Flags
Patch
none
Patch
none
Patch v2
none
Patch v2.1
none
Patch
none
Patch
none
Patch
none
Patch none

Wanming Lin
Reported 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
Attachments
Patch (11.96 KB, patch)
2022-03-15 14:53 PDT, Cameron McCormack (:heycam)
no flags
Patch (11.87 KB, patch)
2022-03-17 14:19 PDT, Cameron McCormack (:heycam)
no flags
Patch v2 (15.42 KB, patch)
2022-03-20 17:29 PDT, Cameron McCormack (:heycam)
no flags
Patch v2.1 (15.63 KB, patch)
2022-03-20 20:40 PDT, Cameron McCormack (:heycam)
no flags
Patch (15.95 KB, patch)
2022-03-23 14:01 PDT, Cameron McCormack (:heycam)
no flags
Patch (17.38 KB, patch)
2022-03-27 15:13 PDT, Cameron McCormack (:heycam)
no flags
Patch (26.27 KB, patch)
2022-03-27 19:29 PDT, Cameron McCormack (:heycam)
no flags
Patch (25.40 KB, patch)
2022-03-27 20:52 PDT, Cameron McCormack (:heycam)
no flags
Alexey Proskuryakov
Comment 1 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.
Wanming Lin
Comment 2 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.
Radar WebKit Bug Importer
Comment 3 2021-02-01 16:22:57 PST
Ryosuke Niwa
Comment 4 2021-02-04 17:50:03 PST
As far as we've tested, there were no improvements on Intel Macs.
Cameron McCormack (:heycam)
Comment 5 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.
Cameron McCormack (:heycam)
Comment 6 2022-03-15 00:43:02 PDT
And 1.2% on my Intel MBP.
Cameron McCormack (:heycam)
Comment 7 2022-03-15 14:53:36 PDT
EWS Watchlist
Comment 8 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
Cameron McCormack (:heycam)
Comment 9 2022-03-17 14:19:57 PDT
EWS
Comment 10 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].
WebKit Commit Bot
Comment 11 2022-03-18 15:18:30 PDT
Re-opened since this is blocked by bug 238098
Cameron McCormack (:heycam)
Comment 13 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.
Cameron McCormack (:heycam)
Comment 14 2022-03-20 17:29:40 PDT
Created attachment 455210 [details] Patch v2
Cameron McCormack (:heycam)
Comment 15 2022-03-20 20:40:14 PDT
Created attachment 455212 [details] Patch v2.1
Cameron McCormack (:heycam)
Comment 16 2022-03-23 14:01:40 PDT
Cameron McCormack (:heycam)
Comment 17 2022-03-27 15:13:23 PDT
Cameron McCormack (:heycam)
Comment 18 2022-03-27 19:29:47 PDT
Cameron McCormack (:heycam)
Comment 19 2022-03-27 20:52:23 PDT
EWS
Comment 20 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].
Note You need to log in before you can comment on or make changes to this bug.