Reviewed by Maciej.
authorap@webkit.org <ap@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Nov 2007 06:43:49 +0000 (06:43 +0000)
committerap@webkit.org <ap@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 23 Nov 2007 06:43:49 +0000 (06:43 +0000)
        http://bugs.webkit.org/show_bug.cgi?id=14977
        Hixie's DOM Core performance test shows insert >10x slower than append

        Each invocation of Element.childNodes[n] was creating and registering a new ChildNodeList,
        which persisted and listened to notifications until GC.

        A fix is to avoid registering child node lists for notifications - they don't need them, as
        they share a common cache in Node.

        * dom/Node.cpp:
        (WebCore::Node::registerNodeList):
        (WebCore::Node::unregisterNodeList):
        (WebCore::Node::notifyLocalNodeListsAttributeChanged):
        (WebCore::Node::notifyLocalNodeListsChildrenChanged):
        * dom/NodeList.h:
        (WebCore::NodeList::needsNotifications):

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

WebCore/ChangeLog
WebCore/dom/Node.cpp
WebCore/dom/NodeList.h

index 3987dcfaf36c6d49f5283fa08bc89de8b0711bef..d9511bee78f714e9d60fbefbadafc5c1f1a7ea48 100644 (file)
@@ -1,3 +1,24 @@
+2007-11-22  Alexey Proskuryakov  <ap@webkit.org>
+
+        Reviewed by Maciej.
+
+        http://bugs.webkit.org/show_bug.cgi?id=14977
+        Hixie's DOM Core performance test shows insert >10x slower than append
+
+        Each invocation of Element.childNodes[n] was creating and registering a new ChildNodeList,
+        which persisted and listened to notifications until GC.
+
+        A fix is to avoid registering child node lists for notifications - they don't need them, as
+        they share a common cache in Node.
+
+        * dom/Node.cpp:
+        (WebCore::Node::registerNodeList):
+        (WebCore::Node::unregisterNodeList):
+        (WebCore::Node::notifyLocalNodeListsAttributeChanged):
+        (WebCore::Node::notifyLocalNodeListsChildrenChanged):
+        * dom/NodeList.h:
+        (WebCore::NodeList::needsNotifications):
+
 2007-11-22  Dan Bernstein  <mitz@apple.com>
 
         Reviewed by Antti Koivisto.
index 3f798559b8268a2192c23215b26396f64b1925b7..dba01bd581e76ecd3e46fec091a7b2091a9fb27b 100644 (file)
@@ -48,7 +48,7 @@ using namespace HTMLNames;
 
 typedef HashSet<NodeList*> NodeListSet;
 struct NodeListsNodeData {
-    NodeListSet m_registeredLists;
+    NodeListSet m_listsToNotify;
     NodeList::Caches m_childNodeListCaches;
 };
 
@@ -439,10 +439,12 @@ void Node::registerNodeList(NodeList* list)
 {
     if (!m_nodeLists)
         m_nodeLists = new NodeListsNodeData;
-    else if (m_nodeLists->m_registeredLists.isEmpty()) 
+    else if (!m_document->hasNodeLists())
+        // We haven't been receiving notifications while there were no registered lists, so the cache is invalid now.
         m_nodeLists->m_childNodeListCaches.reset();
 
-    m_nodeLists->m_registeredLists.add(list);
+    if (list->needsNotifications())
+        m_nodeLists->m_listsToNotify.add(list);
     m_document->addNodeList();
 }
 
@@ -450,7 +452,8 @@ void Node::unregisterNodeList(NodeList* list)
 {
     ASSERT(m_nodeLists);
     m_document->removeNodeList();
-    m_nodeLists->m_registeredLists.remove(list);
+    if (list->needsNotifications())
+        m_nodeLists->m_listsToNotify.remove(list);
 }
 
 void Node::notifyLocalNodeListsAttributeChanged()
@@ -458,8 +461,8 @@ void Node::notifyLocalNodeListsAttributeChanged()
     if (!m_nodeLists)
         return;
 
-    NodeListSet::iterator end = m_nodeLists->m_registeredLists.end();
-    for (NodeListSet::iterator i = m_nodeLists->m_registeredLists.begin(); i != end; ++i)
+    NodeListSet::iterator end = m_nodeLists->m_listsToNotify.end();
+    for (NodeListSet::iterator i = m_nodeLists->m_listsToNotify.begin(); i != end; ++i)
         (*i)->rootNodeAttributeChanged();
 }
 
@@ -476,8 +479,8 @@ void Node::notifyLocalNodeListsChildrenChanged()
 
     m_nodeLists->m_childNodeListCaches.reset();
 
-    NodeListSet::iterator end = m_nodeLists->m_registeredLists.end();
-    for (NodeListSet::iterator i = m_nodeLists->m_registeredLists.begin(); i != end; ++i)
+    NodeListSet::iterator end = m_nodeLists->m_listsToNotify.end();
+    for (NodeListSet::iterator i = m_nodeLists->m_listsToNotify.begin(); i != end; ++i)
         (*i)->rootNodeChildrenChanged();
 }
 
index 988aa772df5b66f2eec18726fe8310b9345fd65e..cf531c215d9a7c84d6fc784e9abc49252d91129c 100644 (file)
@@ -53,6 +53,8 @@ public:
     NodeList(PassRefPtr<Node> rootNode, Caches*);
     virtual ~NodeList();
 
+    bool needsNotifications() const { return m_ownsCaches; }
+
     // DOM methods & attributes for NodeList
     virtual unsigned length() const = 0;
     virtual Node* item(unsigned index) const = 0;