Changeset 291998 in webkit


Ignore:
Timestamp:
Mar 28, 2022 3:35:26 PM (4 months ago)
Author:
Cameron McCormack
Message:

Remove the 1ms minimum for setTimeout
​https://bugs.webkit.org/show_bug.cgi?id=221124
<rdar://problem/73852354>

Reviewed by Sam Weinig.

LayoutTests/imported/w3c:

  • web-platform-tests/html/webappapis/timers/zero-settimeout.any-expected.txt:
  • web-platform-tests/html/webappapis/timers/zero-settimeout.any.html:
  • web-platform-tests/html/webappapis/timers/zero-settimeout.any.js:

(async_test):

  • web-platform-tests/html/webappapis/timers/zero-settimeout.any.worker-expected.txt:
  • web-platform-tests/html/webappapis/timers/zero-settimeout.any.worker.html:

New test checking that 0ms and 1ms timeouts are called in the right
order.

  • web-platform-tests/FileAPI/file/File-constructor.any.worker-expected.txt:
  • web-platform-tests/html/webappapis/microtask-queuing/queue-microtask-exceptions.any.worker-expected.txt:
  • web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-cross-origin.sub.any.worker-expected.txt:
  • web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-redirect-to-cross-origin.sub.any.worker-expected.txt:
  • web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-same-origin.sub.any.worker-expected.txt:
  • web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-setTimeout-cross-origin.sub.any.worker-expected.txt:
  • web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-setTimeout-redirect-to-cross-origin.sub.any.worker-expected.txt:
  • web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-setTimeout-same-origin.sub.any.worker-expected.txt:

Disable console output in some worker tests where the reduced timeout
would cause intemittent failures due to the console message sometimes
not making it to the test output in time.

Source/WebCore:

This patch removes the 1ms minimum for setTimeout. The HTML spec makes
no mention of such a minimum, and Firefox and Chrome do not enforce
this minimum. Removing this for setTimeout results in a 0.7-2.1%
improvement on Speedometer, depending on platform and hardware.

The WPT added here demonstrates how this change can affect pages: if a
page schedules a 1ms and then a 0ms timeout in the same turn of the
event loop, then with this patch they will now be fired in the reverse
order. Since Firefox and Chrome do not impose a 1ms minimum, this
reduces the risk of this being a compatbility problem.

Scheduling a 0ms timeout will cause its callback to be called the next
time around the event loop. Other, non-timer queued tasks, will be
pre-empted. This behavior is permitted by the HTML spec, since the
event loop processing model[1] states that the implementation can
choose which task source to service, and timer callbacks are
dispatched using their own task source. Due to the way the SharedTimer
is called, we don't need to literally dispatch a task with a new
TaskSource::Timer source. (If we decided later to make a different
about whether to service timer callbacks before tasks from all other
task sources, we might need to.)

Not addressing the setTimeout 1ms minimum here, which should likely also
be removed.

While we're here, settle on "one shot" rather rather than "single
shot" as the term for timers that fire once.

[1] ​https://html.spec.whatwg.org/#event-loop-processing-model

Tests: imported/w3c/web-platform-tests/html/webappapis/timers/zero-settimeout.any.html

imported/w3c/web-platform-tests/html/webappapis/timers/zero-settimeout.any.worker.html

  • page/DOMTimer.h:
  • page/DOMTimer.cpp:

(WebCore::DOMTimer::DOMTimer):
(WebCore::DOMTimer::install):
(WebCore::DOMTimer::fired):
(WebCore::DOMTimer::updateTimerIntervalIfNecessary):
(WebCore::DOMTimer::intervalClampedToMinimum const):

LayoutTests:

the reduced timeout would cause intemittent failures due to the
console message sometimes not making it to the test output in time.

  • js/script-tests/weakref-finalizationregistry.js:

(turnEventLoop): Use a timeout of 1ms instead of 0ms so that
the deferred work task that calls the JS FinalizationRegistry
callback gets a chance to run before we continue on to the
assertion that it was run.

Location:
trunk
Files:
5 added
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r291993 r291998  
     12022-03-28  Cameron McCormack  <heycam@apple.com>
     2
     3        Remove the 1ms minimum for setTimeout
     4        https://bugs.webkit.org/show_bug.cgi?id=221124
     5        <rdar://problem/73852354>
     6
     7        Reviewed by Sam Weinig.
     8
     9        * TestExpectations: Disable console output in some worker tests where
     10        the reduced timeout would cause intemittent failures due to the
     11        console message sometimes not making it to the test output in time.
     12        * js/script-tests/weakref-finalizationregistry.js:
     13        (turnEventLoop): Use a timeout of 1ms instead of 0ms so that
     14        the deferred work task that calls the JS FinalizationRegistry
     15        callback gets a chance to run before we continue on to the
     16        assertion that it was run.
     17
    1182022-03-28  Antti Koivisto  <antti@apple.com>
    219

  • trunk/LayoutTests/TestExpectations

    r291993 r291998  
    49134913webkit.org/b/228176 fast/text/variable-system-font-2.html [ Pass ImageOnlyFailure ]
    49144914
     4915# Console message from workers sometimes don't make their way to test output, so disable them to avoid flakiness.
     4916imported/w3c/web-platform-tests/workers/interfaces/WorkerUtils/importScripts/ [ DumpJSConsoleLogInStdErr ]
     4917imported/w3c/web-platform-tests/FileAPI/file/File-constructor.any.worker.html [ DumpJSConsoleLogInStdErr ]
     4918imported/w3c/web-platform-tests/html/webappapis/microtask-queuing/queue-microtask-exceptions.any.worker.html [ DumpJSConsoleLogInStdErr ]
     4919
    49154920# It passes in legacy line layout by sheer luck. Fails with FF and Chrome too (it expects "random" code points to be computed to zero width).
    49164921fast/text/zero-width-characters-complex-script.html [ Failure ]

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r291993 r291998  
     12022-03-28  Cameron McCormack  <heycam@apple.com>
     2
     3        Remove the 1ms minimum for setTimeout
     4        https://bugs.webkit.org/show_bug.cgi?id=221124
     5        <rdar://problem/73852354>
     6
     7        Reviewed by Sam Weinig.
     8
     9        * web-platform-tests/html/webappapis/timers/zero-settimeout.any-expected.txt:
     10        * web-platform-tests/html/webappapis/timers/zero-settimeout.any.html:
     11        * web-platform-tests/html/webappapis/timers/zero-settimeout.any.js:
     12        (async_test):
     13        * web-platform-tests/html/webappapis/timers/zero-settimeout.any.worker-expected.txt:
     14        * web-platform-tests/html/webappapis/timers/zero-settimeout.any.worker.html:
     15        New test checking that 0ms and 1ms timeouts are called in the right
     16        order.
     17
     18        * web-platform-tests/FileAPI/file/File-constructor.any.worker-expected.txt:
     19        * web-platform-tests/html/webappapis/microtask-queuing/queue-microtask-exceptions.any.worker-expected.txt:
     20        * web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-cross-origin.sub.any.worker-expected.txt:
     21        * web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-redirect-to-cross-origin.sub.any.worker-expected.txt:
     22        * web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-same-origin.sub.any.worker-expected.txt:
     23        * web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-setTimeout-cross-origin.sub.any.worker-expected.txt:
     24        * web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-setTimeout-redirect-to-cross-origin.sub.any.worker-expected.txt:
     25        * web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-setTimeout-same-origin.sub.any.worker-expected.txt:
     26        Disable console output in some worker tests where the reduced timeout
     27        would cause intemittent failures due to the console message sometimes
     28        not making it to the test output in time.
     29
    1302022-03-28  Antti Koivisto  <antti@apple.com>
    231

  • trunk/LayoutTests/imported/w3c/web-platform-tests/FileAPI/file/File-constructor.any.worker-expected.txt

    r278962 r291998  
    1 CONSOLE MESSAGE: ReferenceError: Can't find variable: document
    21
    32Harness Error (FAIL), message = ReferenceError: Can't find variable: document

  • trunk/LayoutTests/imported/w3c/web-platform-tests/html/webappapis/microtask-queuing/queue-microtask-exceptions.any.worker-expected.txt

    r288640 r291998  
    1 CONSOLE MESSAGE: Error: boo
    21
    32PASS It rethrows exceptions

  • trunk/LayoutTests/imported/w3c/web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-cross-origin.sub.any.worker-expected.txt

    r279425 r291998  
    1 CONSOLE MESSAGE: Error: Script error.
    21
    32FAIL WorkerGlobalScope error event: error assert_equals: e.error should be a DOMException expected function "function DOMException() {

  • trunk/LayoutTests/imported/w3c/web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-redirect-to-cross-origin.sub.any.worker-expected.txt

    r279425 r291998  
    1 CONSOLE MESSAGE: Error: Script error.
    21
    32FAIL WorkerGlobalScope error event: error assert_equals: e.error should be a DOMException expected function "function DOMException() {

  • trunk/LayoutTests/imported/w3c/web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-same-origin.sub.any.worker-expected.txt

    r279425 r291998  
    1 CONSOLE MESSAGE: SyntaxError: Unexpected token ';'
    21
    32PASS WorkerGlobalScope error event: error

  • trunk/LayoutTests/imported/w3c/web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-setTimeout-cross-origin.sub.any.worker-expected.txt

    r279425 r291998  
    1 CONSOLE MESSAGE: Error: Script error.
    21
    32FAIL WorkerGlobalScope error event: error assert_equals: e.error should be a DOMException expected function "function DOMException() {

  • trunk/LayoutTests/imported/w3c/web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-setTimeout-redirect-to-cross-origin.sub.any.worker-expected.txt

    r279425 r291998  
    1 CONSOLE MESSAGE: Error: Script error.
    21
    32FAIL WorkerGlobalScope error event: error assert_equals: e.error should be a DOMException expected function "function DOMException() {

  • trunk/LayoutTests/imported/w3c/web-platform-tests/workers/interfaces/WorkerUtils/importScripts/report-error-setTimeout-same-origin.sub.any.worker-expected.txt

    r279425 r291998  
    1 CONSOLE MESSAGE: SyntaxError: Unexpected token ';'
    21
    32PASS WorkerGlobalScope error event: error

  • trunk/LayoutTests/js/script-tests/weakref-finalizationregistry.js

    r289152 r291998  
    1919            gc();
    2020            resolve();
    21         }, 0);
     21        }, 1);
    2222    });
    2323}

  • trunk/Source/WebCore/ChangeLog

    r291992 r291998  
     12022-03-28  Cameron McCormack  <heycam@apple.com>
     2
     3        Remove the 1ms minimum for setTimeout
     4        https://bugs.webkit.org/show_bug.cgi?id=221124
     5        <rdar://problem/73852354>
     6
     7        Reviewed by Sam Weinig.
     8
     9        This patch removes the 1ms minimum for setTimeout. The HTML spec makes
     10        no mention of such a minimum, and Firefox and Chrome do not enforce
     11        this minimum. Removing this for setTimeout results in a 0.7-2.1%
     12        improvement on Speedometer, depending on platform and hardware.
     13
     14        The WPT added here demonstrates how this change can affect pages: if a
     15        page schedules a 1ms and then a 0ms timeout in the same turn of the
     16        event loop, then with this patch they will now be fired in the reverse
     17        order. Since Firefox and Chrome do not impose a 1ms minimum, this
     18        reduces the risk of this being a compatbility problem.
     19
     20        Scheduling a 0ms timeout will cause its callback to be called the next
     21        time around the event loop. Other, non-timer queued tasks, will be
     22        pre-empted. This behavior is permitted by the HTML spec, since the
     23        event loop processing model[1] states that the implementation can
     24        choose which task source to service, and timer callbacks are
     25        dispatched using their own task source. Due to the way the SharedTimer
     26        is called, we don't need to literally dispatch a task with a new
     27        TaskSource::Timer source. (If we decided later to make a different
     28        about whether to service timer callbacks before tasks from all other
     29        task sources, we might need to.)
     30
     31        Not addressing the setTimeout 1ms minimum here, which should likely also
     32        be removed.
     33
     34        While we're here, settle on "one shot" rather rather than "single
     35        shot" as the term for timers that fire once.
     36
     37        [1] https://html.spec.whatwg.org/#event-loop-processing-model
     38
     39        Tests: imported/w3c/web-platform-tests/html/webappapis/timers/zero-settimeout.any.html
     40               imported/w3c/web-platform-tests/html/webappapis/timers/zero-settimeout.any.worker.html
     41
     42        * page/DOMTimer.h:
     43        * page/DOMTimer.cpp:
     44        (WebCore::DOMTimer::DOMTimer):
     45        (WebCore::DOMTimer::install):
     46        (WebCore::DOMTimer::fired):
     47        (WebCore::DOMTimer::updateTimerIntervalIfNecessary):
     48        (WebCore::DOMTimer::intervalClampedToMinimum const):
     49
    1502022-03-28  Chris Dumez  <cdumez@apple.com>
    251

  • trunk/Source/WebCore/page/DOMTimer.cpp

    r291513 r291998  
    5050
    5151static const Seconds minIntervalForNonUserObservableChangeTimers { 1_s }; // Empirically determined to maximize battery life.
     52static const Seconds minIntervalForOneShotTimers { 0_ms };
     53static const Seconds minIntervalForRepeatingTimers { 1_ms };
    5254static const int maxTimerNestingLevel = 5;
    5355
    … …  
    156158bool NestedTimersMap::isTrackingNestedTimers = false;
    157159
    158 DOMTimer::DOMTimer(ScriptExecutionContext& context, Function<void(ScriptExecutionContext&)>&& action, Seconds interval, bool singleShot)
     160DOMTimer::DOMTimer(ScriptExecutionContext& context, Function<void(ScriptExecutionContext&)>&& action, Seconds interval, bool oneShot)
    159161    : SuspendableTimerBase(&context)
    160162    , m_nestingLevel(context.timerNestingLevel())
    … …  
    162164    , m_originalInterval(interval)
    163165    , m_throttleState(Undetermined)
     166    , m_oneShot(oneShot)
    164167    , m_currentTimerInterval(intervalClampedToMinimum())
    165168    , m_userGestureTokenToForward(UserGestureIndicator::currentUserGesture())
    166169{
    167     if (singleShot)
     170    if (oneShot)
    168171        startOneShot(m_currentTimerInterval);
    169172    else
    … …  
    173176DOMTimer::~DOMTimer() = default;
    174177
    175 int DOMTimer::install(ScriptExecutionContext& context, std::unique_ptr<ScheduledAction> action, Seconds timeout, bool singleShot)
     178int DOMTimer::install(ScriptExecutionContext& context, std::unique_ptr<ScheduledAction> action, Seconds timeout, bool oneShot)
    176179{
    177180    auto actionFunction = [action = WTFMove(action)](ScriptExecutionContext& context) mutable {
    178181        action->execute(context);
    179182    };
    180     return DOMTimer::install(context, WTFMove(actionFunction), timeout, singleShot);
    181 }
    182 
    183 int DOMTimer::install(ScriptExecutionContext& context, Function<void(ScriptExecutionContext&)>&& action, Seconds timeout, bool singleShot)
    184 {
    185     Ref<DOMTimer> timer = adoptRef(*new DOMTimer(context, WTFMove(action), timeout, singleShot));
     183    return DOMTimer::install(context, WTFMove(actionFunction), timeout, oneShot);
     184}
     185
     186int DOMTimer::install(ScriptExecutionContext& context, Function<void(ScriptExecutionContext&)>&& action, Seconds timeout, bool oneShot)
     187{
     188    Ref<DOMTimer> timer = adoptRef(*new DOMTimer(context, WTFMove(action), timeout, oneShot));
    186189    timer->suspendIfNeeded();
    187190
    … …  
    191194    } while (!context.addTimeout(timer->m_timeoutId, timer.get()));
    192195
    193     InspectorInstrumentation::didInstallTimer(context, timer->m_timeoutId, timeout, singleShot);
     196    InspectorInstrumentation::didInstallTimer(context, timer->m_timeoutId, timeout, oneShot);
    194197
    195198    // Keep track of nested timer installs.
    … …  
    199202    if (is<Document>(context)) {
    200203        auto& document = downcast<Document>(context);
    201         document.contentChangeObserver().didInstallDOMTimer(timer.get(), timeout, singleShot);
     204        document.contentChangeObserver().didInstallDOMTimer(timer.get(), timeout, oneShot);
    202205        if (DeferDOMTimersForScope::isDeferring())
    203206            document.domTimerHoldingTank().add(timer.get());
    … …  
    289292    Ref<DOMTimer> protectedThis(*this);
    290293
    291     bool oneShot = !repeatInterval();
    292 
    293294    ASSERT(scriptExecutionContext());
    294295    ScriptExecutionContext& context = *scriptExecutionContext();
    … …  
    298299        auto& document = downcast<Document>(context);
    299300        if (auto* holdingTank = document.domTimerHoldingTankIfExists(); holdingTank && holdingTank->contains(*this)) {
    300             if (oneShot)
     301            if (m_oneShot)
    301302                startOneShot(0_s);
    302303            return;
    … …  
    316317    m_userGestureTokenToForward = nullptr;
    317318
    318     InspectorInstrumentation::willFireTimer(context, m_timeoutId, oneShot);
     319    InspectorInstrumentation::willFireTimer(context, m_timeoutId, m_oneShot);
    319320
    320321    // Simple case for non-one-shot timers.
    … …  
    327328        m_action(context);
    328329
    329         InspectorInstrumentation::didFireTimer(context, m_timeoutId, oneShot);
     330        InspectorInstrumentation::didFireTimer(context, m_timeoutId, m_oneShot);
    330331
    331332        updateThrottlingStateIfNecessary(fireState);
    … …  
    345346    m_action(context);
    346347
    347     InspectorInstrumentation::didFireTimer(context, m_timeoutId, oneShot);
     348    InspectorInstrumentation::didFireTimer(context, m_timeoutId, m_oneShot);
    348349
    349350    // Check if we should throttle nested single-shot timers.
    … …  
    351352        for (auto& idAndTimer : *nestedTimers) {
    352353            auto& timer = idAndTimer.value;
    353             if (timer->isActive() && !timer->repeatInterval())
     354            if (timer->isActive() && timer->m_oneShot)
    354355                timer->updateThrottlingStateIfNecessary(fireState);
    355356        }
    … …  
    375376        return;
    376377
    377     if (repeatInterval()) {
     378    if (m_oneShot) {
     379        LOG(DOMTimers, "%p - Updating DOMTimer's fire interval from %.2f ms to %.2f ms due to throttling.", this, previousInterval.milliseconds(), m_currentTimerInterval.milliseconds());
     380        augmentFireInterval(m_currentTimerInterval - previousInterval);
     381    } else {
    378382        ASSERT(repeatInterval() == previousInterval);
    379383        LOG(DOMTimers, "%p - Updating DOMTimer's repeat interval from %.2f ms to %.2f ms due to throttling.", this, previousInterval.milliseconds(), m_currentTimerInterval.milliseconds());
    380384        augmentRepeatInterval(m_currentTimerInterval - previousInterval);
    381     } else {
    382         LOG(DOMTimers, "%p - Updating DOMTimer's fire interval from %.2f ms to %.2f ms due to throttling.", this, previousInterval.milliseconds(), m_currentTimerInterval.milliseconds());
    383         augmentFireInterval(m_currentTimerInterval - previousInterval);
    384385    }
    385386}
    … …  
    390391    ASSERT(m_nestingLevel <= maxTimerNestingLevel);
    391392
    392     Seconds interval = std::max(1_ms, m_originalInterval);
     393    Seconds interval = std::max(m_oneShot ? minIntervalForOneShotTimers : minIntervalForRepeatingTimers, m_originalInterval);
    393394
    394395    // Only apply throttling to repeating timers.

  • trunk/Source/WebCore/page/DOMTimer.h

    r291513 r291998  
    9393    Seconds m_originalInterval;
    9494    TimerThrottleState m_throttleState;
     95    bool m_oneShot;
    9596    Seconds m_currentTimerInterval;
    9697    RefPtr<UserGestureToken> m_userGestureTokenToForward;
Note: See TracChangeset for help on using the changeset viewer.