Avoid using a HashMap for DisplayRefreshMonitorManager, which rarely has more than...

https://bugs.webkit.org/show_bug.cgi?id=141353

Reviewed by Anders Carlsson.

No new tests, because there's no behavior change.

* platform/graphics/DisplayRefreshMonitorManager.cpp:
(WebCore::DisplayRefreshMonitorManager::ensureMonitorForClient):
(WebCore::DisplayRefreshMonitorManager::unregisterClient):
(WebCore::DisplayRefreshMonitorManager::displayDidRefresh):
* platform/graphics/DisplayRefreshMonitorManager.h:
Use a Vector of RefPtr<DisplayRefreshMonitor> instead of a HashMap
from uint64_t to RefPtr<DisplayRefreshMonitor>. There's usually only one
display, so there's usually only one DisplayRefreshMonitor. Linear search
on the Vector will be faster than the hash lookup in all conceivable cases.
This also avoids the situation mentioned in the comments in DisplayRefreshMonitorManager.h
where we don't know enough about PlatformDisplayID to safely hash it.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@179850 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index 296dcdf9a8580db3147601a01ca07d9c64da50f5..6406d185f8ff9e4ff728ee1693efaa94ad2a442e 100644 (file)
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,3 +1,24 @@
+2015-02-09  Timothy Horton  <timothy_horton@apple.com>
+
+        Avoid using a HashMap for DisplayRefreshMonitorManager, which rarely has more than one item
+        https://bugs.webkit.org/show_bug.cgi?id=141353
+
+        Reviewed by Anders Carlsson.
+
+        No new tests, because there's no behavior change.
+
+        * platform/graphics/DisplayRefreshMonitorManager.cpp:
+        (WebCore::DisplayRefreshMonitorManager::ensureMonitorForClient):
+        (WebCore::DisplayRefreshMonitorManager::unregisterClient):
+        (WebCore::DisplayRefreshMonitorManager::displayDidRefresh):
+        * platform/graphics/DisplayRefreshMonitorManager.h:
+        Use a Vector of RefPtr<DisplayRefreshMonitor> instead of a HashMap
+        from uint64_t to RefPtr<DisplayRefreshMonitor>. There's usually only one
+        display, so there's usually only one DisplayRefreshMonitor. Linear search
+        on the Vector will be faster than the hash lookup in all conceivable cases.
+        This also avoids the situation mentioned in the comments in DisplayRefreshMonitorManager.h
+        where we don't know enough about PlatformDisplayID to safely hash it.
+
 2015-02-09  Jer Noble  <jer.noble@apple.com>
 
         [Mac] Disable the currentTime estimation code in HTMLMediaElement for Yosemite+

diff --git a/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp b/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp
index 137eb92d6dafb80964bf058b14a5353e4138fa17..411c2ce16c6381250af08651dda595e9a82cda9e 100644 (file)
--- a/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp
+++ b/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp
@@ -46,16 +46,19 @@ DisplayRefreshMonitorManager& DisplayRefreshMonitorManager::sharedManager()
 
 DisplayRefreshMonitor* DisplayRefreshMonitorManager::ensureMonitorForClient(DisplayRefreshMonitorClient* client)
 {
-    DisplayRefreshMonitorMap::iterator it = m_monitors.find(client->displayID());
-    if (it == m_monitors.end()) {
-        RefPtr<DisplayRefreshMonitor> monitor = DisplayRefreshMonitor::create(client);
+    PlatformDisplayID clientDisplayID = client->displayID();
+    for (const RefPtr<DisplayRefreshMonitor>& monitor : m_monitors) {
+        if (monitor->displayID() != clientDisplayID)
+            continue;
         monitor->addClient(client);
-        DisplayRefreshMonitor* result = monitor.get();
-        m_monitors.add(client->displayID(), monitor.release());
-        return result;
+        return monitor.get();
     }
-    it->value->addClient(client);
-    return it->value.get();
+
+    RefPtr<DisplayRefreshMonitor> monitor = DisplayRefreshMonitor::create(client);
+    monitor->addClient(client);
+    DisplayRefreshMonitor* result = monitor.get();
+    m_monitors.append(monitor.release());
+    return result;
 }
 
 void DisplayRefreshMonitorManager::registerClient(DisplayRefreshMonitorClient* client)
@@ -71,14 +74,16 @@ void DisplayRefreshMonitorManager::unregisterClient(DisplayRefreshMonitorClient*
     if (!client->hasDisplayID())
         return;
 
-    DisplayRefreshMonitorMap::iterator it = m_monitors.find(client->displayID());
-    if (it == m_monitors.end())
+    PlatformDisplayID clientDisplayID = client->displayID();
+    for (size_t i = 0; i < m_monitors.size(); ++i) {
+        RefPtr<DisplayRefreshMonitor> monitor = m_monitors[i];
+        if (monitor->displayID() != clientDisplayID)
+            continue;
+        if (monitor->removeClient(client)) {
+            if (!monitor->hasClients())
+                m_monitors.remove(i);
+        }
         return;
-
-    DisplayRefreshMonitor* monitor = it->value.get();
-    if (monitor->removeClient(client)) {
-        if (!monitor->hasClients())
-            m_monitors.remove(it);
     }
 }
 
@@ -95,10 +100,12 @@ bool DisplayRefreshMonitorManager::scheduleAnimation(DisplayRefreshMonitorClient
 
 void DisplayRefreshMonitorManager::displayDidRefresh(DisplayRefreshMonitor* monitor)
 {
-    if (monitor->shouldBeTerminated()) {
-        ASSERT(m_monitors.contains(monitor->displayID()));
-        m_monitors.remove(monitor->displayID());
-    }
+    if (!monitor->shouldBeTerminated())
+        return;
+
+    size_t monitorIndex = m_monitors.find(monitor);
+    ASSERT(monitorIndex != notFound);
+    m_monitors.remove(monitorIndex);
 }
 
 void DisplayRefreshMonitorManager::windowScreenDidChange(PlatformDisplayID displayID, DisplayRefreshMonitorClient* client)

diff --git a/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h b/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h
index 296a7c9306a5e1eeeee3443abe03bd5639bac375..9c43560e2fd21f8ffd64f34ba41bb281b86fa652 100644 (file)
--- a/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h
+++ b/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h
@@ -30,9 +30,9 @@
 
 #include "DisplayRefreshMonitor.h"
 #include "PlatformScreen.h"
-#include <wtf/HashMap.h>
 #include <wtf/NeverDestroyed.h>
 #include <wtf/RefPtr.h>
+#include <wtf/Vector.h>
 
 namespace WebCore {
 
@@ -56,11 +56,7 @@ private:
 
     DisplayRefreshMonitor* ensureMonitorForClient(DisplayRefreshMonitorClient*);
 
-    // We know nothing about the values of PlatformDisplayIDs, so use UnsignedWithZeroKeyHashTraits.
-    // FIXME: Since we know nothing about these values, this is not sufficient.
-    // Even with UnsignedWithZeroKeyHashTraits, there are still two special values used for empty and deleted hash table slots.
-    typedef HashMap<uint64_t, RefPtr<DisplayRefreshMonitor>, WTF::IntHash<uint64_t>, WTF::UnsignedWithZeroKeyHashTraits<uint64_t>> DisplayRefreshMonitorMap;
-    DisplayRefreshMonitorMap m_monitors;
+    Vector<RefPtr<DisplayRefreshMonitor>> m_monitors;
 };
 
 }