[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