[webkit-changes] [WebKit/WebKit] 18a451: REGRESSION (267818 at main): ~6% regression on `nytim...

Wenson Hsieh noreply at github.com
Thu Oct 26 18:26:17 PDT 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 18a4511bc7e2df666e3a46b8032e7b2a823cea06
      https://github.com/WebKit/WebKit/commit/18a4511bc7e2df666e3a46b8032e7b2a823cea06
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2023-10-26 (Thu, 26 Oct 2023)

  Changed paths:
    M Source/WebCore/dom/Document.cpp
    M Source/WebCore/page/OpportunisticTaskScheduler.cpp
    M Source/WebCore/page/OpportunisticTaskScheduler.h
    M Source/WebCore/page/Page.cpp

  Log Message:
  -----------
  REGRESSION (267818 at main): ~6% regression on `nytimes-article` in PLT
https://bugs.webkit.org/show_bug.cgi?id=263727
rdar://116463015

Reviewed by Tim Horton.

After the changes in 267818 at main, the `nytimes-article` subtest in PLT is regressed by roughly 6% on
some iPhone device models. From running several A/B experiments, this is due to the fact that the
opportunistic task scheduler now installs a repeating `CFRunLoopTimer`; even if the runloop timer
callback is a no-op, the overhead alone seems to be enough to impact NYT article pages in
particular.

To fix this, we instead turn the runloop timer back into a one-shot timer, and only schedule it if
needed (i.e., only if opportunistic GC/sweeping is enabled, or if there are scheduled idle
callbacks).

* Source/WebCore/dom/Document.cpp:
(WebCore::Document::requestIdleCallback):

Set a flag when calling into `requestIdleCallback`, to let the `OpportunisticTaskScheduler` know
that it needs to be scheduled for the purposes of firing opportunistic idle callbacks.

* Source/WebCore/page/OpportunisticTaskScheduler.cpp:
(WebCore::OpportunisticTaskScheduler::rescheduleIfNeeded):

Implement the main fix here; see above for more details.

(WebCore::OpportunisticTaskScheduler::checkedPage const):

Add a helper to return a `CheckedPtr<Page>`; take this opportunity to deploy smart pointers (mostly,
`CheckedPtr` in this class as well).

(WebCore::OpportunisticTaskScheduler::isPageInactiveOrLoading const):

Pull this common logic out into a helper method to avoid duplicating the check.

(WebCore::OpportunisticTaskScheduler::runLoopObserverFired):
(WebCore::OpportunisticTaskScheduler::reschedule): Deleted.

Rename this to `rescheduleIfNeeded` to reflect the fact that it may not reschedule the runloop timer
if there are no idle callbacks, and opportunistic GC/sweeping is disabled.

* Source/WebCore/page/OpportunisticTaskScheduler.h:
(WebCore::OpportunisticTaskScheduler::willQueueIdleCallback):
* Source/WebCore/page/Page.cpp:
(WebCore::Page::renderingUpdateCompleted):

Canonical link: https://commits.webkit.org/269833@main




More information about the webkit-changes mailing list