Reviewed by Kevin.
authormjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 14 Nov 2004 02:07:49 +0000 (02:07 +0000)
committermjs <mjs@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 14 Nov 2004 02:07:49 +0000 (02:07 +0000)
<rdar://problem/3878766> VIP: Program listings pages at directv.com takes 75% of time traversing NodeLists

        * khtml/dom/dom_node.cpp:
        (NodeList::itemById): New method, just forward to impl.
        * khtml/dom/dom_node.h: Prototype it.
        * khtml/ecma/kjs_dom.cpp:
        (DOMNodeList::tryGet): Instead of looping over the whole list to do by-id access,
let the NodeList do it. The NodeList might be able to do it more efficiently.
        * khtml/xml/dom_nodeimpl.cpp:
(NodeListImpl::itemById): Optimize for the case where the NodeList
covers the whole document. In this case, just use getElementById,
then check that the element satisfies the list criteria.
        (ChildNodeListImpl::nodeMatches): Return true only if the node is our child.
        (TagNodeListImpl::TagNodeListImpl): Irrelevant change to reformat initializers.
        * khtml/xml/dom_nodeimpl.h:

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

WebCore/ChangeLog-2005-08-23
WebCore/khtml/dom/dom_node.cpp
WebCore/khtml/dom/dom_node.h
WebCore/khtml/ecma/kjs_dom.cpp
WebCore/khtml/xml/dom_nodeimpl.cpp
WebCore/khtml/xml/dom_nodeimpl.h

index 02b6497..1ccf30c 100644 (file)
@@ -1,3 +1,23 @@
+2004-11-13  Maciej Stachowiak  <mjs@apple.com>
+
+        Reviewed by Kevin.
+
+       <rdar://problem/3878766> VIP: Program listings pages at directv.com takes 75% of time traversing NodeLists
+
+        * khtml/dom/dom_node.cpp:
+        (NodeList::itemById): New method, just forward to impl.
+        * khtml/dom/dom_node.h: Prototype it.
+        * khtml/ecma/kjs_dom.cpp:
+        (DOMNodeList::tryGet): Instead of looping over the whole list to do by-id access,
+       let the NodeList do it. The NodeList might be able to do it more efficiently.
+        * khtml/xml/dom_nodeimpl.cpp:
+       (NodeListImpl::itemById): Optimize for the case where the NodeList
+       covers the whole document. In this case, just use getElementById,
+       then check that the element satisfies the list criteria.
+        (ChildNodeListImpl::nodeMatches): Return true only if the node is our child.
+        (TagNodeListImpl::TagNodeListImpl): Irrelevant change to reformat initializers.
+        * khtml/xml/dom_nodeimpl.h:
+
 2004-11-12  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Gramps.
index be1fdfc..6784da9 100644 (file)
@@ -472,6 +472,13 @@ unsigned long NodeList::length() const
     return impl->length();
 }
 
+Node NodeList::itemById (const DOMString& elementId) const
+{
+    if (!impl) return 0;
+    return impl->itemById(elementId);
+}
+
+
 NodeListImpl *NodeList::handle() const
 {
     return impl;
index d59c6a6..f42b88c 100644 (file)
@@ -939,9 +939,12 @@ public:
      * @internal
      * not part of the DOM
      */
+    Node itemById (const DOMString& elementId) const;
+
     NodeListImpl *handle() const;
     bool isNull() const;
 
+
 protected:
     NodeList(const NodeListImpl *i);
     NodeListImpl *impl;
index 97cd05f..d9f0384 100644 (file)
@@ -650,26 +650,16 @@ Value DOMNodeList::tryGet(ExecState *exec, const Identifier &p) const
     // array index ?
     bool ok;
     long unsigned int idx = p.toULong(&ok);
-    if (ok)
+    if (ok) {
       result = getDOMNode(exec,list.item(idx));
-    else {
-      DOM::HTMLElement e;
-      unsigned long l = list.length();
-      bool found = false;
-      
-      for ( unsigned long i = 0; i < l; i++ ) {
-          DOM::Node node = list.item(i);
-          
-          if ( ( e = node ).id() == p.string() ) {
-              result = getDOMNode(exec, node);
-              found = true;
-              break;
-          }
-          
+    } else {
+      DOM::Node node = list.itemById(p.string());
+
+      if (!node.isNull()) {
+        result = getDOMNode(exec, node);
+      } else {
+        result = ObjectImp::get(exec, p);
       }
-      
-      if ( !found )
-          result = ObjectImp::get(exec, p);
     }
   }
 
index cfe7386..b6ffb4f 100644 (file)
@@ -2272,6 +2272,30 @@ NodeImpl *NodeListImpl::recursiveItem ( unsigned long &offset, NodeImpl *start )
     return 0; // no matching node in this subtree
 }
 
+NodeImpl *NodeListImpl::itemById (const DOMString& elementId) const
+{
+    if (rootNode->isDocumentNode()) {
+        DOM::NodeImpl *node = static_cast<DocumentImpl *>(rootNode)->getElementById(elementId);
+        if (nodeMatches(node))
+            return node;
+
+        return 0;
+    }
+
+    unsigned long l = length();
+
+    for ( unsigned long i = 0; i < l; i++ ) {
+        DOM::NodeImpl *node = item(i);
+        
+        if ( static_cast<ElementImpl *>(node)->getIDAttribute() == elementId ) {
+            return node;
+        }
+    }
+
+    return 0;
+}
+
+
 void NodeListImpl::rootNodeSubtreeModified()
 {
     isCacheValid = false;     
@@ -2307,13 +2331,15 @@ NodeImpl *ChildNodeListImpl::item ( unsigned long index ) const
     return n;
 }
 
-bool ChildNodeListImpl::nodeMatches( NodeImpl */*testNode*/ ) const
+bool ChildNodeListImpl::nodeMatches(NodeImpl *testNode) const
 {
-    return true;
+    return testNode->parentNode() == rootNode;
 }
 
 TagNodeListImpl::TagNodeListImpl(NodeImpl *n, NodeImpl::Id _id, NodeImpl::Id _idMask )
-    : NodeListImpl(n), m_id(_id & _idMask), m_idMask(_idMask)
+    : NodeListImpl(n), 
+      m_id(_id & _idMask), 
+      m_idMask(_idMask)
 {
 }
 
index 1491a22..18f6c00 100644 (file)
@@ -559,6 +559,7 @@ public:
     // DOM methods & attributes for NodeList
     virtual unsigned long length() const = 0;
     virtual NodeImpl *item ( unsigned long index ) const = 0;
+    virtual NodeImpl *itemById ( const DOMString & elementId ) const;
 
     // Other methods (not part of DOM)