[webkit-dev] Optimizing DOMNode _nodeWith

Matt Gough devlists at softchaos.com
Fri Apr 28 12:41:48 PDT 2006


On 24 Apr 2006, at 20:58, Maciej Stachowiak wrote:

> I think a better approach would be to make a hash table of function  
> pointers, that will be faster than the log chain of if's. This is  
> what is used to generate the elements in the first place.

On 24 Apr 2006, at 19:24, Timothy Hatcher wrote:
>  I would like to see before and after numbers when you have  
> everything changed over.

OK, so I implemented this as a hashmap. Unfortunately my results show  
only marginal improvement. (i.e only a 1 - 3% reduction in time for  
the various sites I have tested. Investigating further it seems that  
my original assertion that 'For a medium sized DOM (e.g  
news.google.com - just over 4000 nodes) this code takes over one  
tenth of a second on an 867MHz G4. A third of that time is down to  
the calls to hasLocalName' was wrong. I suspect those timings were  
being done with a debug build of WebKit. On the largest page I could  
find (by node count) my changes made about 5ms improvement (http:// 
whatwg.org/specs/web-apps/current-work/).

Anyway, I have put my work on this at the end of the email, but I am  
not convinced it is worth changing to this without there being a  
noticeable benefit. I guess the multiple calls to hasLocalName don't  
have as much impact as we might think. Maybe if someone could test  
this on different hardware it might make a bigger difference.


Matt



Index: WebCore/bindings/objc/DOM.mm
===================================================================
--- WebCore/bindings/objc/DOM.mm        (revision 14055)
+++ WebCore/bindings/objc/DOM.mm        (working copy)
@@ -44,10 +44,13 @@
#import "Range.h"
#import "dom_xmlimpl.h"
#import "HTMLNames.h"
+#import "QualifiedName.h"
#import "RenderImage.h"
#import <JavaScriptCore/WebScriptObjectPrivate.h>
#import <objc/objc-class.h>
+using WebCore::AtomicString;
+using WebCore::AtomicStringImpl;
using WebCore::Attr;
using WebCore::CharacterData;
using WebCore::Document;
@@ -69,6 +72,7 @@ using WebCore::NodeIterator;
using WebCore::NodeList;
using WebCore::Notation;
using WebCore::ProcessingInstruction;
+using WebCore::QualifiedName;
using WebCore::Range;
using WebCore::RenderImage;
using WebCore::RenderObject;
@@ -454,6 +458,218 @@ static ListenerMap *listenerMap;
@end
+typedef HashMap<AtomicStringImpl*, Class> ObjcClassMap;
+
+static ObjcClassMap *objcClassMap;
+
+static void addObjcClassForTag(const QualifiedName& tag, Class  
objcClass)
+{
+    objcClassMap->set(tag.localName().impl(), objcClass);
+}
+
+
+static void createObjcClassMap()
+{
+    // Create the table.
+    objcClassMap = new ObjcClassMap;
+
+    // Populate it with DOM Element classes.
+    addObjcClassForTag(htmlTag, [DOMHTMLHtmlElement class]);
+    addObjcClassForTag(headTag, [DOMHTMLHeadElement class]);
+    addObjcClassForTag(linkTag, [DOMHTMLLinkElement class]);
+    addObjcClassForTag(titleTag, [DOMHTMLTitleElement class]);
+    addObjcClassForTag(metaTag, [DOMHTMLMetaElement class]);
+    addObjcClassForTag(baseTag, [DOMHTMLBaseElement class]);
+    addObjcClassForTag(isindexTag, [DOMHTMLIsIndexElement class]);
+    addObjcClassForTag(styleTag, [DOMHTMLStyleElement class]);
+    addObjcClassForTag(bodyTag, [DOMHTMLBodyElement class]);
+    addObjcClassForTag(formTag, [DOMHTMLFormElement class]);
+    addObjcClassForTag(selectTag, [DOMHTMLSelectElement class]);
+    addObjcClassForTag(optgroupTag, [DOMHTMLOptGroupElement class]);
+    addObjcClassForTag(optionTag, [DOMHTMLOptionElement class]);
+    addObjcClassForTag(inputTag, [DOMHTMLInputElement class]);
+    addObjcClassForTag(textareaTag, [DOMHTMLTextAreaElement class]);
+    addObjcClassForTag(buttonTag, [DOMHTMLButtonElement class]);
+    addObjcClassForTag(labelTag, [DOMHTMLLabelElement class]);
+    addObjcClassForTag(fieldsetTag, [DOMHTMLFieldSetElement class]);
+    addObjcClassForTag(legendTag, [DOMHTMLLegendElement class]);
+    addObjcClassForTag(ulTag, [DOMHTMLUListElement class]);
+    addObjcClassForTag(olTag, [DOMHTMLOListElement class]);
+    addObjcClassForTag(dlTag, [DOMHTMLDListElement class]);
+    addObjcClassForTag(dirTag, [DOMHTMLDirectoryElement class]);
+    addObjcClassForTag(menuTag, [DOMHTMLMenuElement class]);
+    addObjcClassForTag(liTag, [DOMHTMLLIElement class]);
+    addObjcClassForTag(divTag, [DOMHTMLDivElement class]);
+    addObjcClassForTag(pTag, [DOMHTMLParagraphElement class]);
+    addObjcClassForTag(h1Tag, [DOMHTMLHeadingElement class]);
+    addObjcClassForTag(h2Tag, [DOMHTMLHeadingElement class]);
+    addObjcClassForTag(h3Tag, [DOMHTMLHeadingElement class]);
+    addObjcClassForTag(h4Tag, [DOMHTMLHeadingElement class]);
+    addObjcClassForTag(h5Tag, [DOMHTMLHeadingElement class]);
+    addObjcClassForTag(h6Tag, [DOMHTMLHeadingElement class]);
+    addObjcClassForTag(qTag, [DOMHTMLQuoteElement class]);
+    addObjcClassForTag(preTag, [DOMHTMLPreElement class]);
+    addObjcClassForTag(listingTag, [DOMHTMLPreElement class]);
+    addObjcClassForTag(brTag, [DOMHTMLBRElement class]);
+    addObjcClassForTag(basefontTag, [DOMHTMLBaseFontElement class]);
+    addObjcClassForTag(fontTag, [DOMHTMLFontElement class]);
+    addObjcClassForTag(hrTag, [DOMHTMLHRElement class]);
+    addObjcClassForTag(aTag, [DOMHTMLAnchorElement class]);
+    addObjcClassForTag(imgTag, [DOMHTMLImageElement class]);
+    addObjcClassForTag(canvasTag, [DOMHTMLImageElement class]);
+    addObjcClassForTag(objectTag, [DOMHTMLObjectElement class]);
+    addObjcClassForTag(paramTag, [DOMHTMLParamElement class]);
+    addObjcClassForTag(appletTag, [DOMHTMLAppletElement class]);
+    addObjcClassForTag(mapTag, [DOMHTMLMapElement class]);
+    addObjcClassForTag(areaTag, [DOMHTMLAreaElement class]);
+    addObjcClassForTag(scriptTag, [DOMHTMLScriptElement class]);
+    addObjcClassForTag(tableTag, [DOMHTMLTableElement class]);
+    addObjcClassForTag(theadTag, [DOMHTMLTableSectionElement class]);
+    addObjcClassForTag(tbodyTag, [DOMHTMLTableSectionElement class]);
+    addObjcClassForTag(tfootTag, [DOMHTMLTableSectionElement class]);
+    addObjcClassForTag(tdTag, [DOMHTMLTableCellElement class]);
+    addObjcClassForTag(trTag, [DOMHTMLTableRowElement class]);
+    addObjcClassForTag(colTag, [DOMHTMLTableColElement class]);
+    addObjcClassForTag(colgroupTag, [DOMHTMLTableColElement class]);
+    addObjcClassForTag(captionTag, [DOMHTMLTableCaptionElement class]);
+    addObjcClassForTag(framesetTag, [DOMHTMLFrameSetElement class]);
+    addObjcClassForTag(frameTag, [DOMHTMLFrameElement class]);
+    addObjcClassForTag(iframeTag, [DOMHTMLIFrameElement class]);
+}
+
+Class objcClassForTag(const AtomicString& tagName)
+{
+    if (!objcClassMap)
+        createObjcClassMap();
+
+    Class objcClass = objcClassMap->get(tagName.impl());
+    if (!objcClass)
+        objcClass = [DOMHTMLElement class];
+    return objcClass;
+}
+
+Class objcClassForHTMLElement(HTMLElement* htmlElt)
+{
+    // The old way for testing purposes
+    Class wrapperClass;
+    if (htmlElt->hasLocalName(htmlTag))
+        wrapperClass = [DOMHTMLHtmlElement class];
+    else if (htmlElt->hasLocalName(headTag))
+        wrapperClass = [DOMHTMLHeadElement class];
+    else if (htmlElt->hasLocalName(linkTag))
+        wrapperClass = [DOMHTMLLinkElement class];
+    else if (htmlElt->hasLocalName(titleTag))
+        wrapperClass = [DOMHTMLTitleElement class];
+    else if (htmlElt->hasLocalName(metaTag))
+        wrapperClass = [DOMHTMLMetaElement class];
+    else if (htmlElt->hasLocalName(baseTag))
+        wrapperClass = [DOMHTMLBaseElement class];
+    else if (htmlElt->hasLocalName(isindexTag))
+        wrapperClass = [DOMHTMLIsIndexElement class];
+    else if (htmlElt->hasLocalName(styleTag))
+        wrapperClass = [DOMHTMLStyleElement class];
+    else if (htmlElt->hasLocalName(bodyTag))
+        wrapperClass = [DOMHTMLBodyElement class];
+    else if (htmlElt->hasLocalName(formTag))
+        wrapperClass = [DOMHTMLFormElement class];
+    else if (htmlElt->hasLocalName(selectTag))
+        wrapperClass = [DOMHTMLSelectElement class];
+    else if (htmlElt->hasLocalName(optgroupTag))
+        wrapperClass = [DOMHTMLOptGroupElement class];
+    else if (htmlElt->hasLocalName(optionTag))
+        wrapperClass = [DOMHTMLOptionElement class];
+    else if (htmlElt->hasLocalName(inputTag))
+        wrapperClass = [DOMHTMLInputElement class];
+    else if (htmlElt->hasLocalName(textareaTag))
+        wrapperClass = [DOMHTMLTextAreaElement class];
+    else if (htmlElt->hasLocalName(buttonTag))
+        wrapperClass = [DOMHTMLButtonElement class];
+    else if (htmlElt->hasLocalName(labelTag))
+        wrapperClass = [DOMHTMLLabelElement class];
+    else if (htmlElt->hasLocalName(fieldsetTag))
+        wrapperClass = [DOMHTMLFieldSetElement class];
+    else if (htmlElt->hasLocalName(legendTag))
+        wrapperClass = [DOMHTMLLegendElement class];
+    else if (htmlElt->hasLocalName(ulTag))
+        wrapperClass = [DOMHTMLUListElement class];
+    else if (htmlElt->hasLocalName(olTag))
+        wrapperClass = [DOMHTMLOListElement class];
+    else if (htmlElt->hasLocalName(dlTag))
+        wrapperClass = [DOMHTMLDListElement class];
+    else if (htmlElt->hasLocalName(dirTag))
+        wrapperClass = [DOMHTMLDirectoryElement class];
+    else if (htmlElt->hasLocalName(menuTag))
+        wrapperClass = [DOMHTMLMenuElement class];
+    else if (htmlElt->hasLocalName(liTag))
+        wrapperClass = [DOMHTMLLIElement class];
+    else if (htmlElt->hasLocalName(divTag))
+        wrapperClass = [DOMHTMLDivElement class];
+    else if (htmlElt->hasLocalName(pTag))
+        wrapperClass = [DOMHTMLParagraphElement class];
+    else if (htmlElt->hasLocalName(h1Tag) ||
+             htmlElt->hasLocalName(h2Tag) ||
+             htmlElt->hasLocalName(h3Tag) ||
+             htmlElt->hasLocalName(h4Tag) ||
+             htmlElt->hasLocalName(h5Tag) ||
+             htmlElt->hasLocalName(h6Tag))
+        wrapperClass = [DOMHTMLHeadingElement class];
+    else if (htmlElt->hasLocalName(qTag))
+        wrapperClass = [DOMHTMLQuoteElement class];
+    else if (htmlElt->hasLocalName(preTag) || htmlElt->hasLocalName 
(listingTag))
+        wrapperClass = [DOMHTMLPreElement class];
+    else if (htmlElt->hasLocalName(brTag))
+        wrapperClass = [DOMHTMLBRElement class];
+    else if (htmlElt->hasLocalName(basefontTag))
+        wrapperClass = [DOMHTMLBaseFontElement class];
+    else if (htmlElt->hasLocalName(fontTag))
+        wrapperClass = [DOMHTMLFontElement class];
+    else if (htmlElt->hasLocalName(hrTag))
+        wrapperClass = [DOMHTMLHRElement class];
+    else if (htmlElt->hasLocalName(aTag))
+        wrapperClass = [DOMHTMLAnchorElement class];
+    else if (htmlElt->hasLocalName(imgTag) ||
+             htmlElt->hasLocalName(canvasTag))
+        wrapperClass = [DOMHTMLImageElement class];
+    else if (htmlElt->hasLocalName(objectTag))
+        wrapperClass = [DOMHTMLObjectElement class];
+    else if (htmlElt->hasLocalName(paramTag))
+        wrapperClass = [DOMHTMLParamElement class];
+    else if (htmlElt->hasLocalName(appletTag))
+        wrapperClass = [DOMHTMLAppletElement class];
+    else if (htmlElt->hasLocalName(mapTag))
+        wrapperClass = [DOMHTMLMapElement class];
+    else if (htmlElt->hasLocalName(areaTag))
+        wrapperClass = [DOMHTMLAreaElement class];
+    else if (htmlElt->hasLocalName(scriptTag))
+        wrapperClass = [DOMHTMLScriptElement class];
+    else if (htmlElt->hasLocalName(tableTag))
+        wrapperClass = [DOMHTMLTableElement class];
+    else if (htmlElt->hasLocalName(theadTag) ||
+             htmlElt->hasLocalName(tbodyTag) ||
+             htmlElt->hasLocalName(tfootTag))
+        wrapperClass = [DOMHTMLTableSectionElement class];
+    else if (htmlElt->hasLocalName(tdTag))
+        wrapperClass = [DOMHTMLTableCellElement class];
+    else if (htmlElt->hasLocalName(trTag))
+        wrapperClass = [DOMHTMLTableRowElement class];
+    else if (htmlElt->hasLocalName(colTag) ||
+             htmlElt->hasLocalName(colgroupTag))
+        wrapperClass = [DOMHTMLTableColElement class];
+    else if (htmlElt->hasLocalName(captionTag))
+        wrapperClass = [DOMHTMLTableCaptionElement class];
+    else if (htmlElt->hasLocalName(framesetTag))
+        wrapperClass = [DOMHTMLFrameSetElement class];
+    else if (htmlElt->hasLocalName(frameTag))
+        wrapperClass = [DOMHTMLFrameElement class];
+    else if (htmlElt->hasLocalName(iframeTag))
+        wrapperClass = [DOMHTMLIFrameElement class];
+    else
+        wrapperClass = [DOMHTMLElement class];
+
+    return wrapperClass;
+}
+
+
@implementation DOMNode (WebCoreInternal)
- (id)_initWithNode:(Node *)impl
@@ -482,122 +698,12 @@ static ListenerMap *listenerMap;
          case Node::ELEMENT_NODE:
              if (impl->isHTMLElement()) {
                  // FIXME: Reflect marquee once the API has been  
determined.
-                // FIXME: We could make the HTML classes hand back  
their class names and then use that to make
-                // the appropriate Obj-C class from the string.
                  HTMLElement* htmlElt = static_cast<HTMLElement*> 
(impl);
-                if (htmlElt->hasLocalName(htmlTag))
-                    wrapperClass = [DOMHTMLHtmlElement class];
-                else if (htmlElt->hasLocalName(headTag))
-                    wrapperClass = [DOMHTMLHeadElement class];
-                else if (htmlElt->hasLocalName(linkTag))
-                    wrapperClass = [DOMHTMLLinkElement class];
-                else if (htmlElt->hasLocalName(titleTag))
-                    wrapperClass = [DOMHTMLTitleElement class];
-                else if (htmlElt->hasLocalName(metaTag))
-                    wrapperClass = [DOMHTMLMetaElement class];
-                else if (htmlElt->hasLocalName(baseTag))
-                    wrapperClass = [DOMHTMLBaseElement class];
-                else if (htmlElt->hasLocalName(isindexTag))
-                    wrapperClass = [DOMHTMLIsIndexElement class];
-                else if (htmlElt->hasLocalName(styleTag))
-                    wrapperClass = [DOMHTMLStyleElement class];
-                else if (htmlElt->hasLocalName(bodyTag))
-                    wrapperClass = [DOMHTMLBodyElement class];
-                else if (htmlElt->hasLocalName(formTag))
-                    wrapperClass = [DOMHTMLFormElement class];
-                else if (htmlElt->hasLocalName(selectTag))
-                    wrapperClass = [DOMHTMLSelectElement class];
-                else if (htmlElt->hasLocalName(optgroupTag))
-                    wrapperClass = [DOMHTMLOptGroupElement class];
-                else if (htmlElt->hasLocalName(optionTag))
-                    wrapperClass = [DOMHTMLOptionElement class];
-                else if (htmlElt->hasLocalName(inputTag))
-                    wrapperClass = [DOMHTMLInputElement class];
-                else if (htmlElt->hasLocalName(textareaTag))
-                    wrapperClass = [DOMHTMLTextAreaElement class];
-                else if (htmlElt->hasLocalName(buttonTag))
-                    wrapperClass = [DOMHTMLButtonElement class];
-                else if (htmlElt->hasLocalName(labelTag))
-                    wrapperClass = [DOMHTMLLabelElement class];
-                else if (htmlElt->hasLocalName(fieldsetTag))
-                    wrapperClass = [DOMHTMLFieldSetElement class];
-                else if (htmlElt->hasLocalName(legendTag))
-                    wrapperClass = [DOMHTMLLegendElement class];
-                else if (htmlElt->hasLocalName(ulTag))
-                    wrapperClass = [DOMHTMLUListElement class];
-                else if (htmlElt->hasLocalName(olTag))
-                    wrapperClass = [DOMHTMLOListElement class];
-                else if (htmlElt->hasLocalName(dlTag))
-                    wrapperClass = [DOMHTMLDListElement class];
-                else if (htmlElt->hasLocalName(dirTag))
-                    wrapperClass = [DOMHTMLDirectoryElement class];
-                else if (htmlElt->hasLocalName(menuTag))
-                    wrapperClass = [DOMHTMLMenuElement class];
-                else if (htmlElt->hasLocalName(liTag))
-                    wrapperClass = [DOMHTMLLIElement class];
-                else if (htmlElt->hasLocalName(divTag))
-                    wrapperClass = [DOMHTMLDivElement class];
-                else if (htmlElt->hasLocalName(pTag))
-                    wrapperClass = [DOMHTMLParagraphElement class];
-                else if (htmlElt->hasLocalName(h1Tag) ||
-                         htmlElt->hasLocalName(h2Tag) ||
-                         htmlElt->hasLocalName(h3Tag) ||
-                         htmlElt->hasLocalName(h4Tag) ||
-                         htmlElt->hasLocalName(h5Tag) ||
-                         htmlElt->hasLocalName(h6Tag))
-                    wrapperClass = [DOMHTMLHeadingElement class];
-                else if (htmlElt->hasLocalName(qTag))
-                    wrapperClass = [DOMHTMLQuoteElement class];
-                else if (htmlElt->hasLocalName(preTag) || htmlElt- 
 >hasLocalName(listingTag))
-                    wrapperClass = [DOMHTMLPreElement class];
-                else if (htmlElt->hasLocalName(brTag))
-                    wrapperClass = [DOMHTMLBRElement class];
-                else if (htmlElt->hasLocalName(basefontTag))
-                    wrapperClass = [DOMHTMLBaseFontElement class];
-                else if (htmlElt->hasLocalName(fontTag))
-                    wrapperClass = [DOMHTMLFontElement class];
-                else if (htmlElt->hasLocalName(hrTag))
-                    wrapperClass = [DOMHTMLHRElement class];
-                else if (htmlElt->hasLocalName(aTag))
-                    wrapperClass = [DOMHTMLAnchorElement class];
-                else if (htmlElt->hasLocalName(imgTag) ||
-                         htmlElt->hasLocalName(canvasTag))
-                    wrapperClass = [DOMHTMLImageElement class];
-                else if (htmlElt->hasLocalName(objectTag))
-                    wrapperClass = [DOMHTMLObjectElement class];
-                else if (htmlElt->hasLocalName(paramTag))
-                    wrapperClass = [DOMHTMLParamElement class];
-                else if (htmlElt->hasLocalName(appletTag))
-                    wrapperClass = [DOMHTMLAppletElement class];
-                else if (htmlElt->hasLocalName(mapTag))
-                    wrapperClass = [DOMHTMLMapElement class];
-                else if (htmlElt->hasLocalName(areaTag))
-                    wrapperClass = [DOMHTMLAreaElement class];
-                else if (htmlElt->hasLocalName(scriptTag))
-                    wrapperClass = [DOMHTMLScriptElement class];
-                else if (htmlElt->hasLocalName(tableTag))
-                    wrapperClass = [DOMHTMLTableElement class];
-                else if (htmlElt->hasLocalName(theadTag) ||
-                         htmlElt->hasLocalName(tbodyTag) ||
-                         htmlElt->hasLocalName(tfootTag))
-                    wrapperClass = [DOMHTMLTableSectionElement class];
-                else if (htmlElt->hasLocalName(tdTag))
-                    wrapperClass = [DOMHTMLTableCellElement class];
-                else if (htmlElt->hasLocalName(trTag))
-                    wrapperClass = [DOMHTMLTableRowElement class];
-                else if (htmlElt->hasLocalName(colTag) ||
-                         htmlElt->hasLocalName(colgroupTag))
-                    wrapperClass = [DOMHTMLTableColElement class];
-                else if (htmlElt->hasLocalName(captionTag))
-                    wrapperClass = [DOMHTMLTableCaptionElement class];
-                else if (htmlElt->hasLocalName(framesetTag))
-                    wrapperClass = [DOMHTMLFrameSetElement class];
-                else if (htmlElt->hasLocalName(frameTag))
-                    wrapperClass = [DOMHTMLFrameElement class];
-                else if (htmlElt->hasLocalName(iframeTag))
-                    wrapperClass = [DOMHTMLIFrameElement class];
-                else
-                    wrapperClass = [DOMHTMLElement class];
+
+                // Do it both ways to simplify comparing the two  
ways in Shark
+                // Obviously only the first one should used if we go  
with this patch
+                wrapperClass = objcClassForTag(htmlElt->localName());
+                wrapperClass = objcClassForHTMLElement(htmlElt);
              } else {
                  wrapperClass = [DOMElement class];
              }




More information about the webkit-dev mailing list