[webkit-dev] Optimizing DOMNode _nodeWith
Maciej Stachowiak
mjs at apple.com
Fri Apr 28 13:16:27 PDT 2006
On Apr 28, 2006, at 12:41 PM, Matt Gough wrote:
> 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.
Generally, we consider a repeatable 1-3% performance improvement to
be worth it. And I think your change is a cleaner design anyway. I
encourage you to put it up on bugzilla.
Regards,
Maciej
>
>
> 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