- Revision
- 240759
- Author
- [email protected]
- Date
- 2019-01-30 20:53:34 -0800 (Wed, 30 Jan 2019)
Log Message
<rdar://problem/47570443> Responsiveness timers are too expensive for frequent events
https://bugs.webkit.org/show_bug.cgi?id=194003
Reviewed by Geoffrey Garen.
The problem here is specific to wheel events.
For every wheel event, we start a responsiveness timer and send
a ping to the WebProcess. When the WebProcess respond, we stop the timer.
The cost of setting up the timers adds up since we get many events.
The first step to improve the situation was to switch ResponsivenessTimer
to WebCore::Timer. Since WebCore::Timer reuse the same CFRunLoopTimerRef,
we save the allocation/deallocation, insertion in the event loop, etc.
Using WebCore::Timer saves some instructions but we were still hitting
the kernel at 120hz to set up then kill each timer.
The second improvement of the patch is to avoid that by not killing the timer
when we hear back from the WebProcess.
Instead of killing the timer, we let it run and ignore the result.
When the next event comes, we reschedule the existing timer.
This brings down the timers to 60Hz, the same rate as the events.
The very last event does time out. In that case, we have a bad idle wake up:
we wake up a sleeping CPU do do nothing.
In the case of wheel events, this is fine since we saved a bunch of CPU already.
For all the other cases, I kept the normal operating mode to avoid the idle wake.
* UIProcess/ResponsivenessTimer.cpp:
(WebKit::ResponsivenessTimer::ResponsivenessTimer):
(WebKit::ResponsivenessTimer::invalidate):
(WebKit::ResponsivenessTimer::timerFired):
(WebKit::ResponsivenessTimer::start):
(WebKit::ResponsivenessTimer::startWithLazyStop):
(WebKit::ResponsivenessTimer::stop):
(WebKit::ResponsivenessTimer::processTerminated):
(WebKit::ResponsivenessTimer::~ResponsivenessTimer): Deleted.
* UIProcess/ResponsivenessTimer.h:
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::sendWheelEvent):
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::isResponsiveWithLazyStop):
* UIProcess/WebProcessProxy.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (240758 => 240759)
--- trunk/Source/WebKit/ChangeLog 2019-01-31 04:49:57 UTC (rev 240758)
+++ trunk/Source/WebKit/ChangeLog 2019-01-31 04:53:34 UTC (rev 240759)
@@ -1,3 +1,51 @@
+2019-01-30 Benjamin Poulain <[email protected]>
+
+ <rdar://problem/47570443> Responsiveness timers are too expensive for frequent events
+ https://bugs.webkit.org/show_bug.cgi?id=194003
+
+ Reviewed by Geoffrey Garen.
+
+ The problem here is specific to wheel events.
+
+ For every wheel event, we start a responsiveness timer and send
+ a ping to the WebProcess. When the WebProcess respond, we stop the timer.
+
+ The cost of setting up the timers adds up since we get many events.
+
+ The first step to improve the situation was to switch ResponsivenessTimer
+ to WebCore::Timer. Since WebCore::Timer reuse the same CFRunLoopTimerRef,
+ we save the allocation/deallocation, insertion in the event loop, etc.
+
+ Using WebCore::Timer saves some instructions but we were still hitting
+ the kernel at 120hz to set up then kill each timer.
+ The second improvement of the patch is to avoid that by not killing the timer
+ when we hear back from the WebProcess.
+
+ Instead of killing the timer, we let it run and ignore the result.
+ When the next event comes, we reschedule the existing timer.
+ This brings down the timers to 60Hz, the same rate as the events.
+
+ The very last event does time out. In that case, we have a bad idle wake up:
+ we wake up a sleeping CPU do do nothing.
+ In the case of wheel events, this is fine since we saved a bunch of CPU already.
+ For all the other cases, I kept the normal operating mode to avoid the idle wake.
+
+ * UIProcess/ResponsivenessTimer.cpp:
+ (WebKit::ResponsivenessTimer::ResponsivenessTimer):
+ (WebKit::ResponsivenessTimer::invalidate):
+ (WebKit::ResponsivenessTimer::timerFired):
+ (WebKit::ResponsivenessTimer::start):
+ (WebKit::ResponsivenessTimer::startWithLazyStop):
+ (WebKit::ResponsivenessTimer::stop):
+ (WebKit::ResponsivenessTimer::processTerminated):
+ (WebKit::ResponsivenessTimer::~ResponsivenessTimer): Deleted.
+ * UIProcess/ResponsivenessTimer.h:
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::sendWheelEvent):
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::isResponsiveWithLazyStop):
+ * UIProcess/WebProcessProxy.h:
+
2019-01-30 Daniel Bates <[email protected]>
[iOS] REGRESSION (r238635): Text area fails to re-focus after dismissal of keyboard on support.apple.com
Modified: trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp (240758 => 240759)
--- trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp 2019-01-31 04:49:57 UTC (rev 240758)
+++ trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp 2019-01-31 04:53:34 UTC (rev 240759)
@@ -32,27 +32,32 @@
ResponsivenessTimer::ResponsivenessTimer(ResponsivenessTimer::Client& client)
: m_client(client)
- , m_isResponsive(true)
- , m_timer(RunLoop::main(), this, &ResponsivenessTimer::timerFired)
+ , m_timer(*this, &ResponsivenessTimer::timerFired)
{
}
-ResponsivenessTimer::~ResponsivenessTimer()
-{
- m_timer.stop();
-}
+ResponsivenessTimer::~ResponsivenessTimer() = default;
void ResponsivenessTimer::invalidate()
{
+ m_waitingForTimer = false;
+ m_useLazyStop = false;
m_timer.stop();
}
void ResponsivenessTimer::timerFired()
{
+ if (!m_waitingForTimer)
+ return;
+
+ m_waitingForTimer = false;
+ m_useLazyStop = false;
+
if (!m_isResponsive)
return;
if (!m_client.mayBecomeUnresponsive()) {
+ m_waitingForTimer = true;
m_timer.startOneShot(responsivenessTimeout);
return;
}
@@ -66,12 +71,22 @@
void ResponsivenessTimer::start()
{
- if (m_timer.isActive())
+ if (m_waitingForTimer)
return;
+ m_waitingForTimer = true;
+ m_useLazyStop = false;
m_timer.startOneShot(responsivenessTimeout);
}
+void ResponsivenessTimer::startWithLazyStop()
+{
+ if (!m_waitingForTimer) {
+ start();
+ m_useLazyStop = true;
+ }
+}
+
void ResponsivenessTimer::stop()
{
if (!m_isResponsive) {
@@ -83,13 +98,19 @@
m_client.didBecomeResponsive();
}
- m_timer.stop();
+ m_waitingForTimer = false;
+
+ if (m_useLazyStop)
+ m_useLazyStop = false;
+ else
+ m_timer.stop();
}
void ResponsivenessTimer::processTerminated()
{
// Since there is no web process, we must not be waiting for it anymore.
- stop();
+ m_waitingForTimer = false;
+ m_timer.stop();
}
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h (240758 => 240759)
--- trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h 2019-01-31 04:49:57 UTC (rev 240758)
+++ trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h 2019-01-31 04:53:34 UTC (rev 240759)
@@ -26,7 +26,7 @@
#ifndef ResponsivenessTimer_h
#define ResponsivenessTimer_h
-#include <wtf/RunLoop.h>
+#include <WebCore/Timer.h>
namespace WebKit {
@@ -48,6 +48,17 @@
~ResponsivenessTimer();
void start();
+
+ // A responsiveness timer with lazy stop does not stop the underlying system timer when stopped.
+ // Instead, it ignores the timeout if stop() was already called.
+ //
+ // This exists to reduce the rate at which we reset the timer.
+ //
+ // With a non lazy timer, we may set a timer and reset it soon after because the process is responsive.
+ // For events, this means reseting a timer 120 times/s for a 60 Hz event source.
+ // By not reseting the timer when responsive, we cut that in half to 60 timeout changes.
+ void startWithLazyStop();
+
void stop();
void invalidate();
@@ -60,9 +71,10 @@
void timerFired();
ResponsivenessTimer::Client& m_client;
- bool m_isResponsive;
-
- RunLoop::Timer<ResponsivenessTimer> m_timer;
+ WebCore::Timer m_timer;
+ bool m_isResponsive { true };
+ bool m_waitingForTimer { false };
+ bool m_useLazyStop { false };
};
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (240758 => 240759)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-01-31 04:49:57 UTC (rev 240758)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-01-31 04:53:34 UTC (rev 240759)
@@ -2357,7 +2357,7 @@
// Manually ping the web process to check for responsiveness since our wheel
// event will dispatch to a non-main thread, which always responds.
- m_process->isResponsive(nullptr);
+ m_process->isResponsiveWithLazyStop();
}
bool WebPageProxy::shouldProcessWheelEventNow(const WebWheelEvent& event) const
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (240758 => 240759)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-01-31 04:49:57 UTC (rev 240758)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-01-31 04:53:34 UTC (rev 240759)
@@ -1117,6 +1117,15 @@
send(Messages::WebProcess::MainThreadPing(), 0);
}
+void WebProcessProxy::isResponsiveWithLazyStop()
+{
+ if (m_isResponsive == NoOrMaybe::No)
+ return;
+
+ responsivenessTimer().startWithLazyStop();
+ send(Messages::WebProcess::MainThreadPing(), 0);
+}
+
bool WebProcessProxy::isJITEnabled() const
{
return processPool().configuration().isJITEnabled();
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (240758 => 240759)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-01-31 04:49:57 UTC (rev 240758)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-01-31 04:53:34 UTC (rev 240759)
@@ -206,6 +206,7 @@
ProcessThrottler& throttler() { return m_throttler; }
void isResponsive(WTF::Function<void(bool isWebProcessResponsive)>&&);
+ void isResponsiveWithLazyStop();
void didReceiveMainThreadPing();
void didReceiveBackgroundResponsivenessPing();
