Modified: trunk/Source/WebCore/ChangeLog (128001 => 128002)
--- trunk/Source/WebCore/ChangeLog 2012-09-09 22:20:18 UTC (rev 128001)
+++ trunk/Source/WebCore/ChangeLog 2012-09-09 22:26:52 UTC (rev 128002)
@@ -1,3 +1,40 @@
+2012-09-09 Andreas Kling <[email protected]>
+
+ EventListenerMap: Use Vector instead of HashMap as backend.
+ <http://webkit.org/b/77982>
+
+ Reviewed by Geoff Garen.
+
+ Refactor EventListenerMap to store pair<AtomicString, EventListenerVector> in a Vector
+ instead of using key/value HashMap stores. This is much more space efficient and actually
+ faster since we were spending more time/effort managing the hash map than it costs us
+ to iterate over and compare a couple of pointers. (It's very rare to have more than
+ 4 different event types registered in a single EventListenerMap.)
+
+ This gets rid of the slightly hacky optimization for nodes with listeners of a single type,
+ reducing the complexity of EventListenerMap greatly.
+
+ ~1.1MB progression on Membuster. Also strong (20+ MB on larger patches) savings on WebKit
+ bugzilla review pages, though they don't necessarily represent a common usecase.
+
+ * dom/EventListenerMap.cpp:
+ (WebCore::EventListenerMap::contains):
+ (WebCore::EventListenerMap::clear):
+ (WebCore::EventListenerMap::eventTypes):
+ (WebCore::EventListenerMap::add):
+ (WebCore::EventListenerMap::remove):
+ (WebCore::EventListenerMap::find):
+ (WebCore::EventListenerMap::removeFirstEventListenerCreatedFromMarkup):
+ (WebCore::EventListenerMap::copyEventListenersNotCreatedFromMarkupToTarget):
+ (WebCore::EventListenerIterator::EventListenerIterator):
+ (WebCore::EventListenerIterator::nextListener):
+ * dom/EventListenerMap.h:
+ (WebCore::EventListenerMap::isEmpty):
+ (WebCore::EventListenerMapEntry::EventListenerMapEntry):
+ (EventListenerMapEntry):
+ (EventListenerMap):
+ (EventListenerIterator):
+
2012-09-09 Lucas Forschler <[email protected]>
Build Fix. <rdar://problem/12260893>
Modified: trunk/Source/WebCore/dom/EventListenerMap.cpp (128001 => 128002)
--- trunk/Source/WebCore/dom/EventListenerMap.cpp 2012-09-09 22:20:18 UTC (rev 128001)
+++ trunk/Source/WebCore/dom/EventListenerMap.cpp 2012-09-09 22:26:52 UTC (rev 128002)
@@ -69,43 +69,29 @@
{
}
-bool EventListenerMap::isEmpty() const
-{
- if (m_hashMap)
- return m_hashMap->isEmpty();
- return !m_singleEventListenerVector;
-}
-
bool EventListenerMap::contains(const AtomicString& eventType) const
{
- if (m_hashMap)
- return m_hashMap->contains(eventType);
- return m_singleEventListenerType == eventType;
+ for (unsigned i = 0; i < m_entries.size(); ++i) {
+ if (m_entries[i].first == eventType)
+ return true;
+ }
+ return false;
}
void EventListenerMap::clear()
{
assertNoActiveIterators();
- if (m_hashMap)
- m_hashMap.clear();
- else {
- m_singleEventListenerType = nullAtom;
- m_singleEventListenerVector.clear();
- }
+ m_entries.clear();
}
Vector<AtomicString> EventListenerMap::eventTypes() const
{
Vector<AtomicString> types;
+ types.reserveInitialCapacity(m_entries.size());
- if (m_hashMap) {
- EventListenerHashMap::iterator it = m_hashMap->begin();
- EventListenerHashMap::iterator end = m_hashMap->end();
- for (; it != end; ++it)
- types.append(it->first);
- } else if (m_singleEventListenerVector)
- types.append(m_singleEventListenerType);
+ for (unsigned i = 0; i < m_entries.size(); ++i)
+ types.uncheckedAppend(m_entries[i].first);
return types;
}
@@ -125,30 +111,13 @@
{
assertNoActiveIterators();
- if (m_singleEventListenerVector && m_singleEventListenerType != eventType) {
- // We already have a single (first) listener vector, and this event is not
- // of that type, so create the hash map and move the first listener vector there.
- ASSERT(!m_hashMap);
- m_hashMap = adoptPtr(new EventListenerHashMap);
- m_hashMap->add(m_singleEventListenerType, m_singleEventListenerVector.release());
- m_singleEventListenerType = nullAtom;
+ for (unsigned i = 0; i < m_entries.size(); ++i) {
+ if (m_entries[i].first == eventType)
+ return addListenerToVector(m_entries[i].second.get(), listener, useCapture);
}
- if (m_hashMap) {
- EventListenerHashMap::AddResult result = m_hashMap->add(eventType, nullptr);
- if (result.isNewEntry)
- result.iterator->second = adoptPtr(new EventListenerVector);
-
- return addListenerToVector(result.iterator->second.get(), listener, useCapture);
- }
-
- if (!m_singleEventListenerVector) {
- m_singleEventListenerType = eventType;
- m_singleEventListenerVector = adoptPtr(new EventListenerVector);
- }
-
- ASSERT(m_singleEventListenerType == eventType);
- return addListenerToVector(m_singleEventListenerVector.get(), listener, useCapture);
+ m_entries.append(std::make_pair(eventType, adoptPtr(new EventListenerVector)));
+ return addListenerToVector(m_entries.last().second.get(), listener, useCapture);
}
static bool removeListenerFromVector(EventListenerVector* listenerVector, EventListener* listener, bool useCapture, size_t& indexOfRemovedListener)
@@ -165,41 +134,27 @@
{
assertNoActiveIterators();
- if (!m_hashMap) {
- if (m_singleEventListenerType != eventType)
- return false;
- bool wasRemoved = removeListenerFromVector(m_singleEventListenerVector.get(), listener, useCapture, indexOfRemovedListener);
- if (m_singleEventListenerVector->isEmpty()) {
- m_singleEventListenerVector.clear();
- m_singleEventListenerType = nullAtom;
+ for (unsigned i = 0; i < m_entries.size(); ++i) {
+ if (m_entries[i].first == eventType) {
+ bool wasRemoved = removeListenerFromVector(m_entries[i].second.get(), listener, useCapture, indexOfRemovedListener);
+ if (m_entries[i].second->isEmpty())
+ m_entries.remove(i);
+ return wasRemoved;
}
- return wasRemoved;
}
- EventListenerHashMap::iterator it = m_hashMap->find(eventType);
- if (it == m_hashMap->end())
- return false;
-
- bool wasRemoved = removeListenerFromVector(it->second.get(), listener, useCapture, indexOfRemovedListener);
- if (it->second->isEmpty())
- m_hashMap->remove(it);
- return wasRemoved;
+ return false;
}
EventListenerVector* EventListenerMap::find(const AtomicString& eventType)
{
assertNoActiveIterators();
- if (m_hashMap) {
- EventListenerHashMap::iterator it = m_hashMap->find(eventType);
- if (it == m_hashMap->end())
- return 0;
- return it->second.get();
+ for (unsigned i = 0; i < m_entries.size(); ++i) {
+ if (m_entries[i].first == eventType)
+ return m_entries[i].second.get();
}
- if (m_singleEventListenerType == eventType)
- return m_singleEventListenerVector.get();
-
return 0;
}
@@ -224,29 +179,14 @@
{
assertNoActiveIterators();
- if (m_hashMap) {
- EventListenerHashMap::iterator result = m_hashMap->find(eventType);
- ASSERT(result != m_hashMap->end());
-
- EventListenerVector* listenerVector = result->second.get();
- ASSERT(listenerVector);
-
- removeFirstListenerCreatedFromMarkup(listenerVector);
-
- if (listenerVector->isEmpty())
- m_hashMap->remove(result);
-
- return;
+ for (unsigned i = 0; i < m_entries.size(); ++i) {
+ if (m_entries[i].first == eventType) {
+ removeFirstListenerCreatedFromMarkup(m_entries[i].second.get());
+ if (m_entries[i].second->isEmpty())
+ m_entries.remove(i);
+ return;
+ }
}
-
- ASSERT(m_singleEventListenerVector);
- ASSERT(m_singleEventListenerType == eventType);
-
- removeFirstListenerCreatedFromMarkup(m_singleEventListenerVector.get());
- if (m_singleEventListenerVector->isEmpty()) {
- m_singleEventListenerVector.clear();
- m_singleEventListenerType = nullAtom;
- }
}
static void copyListenersNotCreatedFromMarkupToTarget(const AtomicString& eventType, EventListenerVector* listenerVector, EventTarget* target)
@@ -263,29 +203,22 @@
{
assertNoActiveIterators();
- if (m_hashMap) {
- EventListenerHashMap::iterator end = m_hashMap->end();
- for (EventListenerHashMap::iterator it = m_hashMap->begin(); it != end; ++it)
- copyListenersNotCreatedFromMarkupToTarget(it->first, it->second.get(), target);
- return;
- }
-
- if (!m_singleEventListenerVector)
- return;
-
- copyListenersNotCreatedFromMarkupToTarget(m_singleEventListenerType, m_singleEventListenerVector.get(), target);
+ for (unsigned i = 0; i < m_entries.size(); ++i)
+ copyListenersNotCreatedFromMarkupToTarget(m_entries[i].first, m_entries[i].second.get(), target);
}
#endif // ENABLE(SVG)
EventListenerIterator::EventListenerIterator()
: m_map(0)
+ , m_entryIndex(0)
, m_index(0)
{
}
EventListenerIterator::EventListenerIterator(EventTarget* target)
: m_map(0)
+ , m_entryIndex(0)
, m_index(0)
{
ASSERT(target);
@@ -302,11 +235,6 @@
m_map->m_activeIteratorCount++;
}
#endif
-
- if (m_map->m_hashMap) {
- m_mapIterator = m_map->m_hashMap->begin();
- m_mapEnd = m_map->m_hashMap->end();
- }
}
#ifndef NDEBUG
@@ -324,21 +252,13 @@
if (!m_map)
return 0;
- if (m_map->m_hashMap) {
- for (; m_mapIterator != m_mapEnd; ++m_mapIterator) {
- EventListenerVector& listeners = *m_mapIterator->second;
- if (m_index < listeners.size())
- return listeners[m_index++].listener.get();
- m_index = 0;
- }
- return 0;
+ for (; m_entryIndex < m_map->m_entries.size(); ++m_entryIndex) {
+ EventListenerVector& listeners = *m_map->m_entries[m_entryIndex].second;
+ if (m_index < listeners.size())
+ return listeners[m_index++].listener.get();
+ m_index = 0;
}
- if (!m_map->m_singleEventListenerVector)
- return 0;
- EventListenerVector& listeners = *m_map->m_singleEventListenerVector;
- if (m_index < listeners.size())
- return listeners[m_index++].listener.get();
return 0;
}
Modified: trunk/Source/WebCore/dom/EventListenerMap.h (128001 => 128002)
--- trunk/Source/WebCore/dom/EventListenerMap.h 2012-09-09 22:20:18 UTC (rev 128001)
+++ trunk/Source/WebCore/dom/EventListenerMap.h 2012-09-09 22:26:52 UTC (rev 128002)
@@ -2,7 +2,7 @@
* Copyright (C) 1999 Lars Knoll ([email protected])
* (C) 1999 Antti Koivisto ([email protected])
* (C) 2001 Dirk Mueller ([email protected])
- * Copyright (C) 2004, 2005, 2006, 2007 Apple Inc. All rights reserved.
+ * Copyright (C) 2004, 2005, 2006, 2007, 2012 Apple Inc. All rights reserved.
* Copyright (C) 2006 Alexey Proskuryakov ([email protected])
* (C) 2007, 2008 Nikolas Zimmermann <[email protected]>
* Copyright (C) 2011 Andreas Kling ([email protected])
@@ -35,7 +35,7 @@
#include "RegisteredEventListener.h"
#include <wtf/Forward.h>
-#include <wtf/HashMap.h>
+#include <wtf/PassOwnPtr.h>
#include <wtf/text/AtomicStringHash.h>
namespace WebCore {
@@ -48,7 +48,7 @@
public:
EventListenerMap();
- bool isEmpty() const;
+ bool isEmpty() const { return m_entries.isEmpty(); }
bool contains(const AtomicString& eventType) const;
void clear();
@@ -67,16 +67,8 @@
void assertNoActiveIterators();
- struct EventListenerHashMapTraits : HashTraits<WTF::AtomicString> {
- static const int minimumTableSize = 32;
- };
- typedef HashMap<AtomicString, OwnPtr<EventListenerVector>, AtomicStringHash, EventListenerHashMapTraits> EventListenerHashMap;
+ Vector<std::pair<AtomicString, OwnPtr<EventListenerVector> >, 2> m_entries;
- OwnPtr<EventListenerHashMap> m_hashMap;
-
- AtomicString m_singleEventListenerType;
- OwnPtr<EventListenerVector> m_singleEventListenerVector;
-
#ifndef NDEBUG
int m_activeIteratorCount;
#endif
@@ -95,8 +87,7 @@
private:
EventListenerMap* m_map;
- EventListenerMap::EventListenerHashMap::iterator m_mapIterator;
- EventListenerMap::EventListenerHashMap::iterator m_mapEnd;
+ unsigned m_entryIndex;
unsigned m_index;
};
