Closed Bug 613595 Opened 12 years ago Closed 11 years ago

Speed up getElementsByTagName cache hits


(Core :: DOM: Core & HTML, defect, P1)






(Reporter: bzbarsky, Assigned: bzbarsky)


(Blocks 1 open bug)



(1 file, 1 obsolete file)

Right now, for getElementsByTagName, we end up atomizing, lowercasing, and atomizing again all before we do our cache check.  That's a total of one lowercasing and three hashtable lookups, in the best case.

Patch coming up that just makes us use the input string to do the hashtable lookup in the cache, and only atomize after that on a miss.  That makes the best-case scenario a single hashtable lookup, period.

This speeds up the dom-query dromaeo tests overall number by 15% or so; the win is all on the getElementsByTagName tests, of course.
Attachment #491936 - Flags: review?(peterv)
Whiteboard: [need review]
Comment on attachment 491936 [details] [diff] [review]
Speed up the cache-hit case for getElementsByTagName.

>diff --git a/content/base/src/nsContentList.h b/content/base/src/nsContentList.h

>+  PRBool MatchesKey(const nsContentListKey& aKey) const {

Nit: I think local style puts { on its own line here.

>+// If aMatchNameSpaceId is kNameSpaceID_Unknown, this will return a
>+// content list which matches ASCIIToLower(aTagname) against HTML
>+// elements in HTML documents and aString against everything else.
>+// For any other value of aMatchNameSpaceId, the list will match
>+// aString against all elements.


> already_AddRefed<nsContentList>
> NS_GetContentList(nsINode* aRootNode,
>                   PRInt32 aMatchNameSpaceId,
>-                  nsIAtom* aHTMLMatchAtom,
>-                  nsIAtom* aXMLMatchAtom = nsnull);
>+                  const nsAString& aTagname);
Attachment #491936 - Flags: review?(peterv) → review+
Attachment #491936 - Attachment is obsolete: true
Comment on attachment 492714 [details] [diff] [review]
Updated to comments

Requesting approval.  This is a very safe perf win.
Attachment #492714 - Flags: approval2.0?
Whiteboard: [need review] → [need approval]
Whiteboard: [need approval] → [need gk2 ship]
Attachment #492714 - Flags: approval2.0? → approval2.0-
Whiteboard: [need gk2 ship] → [need gk2 ship][fixed-in-cedar]
Whiteboard: [need gk2 ship][fixed-in-cedar] → [fixed-in-cedar]
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-cedar]
Target Milestone: --- → mozilla2.2
Flags: in-testsuite-
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.