<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[179850] trunk/Source/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/179850">179850</a></dd>
<dt>Author</dt> <dd>timothy_horton@apple.com</dd>
<dt>Date</dt> <dd>2015-02-09 15:39:41 -0800 (Mon, 09 Feb 2015)</dd>
</dl>

<h3>Log Message</h3>
<pre>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&lt;DisplayRefreshMonitor&gt; instead of a HashMap
from uint64_t to RefPtr&lt;DisplayRefreshMonitor&gt;. 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.</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsDisplayRefreshMonitorManagercpp">trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp</a></li>
<li><a href="#trunkSourceWebCoreplatformgraphicsDisplayRefreshMonitorManagerh">trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (179849 => 179850)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog        2015-02-09 23:26:22 UTC (rev 179849)
+++ trunk/Source/WebCore/ChangeLog        2015-02-09 23:39:41 UTC (rev 179850)
</span><span class="lines">@@ -1,3 +1,24 @@
</span><ins>+2015-02-09  Timothy Horton  &lt;timothy_horton@apple.com&gt;
+
+        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&lt;DisplayRefreshMonitor&gt; instead of a HashMap
+        from uint64_t to RefPtr&lt;DisplayRefreshMonitor&gt;. 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.
+
</ins><span class="cx"> 2015-02-09  Jer Noble  &lt;jer.noble@apple.com&gt;
</span><span class="cx"> 
</span><span class="cx">         [Mac] Disable the currentTime estimation code in HTMLMediaElement for Yosemite+
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsDisplayRefreshMonitorManagercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp (179849 => 179850)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp        2015-02-09 23:26:22 UTC (rev 179849)
+++ trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.cpp        2015-02-09 23:39:41 UTC (rev 179850)
</span><span class="lines">@@ -46,16 +46,19 @@
</span><span class="cx"> 
</span><span class="cx"> DisplayRefreshMonitor* DisplayRefreshMonitorManager::ensureMonitorForClient(DisplayRefreshMonitorClient* client)
</span><span class="cx"> {
</span><del>-    DisplayRefreshMonitorMap::iterator it = m_monitors.find(client-&gt;displayID());
-    if (it == m_monitors.end()) {
-        RefPtr&lt;DisplayRefreshMonitor&gt; monitor = DisplayRefreshMonitor::create(client);
</del><ins>+    PlatformDisplayID clientDisplayID = client-&gt;displayID();
+    for (const RefPtr&lt;DisplayRefreshMonitor&gt;&amp; monitor : m_monitors) {
+        if (monitor-&gt;displayID() != clientDisplayID)
+            continue;
</ins><span class="cx">         monitor-&gt;addClient(client);
</span><del>-        DisplayRefreshMonitor* result = monitor.get();
-        m_monitors.add(client-&gt;displayID(), monitor.release());
-        return result;
</del><ins>+        return monitor.get();
</ins><span class="cx">     }
</span><del>-    it-&gt;value-&gt;addClient(client);
-    return it-&gt;value.get();
</del><ins>+
+    RefPtr&lt;DisplayRefreshMonitor&gt; monitor = DisplayRefreshMonitor::create(client);
+    monitor-&gt;addClient(client);
+    DisplayRefreshMonitor* result = monitor.get();
+    m_monitors.append(monitor.release());
+    return result;
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void DisplayRefreshMonitorManager::registerClient(DisplayRefreshMonitorClient* client)
</span><span class="lines">@@ -71,14 +74,16 @@
</span><span class="cx">     if (!client-&gt;hasDisplayID())
</span><span class="cx">         return;
</span><span class="cx"> 
</span><del>-    DisplayRefreshMonitorMap::iterator it = m_monitors.find(client-&gt;displayID());
-    if (it == m_monitors.end())
</del><ins>+    PlatformDisplayID clientDisplayID = client-&gt;displayID();
+    for (size_t i = 0; i &lt; m_monitors.size(); ++i) {
+        RefPtr&lt;DisplayRefreshMonitor&gt; monitor = m_monitors[i];
+        if (monitor-&gt;displayID() != clientDisplayID)
+            continue;
+        if (monitor-&gt;removeClient(client)) {
+            if (!monitor-&gt;hasClients())
+                m_monitors.remove(i);
+        }
</ins><span class="cx">         return;
</span><del>-
-    DisplayRefreshMonitor* monitor = it-&gt;value.get();
-    if (monitor-&gt;removeClient(client)) {
-        if (!monitor-&gt;hasClients())
-            m_monitors.remove(it);
</del><span class="cx">     }
</span><span class="cx"> }
</span><span class="cx"> 
</span><span class="lines">@@ -95,10 +100,12 @@
</span><span class="cx"> 
</span><span class="cx"> void DisplayRefreshMonitorManager::displayDidRefresh(DisplayRefreshMonitor* monitor)
</span><span class="cx"> {
</span><del>-    if (monitor-&gt;shouldBeTerminated()) {
-        ASSERT(m_monitors.contains(monitor-&gt;displayID()));
-        m_monitors.remove(monitor-&gt;displayID());
-    }
</del><ins>+    if (!monitor-&gt;shouldBeTerminated())
+        return;
+
+    size_t monitorIndex = m_monitors.find(monitor);
+    ASSERT(monitorIndex != notFound);
+    m_monitors.remove(monitorIndex);
</ins><span class="cx"> }
</span><span class="cx"> 
</span><span class="cx"> void DisplayRefreshMonitorManager::windowScreenDidChange(PlatformDisplayID displayID, DisplayRefreshMonitorClient* client)
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformgraphicsDisplayRefreshMonitorManagerh"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h (179849 => 179850)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h        2015-02-09 23:26:22 UTC (rev 179849)
+++ trunk/Source/WebCore/platform/graphics/DisplayRefreshMonitorManager.h        2015-02-09 23:39:41 UTC (rev 179850)
</span><span class="lines">@@ -30,9 +30,9 @@
</span><span class="cx"> 
</span><span class="cx"> #include &quot;DisplayRefreshMonitor.h&quot;
</span><span class="cx"> #include &quot;PlatformScreen.h&quot;
</span><del>-#include &lt;wtf/HashMap.h&gt;
</del><span class="cx"> #include &lt;wtf/NeverDestroyed.h&gt;
</span><span class="cx"> #include &lt;wtf/RefPtr.h&gt;
</span><ins>+#include &lt;wtf/Vector.h&gt;
</ins><span class="cx"> 
</span><span class="cx"> namespace WebCore {
</span><span class="cx"> 
</span><span class="lines">@@ -56,11 +56,7 @@
</span><span class="cx"> 
</span><span class="cx">     DisplayRefreshMonitor* ensureMonitorForClient(DisplayRefreshMonitorClient*);
</span><span class="cx"> 
</span><del>-    // 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&lt;uint64_t, RefPtr&lt;DisplayRefreshMonitor&gt;, WTF::IntHash&lt;uint64_t&gt;, WTF::UnsignedWithZeroKeyHashTraits&lt;uint64_t&gt;&gt; DisplayRefreshMonitorMap;
-    DisplayRefreshMonitorMap m_monitors;
</del><ins>+    Vector&lt;RefPtr&lt;DisplayRefreshMonitor&gt;&gt; m_monitors;
</ins><span class="cx"> };
</span><span class="cx"> 
</span><span class="cx"> }
</span></span></pre>
</div>
</div>

</body>
</html>