- Revision
- 241351
- Author
- [email protected]
- Date
- 2019-02-13 00:41:22 -0800 (Wed, 13 Feb 2019)
Log Message
Responsiveness timers are too expensive for frequent events
https://bugs.webkit.org/show_bug.cgi?id=194003
Reviewed by Geoffrey Garen.
With each event, we set a responsivness timer to check if the WebProcess
is responsive, and reset the timer when the WebProcess sends an answer.
For frequent events (e.g. wheel events, mouse force events, etc),
we are spamming the kernel with hundreds of timers per second.
That is a bit inefficient.
Another source of inefficiency comes from the timer implementation
itself. Stopping a RunLoop::Timer removes the timer from every mode
and invalidate the timer. It becomes costly since we do it a lot.
With this patch, I tweak ResponsivenessTimer and its use to minimize
how often we schedule system timers.
The first change is to not stop the timer when we get the stop()
calls if we expect more events to come in. Instead, we keep track
if we care about the timeout or not in the attribute "m_waitingForTimer".
When the next event starts, we can reschedule the timer without ever
having told the kernel about the stop.
If there are no next events, the timeout fires but m_waitingForTimer
is false. To avoid idle wake up, the lazy stop is only used when having
following events is common.
The second improvements comes from not even rescheduling the timer
when restarted. Instead of changing the timer, we let the original timer
fire and re-shedule a new one with the missing time.
For more context, also see patches r240759 and r240944.
* 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:
(WebKit::ResponsivenessTimer::hasActiveTimer const):
* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::processNextQueuedMouseEvent):
(WebKit::WebPageProxy::sendWheelEvent):
(WebKit::WebPageProxy::handleKeyboardEvent):
(WebKit::WebPageProxy::handleGestureEvent):
* UIProcess/WebProcessProxy.cpp:
(WebKit::WebProcessProxy::isResponsiveWithLazyStop):
* UIProcess/WebProcessProxy.h:
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (241350 => 241351)
--- trunk/Source/WebKit/ChangeLog 2019-02-13 08:26:23 UTC (rev 241350)
+++ trunk/Source/WebKit/ChangeLog 2019-02-13 08:41:22 UTC (rev 241351)
@@ -1,3 +1,59 @@
+2019-02-13 Benjamin Poulain <[email protected]>
+
+ Responsiveness timers are too expensive for frequent events
+ https://bugs.webkit.org/show_bug.cgi?id=194003
+
+ Reviewed by Geoffrey Garen.
+
+ With each event, we set a responsivness timer to check if the WebProcess
+ is responsive, and reset the timer when the WebProcess sends an answer.
+
+ For frequent events (e.g. wheel events, mouse force events, etc),
+ we are spamming the kernel with hundreds of timers per second.
+ That is a bit inefficient.
+
+ Another source of inefficiency comes from the timer implementation
+ itself. Stopping a RunLoop::Timer removes the timer from every mode
+ and invalidate the timer. It becomes costly since we do it a lot.
+
+ With this patch, I tweak ResponsivenessTimer and its use to minimize
+ how often we schedule system timers.
+
+ The first change is to not stop the timer when we get the stop()
+ calls if we expect more events to come in. Instead, we keep track
+ if we care about the timeout or not in the attribute "m_waitingForTimer".
+ When the next event starts, we can reschedule the timer without ever
+ having told the kernel about the stop.
+ If there are no next events, the timeout fires but m_waitingForTimer
+ is false. To avoid idle wake up, the lazy stop is only used when having
+ following events is common.
+
+ The second improvements comes from not even rescheduling the timer
+ when restarted. Instead of changing the timer, we let the original timer
+ fire and re-shedule a new one with the missing time.
+
+ For more context, also see patches r240759 and r240944.
+
+ * 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:
+ (WebKit::ResponsivenessTimer::hasActiveTimer const):
+ * UIProcess/WebPageProxy.cpp:
+ (WebKit::WebPageProxy::processNextQueuedMouseEvent):
+ (WebKit::WebPageProxy::sendWheelEvent):
+ (WebKit::WebPageProxy::handleKeyboardEvent):
+ (WebKit::WebPageProxy::handleGestureEvent):
+ * UIProcess/WebProcessProxy.cpp:
+ (WebKit::WebProcessProxy::isResponsiveWithLazyStop):
+ * UIProcess/WebProcessProxy.h:
+
2019-02-12 Tim Horton <[email protected]>
Null deref in userInterfaceLayoutDirection under ViewGestureController::handleSwipeGesture
Modified: trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp (241350 => 241351)
--- trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp 2019-02-13 08:26:23 UTC (rev 241350)
+++ trunk/Source/WebKit/UIProcess/ResponsivenessTimer.cpp 2019-02-13 08:41:22 UTC (rev 241351)
@@ -32,27 +32,44 @@
ResponsivenessTimer::ResponsivenessTimer(ResponsivenessTimer::Client& client)
: m_client(client)
- , m_isResponsive(true)
, m_timer(RunLoop::main(), this, &ResponsivenessTimer::timerFired)
{
}
-ResponsivenessTimer::~ResponsivenessTimer()
-{
- m_timer.stop();
-}
+ResponsivenessTimer::~ResponsivenessTimer() = default;
void ResponsivenessTimer::invalidate()
{
m_timer.stop();
+ m_restartFireTime = MonotonicTime();
+ m_waitingForTimer = false;
+ m_useLazyStop = false;
}
void ResponsivenessTimer::timerFired()
{
+ if (!m_waitingForTimer)
+ return;
+
+ if (m_restartFireTime) {
+ MonotonicTime now = MonotonicTime::now();
+ MonotonicTime restartFireTime = m_restartFireTime;
+ m_restartFireTime = MonotonicTime();
+
+ if (restartFireTime > now) {
+ m_timer.startOneShot(now - restartFireTime);
+ 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 +83,33 @@
void ResponsivenessTimer::start()
{
- if (m_timer.isActive())
+ if (m_waitingForTimer)
return;
- m_timer.startOneShot(responsivenessTimeout);
+ m_waitingForTimer = true;
+ m_useLazyStop = false;
+
+ if (m_timer.isActive()) {
+ // The timer is still active from a lazy stop.
+ // Instead of restarting the timer, we schedule a new delay after this one finishes.
+ //
+ // In most cases, stop is called before we get to schedule the second timer, saving us
+ // the scheduling of the timer entirely.
+ m_restartFireTime = MonotonicTime::now() + responsivenessTimeout;
+ } else {
+ m_restartFireTime = MonotonicTime();
+ m_timer.startOneShot(responsivenessTimeout);
+ }
}
+void ResponsivenessTimer::startWithLazyStop()
+{
+ if (!m_waitingForTimer) {
+ start();
+ m_useLazyStop = true;
+ }
+}
+
void ResponsivenessTimer::stop()
{
if (!m_isResponsive) {
@@ -83,13 +121,17 @@
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();
+ invalidate();
}
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h (241350 => 241351)
--- trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h 2019-02-13 08:26:23 UTC (rev 241350)
+++ trunk/Source/WebKit/UIProcess/ResponsivenessTimer.h 2019-02-13 08:41:22 UTC (rev 241351)
@@ -46,14 +46,29 @@
explicit ResponsivenessTimer(ResponsivenessTimer::Client&);
~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();
-
+
+ // Return true if stop() was not called betfore the responsiveness timeout.
bool isResponsive() const { return m_isResponsive; }
+ // Return true if there is an active timer. The state could be responsive or not.
+ bool hasActiveTimer() const { return m_waitingForTimer; }
+
void processTerminated();
private:
@@ -60,9 +75,13 @@
void timerFired();
ResponsivenessTimer::Client& m_client;
- bool m_isResponsive;
RunLoop::Timer<ResponsivenessTimer> m_timer;
+ MonotonicTime m_restartFireTime;
+
+ bool m_isResponsive { true };
+ bool m_waitingForTimer { false };
+ bool m_useLazyStop { false };
};
} // namespace WebKit
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.cpp (241350 => 241351)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-02-13 08:26:23 UTC (rev 241350)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.cpp 2019-02-13 08:41:22 UTC (rev 241351)
@@ -2232,15 +2232,19 @@
ASSERT(!m_mouseEventQueue.isEmpty());
const NativeWebMouseEvent& event = m_mouseEventQueue.first();
-
+
if (pageClient().windowIsFrontWindowUnderMouse(event))
setToolTip(String());
- // NOTE: This does not start the responsiveness timer because mouse move should not indicate interaction.
- if (event.type() != WebEvent::MouseMove)
+ WebEvent::Type eventType = event.type();
+ if (eventType == WebEvent::MouseDown || eventType == WebEvent::MouseForceChanged || eventType == WebEvent::MouseForceDown)
+ m_process->responsivenessTimer().startWithLazyStop();
+ else if (eventType != WebEvent::MouseMove) {
+ // NOTE: This does not start the responsiveness timer because mouse move should not indicate interaction.
m_process->responsivenessTimer().start();
+ }
- LOG(MouseHandling, "UIProcess: sent mouse event %s (queue size %zu)", webMouseEventTypeString(event.type()), m_mouseEventQueue.size());
+ LOG(MouseHandling, "UIProcess: sent mouse event %s (queue size %zu)", webMouseEventTypeString(eventType), m_mouseEventQueue.size());
m_process->send(Messages::WebPage::MouseEvent(event), m_pageID);
}
@@ -2360,7 +2364,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
@@ -2393,7 +2397,12 @@
m_keyEventQueue.append(event);
- m_process->responsivenessTimer().start();
+ ResponsivenessTimer& responsivenessTimer = m_process->responsivenessTimer();
+ if (event.type() == WebEvent::KeyDown)
+ responsivenessTimer.startWithLazyStop();
+ else
+ responsivenessTimer.start();
+
if (m_keyEventQueue.size() == 1) { // Otherwise, sent from DidReceiveEvent message handler.
LOG(KeyHandling, " UI process: sent keyEvent from handleKeyboardEvent");
m_process->send(Messages::WebPage::KeyEvent(event), m_pageID);
@@ -2555,8 +2564,13 @@
m_gestureEventQueue.append(event);
// FIXME: Consider doing some coalescing here.
- m_process->responsivenessTimer().start();
+ ResponsivenessTimer& responsivenessTimer = m_process->responsivenessTimer();
+ if (event.type() == WebEvent::GestureStart || event.type() == WebEvent::GestureChange)
+ responsivenessTimer.startWithLazyStop();
+ else
+ responsivenessTimer.start();
+
m_process->send(Messages::EventDispatcher::GestureEvent(m_pageID, event), 0);
}
#endif
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp (241350 => 241351)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-02-13 08:26:23 UTC (rev 241350)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp 2019-02-13 08:41:22 UTC (rev 241351)
@@ -1133,6 +1133,19 @@
send(Messages::WebProcess::MainThreadPing(), 0);
}
+void WebProcessProxy::isResponsiveWithLazyStop()
+{
+ if (m_isResponsive == NoOrMaybe::No)
+ return;
+
+ if (!responsivenessTimer().hasActiveTimer()) {
+ // We do not send a ping if we are already waiting for the WebProcess.
+ // Spamming pings on a slow web process is not helpful.
+ responsivenessTimer().startWithLazyStop();
+ send(Messages::WebProcess::MainThreadPing(), 0);
+ }
+}
+
bool WebProcessProxy::isJITEnabled() const
{
return processPool().configuration().isJITEnabled();
Modified: trunk/Source/WebKit/UIProcess/WebProcessProxy.h (241350 => 241351)
--- trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-02-13 08:26:23 UTC (rev 241350)
+++ trunk/Source/WebKit/UIProcess/WebProcessProxy.h 2019-02-13 08:41:22 UTC (rev 241351)
@@ -202,6 +202,7 @@
ProcessThrottler& throttler() { return m_throttler; }
void isResponsive(WTF::Function<void(bool isWebProcessResponsive)>&&);
+ void isResponsiveWithLazyStop();
void didReceiveMainThreadPing();
void didReceiveBackgroundResponsivenessPing();
