Changeset 247534 in webkit

Timestamp:

Jul 17, 2019 1:43:21 PM (5 years ago)

Author:

Chris Dumez

Message:

Avoid unnecessary copy of monitors under DisplayRefreshMonitorManager::displayWasUpdated()
​https://bugs.webkit.org/show_bug.cgi?id=199847

Reviewed by Said Abou-Hallawa.

There is always one one monitor in m_monitors for a given displayID so there is
no need to copy the monitors inside DisplayRefreshMonitorManager::displayWasUpdated()
and no need to iterate over all of them. This patch also factors a bit differently
so that it becomes more obvious.

• platform/graphics/DisplayRefreshMonitorManager.cpp:

(WebCore::DisplayRefreshMonitorManager::createMonitorForClient):
(WebCore::DisplayRefreshMonitorManager::unregisterClient):
(WebCore::DisplayRefreshMonitorManager::displayDidRefresh):
(WebCore::DisplayRefreshMonitorManager::displayWasUpdated):
(WebCore::DisplayRefreshMonitorManager::findMonitorForDisplay const):
(WebCore::DisplayRefreshMonitorManager::monitorForDisplay const):

• platform/graphics/DisplayRefreshMonitorManager.h:

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/DisplayRefreshMonitorManager.cpp (modified) (4 diffs)
• platform/graphics/DisplayRefreshMonitorManager.h (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-07-17  Chris Dumez  <cdumez@apple.com>
+
+        Avoid unnecessary copy of monitors under DisplayRefreshMonitorManager::displayWasUpdated()
+        https://bugs.webkit.org/show_bug.cgi?id=199847
+
+        Reviewed by Said Abou-Hallawa.
+
+        There is always one one monitor in m_monitors for a given displayID so there is
+        no need to copy the monitors inside DisplayRefreshMonitorManager::displayWasUpdated()
+        and no need to iterate over all of them. This patch also factors a bit differently
+        so that it becomes more obvious.
+
+        * platform/graphics/DisplayRefreshMonitorManager.cpp:
+        (WebCore::DisplayRefreshMonitorManager::createMonitorForClient):
+        (WebCore::DisplayRefreshMonitorManager::unregisterClient):
+        (WebCore::DisplayRefreshMonitorManager::displayDidRefresh):
+        (WebCore::DisplayRefreshMonitorManager::displayWasUpdated):
+        (WebCore::DisplayRefreshMonitorManager::findMonitorForDisplay const):
+        (WebCore::DisplayRefreshMonitorManager::monitorForDisplay const):
+        * platform/graphics/DisplayRefreshMonitorManager.h:
+
 2019-07-17  Sihui Liu  <sihui_liu@apple.com>
 

trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp

@@ -46 +46 @@
 {
     PlatformDisplayID clientDisplayID = client.displayID();
-    for (auto& monitorWrapper : m_monitors) {
-        auto& monitor = monitorWrapper.monitor;
-        if (monitor->displayID() != clientDisplayID)
-            continue;
-        monitor->addClient(client);
-        return monitor.get();
+    if (auto* existingMonitor = monitorForDisplayID(clientDisplayID)) {
+        existingMonitor->addClient(client);
+        return existingMonitor;
     }
 
@@ -79 +76 @@
 
     PlatformDisplayID clientDisplayID = client.displayID();
-    for (size_t i = 0; i < m_monitors.size(); ++i) {
-        RefPtr<DisplayRefreshMonitor> monitor = m_monitors[i].monitor;
-        if (monitor->displayID() != clientDisplayID)
-            continue;
-        if (monitor->removeClient(client)) {
-            if (!monitor->hasClients())
-                m_monitors.remove(i);
-        }
+    auto index = findMonitorForDisplayID(clientDisplayID);
+    if (index == notFound)
         return;
+    RefPtr<DisplayRefreshMonitor> monitor = m_monitors[index].monitor;
+    if (monitor->removeClient(client)) {
+        if (!monitor->hasClients())
+            m_monitors.remove(index);
     }
 }
@@ -110 +105 @@
     LOG(RequestAnimationFrame, "DisplayRefreshMonitorManager::displayDidRefresh() - destroying monitor %p", &monitor);
 
-    size_t monitorIndex = m_monitors.findMatching([&](auto& monitorWrapper) { return monitorWrapper.monitor == &monitor; });
-    if (monitorIndex != notFound)
-        m_monitors.remove(monitorIndex);
+    m_monitors.removeFirstMatching([&](auto& monitorWrapper) {
+        return monitorWrapper.monitor == &monitor;
+    });
 }
 
@@ -129 +124 @@
 void DisplayRefreshMonitorManager::displayWasUpdated(PlatformDisplayID displayID)
 {
-    Vector<RefPtr<DisplayRefreshMonitor>> monitors = WTF::map(m_monitors, [](auto& monitorWrapper) {
-        return monitorWrapper.monitor;
+    auto* monitor = monitorForDisplayID(displayID);
+    if (monitor && monitor->hasRequestedRefreshCallback())
+        monitor->displayLinkFired();
+}
+
+size_t DisplayRefreshMonitorManager::findMonitorForDisplayID(PlatformDisplayID displayID) const
+{
+    return m_monitors.findMatching([&](auto& monitorWrapper) {
+        return monitorWrapper.monitor->displayID() == displayID;
     });
-    for (auto& monitor : monitors) {
-        if (displayID == monitor->displayID() && monitor->hasRequestedRefreshCallback())
-            monitor->displayLinkFired();
-    }
+}
+
+DisplayRefreshMonitor* DisplayRefreshMonitorManager::monitorForDisplayID(PlatformDisplayID displayID) const
+{
+    auto index = findMonitorForDisplayID(displayID);
+    return index == notFound ? nullptr : m_monitors[index].monitor.get();
 }
 

trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h

@@ -56 +56 @@
     virtual ~DisplayRefreshMonitorManager();
 
+    size_t findMonitorForDisplayID(PlatformDisplayID) const;
+    DisplayRefreshMonitor* monitorForDisplayID(PlatformDisplayID) const;
+
     DisplayRefreshMonitor* createMonitorForClient(DisplayRefreshMonitorClient&);