Diff
Modified: trunk/Source/WebCore/ChangeLog (266709 => 266710)
--- trunk/Source/WebCore/ChangeLog 2020-09-07 19:36:53 UTC (rev 266709)
+++ trunk/Source/WebCore/ChangeLog 2020-09-07 20:55:31 UTC (rev 266710)
@@ -1,3 +1,17 @@
+2020-09-07 Commit Queue <[email protected]>
+
+ Unreviewed, reverting r266645.
+ https://bugs.webkit.org/show_bug.cgi?id=216251
+
+ Caused MotionMark regression
+
+ Reverted changeset:
+
+ "Move lazy DisplayLink tear down logic from the WebProcess to
+ the UIProcess"
+ https://bugs.webkit.org/show_bug.cgi?id=216195
+ https://trac.webkit.org/changeset/266645
+
2020-09-07 Sam Weinig <[email protected]>
[WebIDL] Fix issues found by preprocess-idls.pl parser validation and enabled parser validation by default for the tests
Modified: trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp (266709 => 266710)
--- trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp 2020-09-07 19:36:53 UTC (rev 266709)
+++ trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitor.cpp 2020-09-07 20:55:31 UTC (rev 266710)
@@ -94,7 +94,12 @@
{
{
LockHolder lock(m_mutex);
- LOG(RequestAnimationFrame, "DisplayRefreshMonitor::displayDidRefresh(%p) - m_scheduled(%d)", this, m_scheduled);
+ LOG(RequestAnimationFrame, "DisplayRefreshMonitor::displayDidRefresh(%p) - m_scheduled(%d), m_unscheduledFireCount(%d)", this, m_scheduled, m_unscheduledFireCount);
+ if (!m_scheduled)
+ ++m_unscheduledFireCount;
+ else
+ m_unscheduledFireCount = 0;
+
m_scheduled = false;
}
Modified: trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h (266709 => 266710)
--- trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h 2020-09-07 19:36:53 UTC (rev 266709)
+++ trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitor.h 2020-09-07 20:55:31 UTC (rev 266710)
@@ -60,7 +60,11 @@
PlatformDisplayID displayID() const { return m_displayID; }
- bool shouldBeTerminated() const { return !m_scheduled; }
+ bool shouldBeTerminated() const
+ {
+ const int maxInactiveFireCount = 20;
+ return !m_scheduled && m_unscheduledFireCount > maxInactiveFireCount;
+ }
static RefPtr<DisplayRefreshMonitor> createDefaultDisplayRefreshMonitor(PlatformDisplayID);
@@ -90,6 +94,7 @@
HashSet<DisplayRefreshMonitorClient*>* m_clientsToBeNotified { nullptr };
Lock m_mutex;
PlatformDisplayID m_displayID { 0 };
+ int m_unscheduledFireCount { 0 }; // Number of times the display link has fired with no clients.
bool m_active { true };
bool m_scheduled { false };
bool m_previousFrameDone { true };
Modified: trunk/Source/WebKit/ChangeLog (266709 => 266710)
--- trunk/Source/WebKit/ChangeLog 2020-09-07 19:36:53 UTC (rev 266709)
+++ trunk/Source/WebKit/ChangeLog 2020-09-07 20:55:31 UTC (rev 266710)
@@ -1,3 +1,17 @@
+2020-09-07 Commit Queue <[email protected]>
+
+ Unreviewed, reverting r266645.
+ https://bugs.webkit.org/show_bug.cgi?id=216251
+
+ Caused MotionMark regression
+
+ Reverted changeset:
+
+ "Move lazy DisplayLink tear down logic from the WebProcess to
+ the UIProcess"
+ https://bugs.webkit.org/show_bug.cgi?id=216195
+ https://trac.webkit.org/changeset/266645
+
2020-09-07 Mike Gorse <[email protected]>
Build failure; cannot find seccomp.h
Modified: trunk/Source/WebKit/UIProcess/mac/DisplayLink.cpp (266709 => 266710)
--- trunk/Source/WebKit/UIProcess/mac/DisplayLink.cpp 2020-09-07 19:36:53 UTC (rev 266709)
+++ trunk/Source/WebKit/UIProcess/mac/DisplayLink.cpp 2020-09-07 20:55:31 UTC (rev 266710)
@@ -32,8 +32,6 @@
#include <wtf/ProcessPrivilege.h>
namespace WebKit {
-
-constexpr unsigned maxFireCountWithObservers { 20 };
DisplayLink::DisplayLink(WebCore::PlatformDisplayID displayID)
: m_displayID(displayID)
@@ -74,13 +72,16 @@
void DisplayLink::addObserver(IPC::Connection& connection, DisplayLinkObserverID observerID)
{
ASSERT(RunLoop::isMain());
+ bool isRunning = !m_observers.isEmpty();
- LockHolder locker(m_observersLock);
- m_observers.ensure(&connection, [] {
- return Vector<DisplayLinkObserverID> { };
- }).iterator->value.append(observerID);
+ {
+ LockHolder locker(m_observersLock);
+ m_observers.ensure(&connection, [] {
+ return Vector<DisplayLinkObserverID> { };
+ }).iterator->value.append(observerID);
+ }
- if (!CVDisplayLinkIsRunning(m_displayLink)) {
+ if (!isRunning) {
CVReturn error = CVDisplayLinkStart(m_displayLink);
if (error)
WTFLogAlways("Could not start the display link: %d", error);
@@ -91,19 +92,20 @@
{
ASSERT(RunLoop::isMain());
- LockHolder locker(m_observersLock);
+ {
+ LockHolder locker(m_observersLock);
- auto it = m_observers.find(&connection);
- if (it == m_observers.end())
- return;
- bool removed = it->value.removeFirst(observerID);
- ASSERT_UNUSED(removed, removed);
- if (it->value.isEmpty())
- m_observers.remove(it);
+ auto it = m_observers.find(&connection);
+ if (it == m_observers.end())
+ return;
+ bool removed = it->value.removeFirst(observerID);
+ ASSERT_UNUSED(removed, removed);
+ if (it->value.isEmpty())
+ m_observers.remove(it);
+ }
- // We do not stop the display link right away when |m_observers| becomes empty. Instead, we
- // let the display link fire up to |maxFireCountWithObservers| times without observers to avoid
- // killing & restarting too many threads when observers gets removed & added in quick succession.
+ if (m_observers.isEmpty())
+ CVDisplayLinkStop(m_displayLink);
}
void DisplayLink::removeObservers(IPC::Connection& connection)
@@ -110,27 +112,25 @@
{
ASSERT(RunLoop::isMain());
- LockHolder locker(m_observersLock);
- m_observers.remove(&connection);
+ {
+ LockHolder locker(m_observersLock);
+ m_observers.remove(&connection);
+ }
- // We do not stop the display link right away when |m_observers| becomes empty. Instead, we
- // let the display link fire up to |maxFireCountWithObservers| times without observers to avoid
- // killing & restarting too many threads when observers gets removed & added in quick succession.
+ if (m_observers.isEmpty())
+ CVDisplayLinkStop(m_displayLink);
}
+bool DisplayLink::hasObservers() const
+{
+ ASSERT(RunLoop::isMain());
+ return !m_observers.isEmpty();
+}
+
CVReturn DisplayLink::displayLinkCallback(CVDisplayLinkRef displayLinkRef, const CVTimeStamp*, const CVTimeStamp*, CVOptionFlags, CVOptionFlags*, void* data)
{
- ASSERT(!RunLoop::isMain());
auto* displayLink = static_cast<DisplayLink*>(data);
-
LockHolder locker(displayLink->m_observersLock);
- if (displayLink->m_observers.isEmpty()) {
- if (++(displayLink->m_fireCountWithoutObservers) >= maxFireCountWithObservers)
- CVDisplayLinkStop(displayLink->m_displayLink);
- return kCVReturnSuccess;
- }
- displayLink->m_fireCountWithoutObservers = 0;
-
for (auto& connection : displayLink->m_observers.keys())
connection->send(Messages::EventDispatcher::DisplayWasRefreshed(displayLink->m_displayID), 0);
return kCVReturnSuccess;
Modified: trunk/Source/WebKit/UIProcess/mac/DisplayLink.h (266709 => 266710)
--- trunk/Source/WebKit/UIProcess/mac/DisplayLink.h 2020-09-07 19:36:53 UTC (rev 266709)
+++ trunk/Source/WebKit/UIProcess/mac/DisplayLink.h 2020-09-07 20:55:31 UTC (rev 266710)
@@ -48,6 +48,7 @@
void addObserver(IPC::Connection&, DisplayLinkObserverID);
void removeObserver(IPC::Connection&, DisplayLinkObserverID);
void removeObservers(IPC::Connection&);
+ bool hasObservers() const;
WebCore::PlatformDisplayID displayID() const { return m_displayID; }
@@ -60,7 +61,6 @@
Lock m_observersLock;
HashMap<RefPtr<IPC::Connection>, Vector<DisplayLinkObserverID>> m_observers;
WebCore::PlatformDisplayID m_displayID;
- unsigned m_fireCountWithoutObservers { 0 };
};
} // namespace WebKit
