1374030 - Optimize the loop in nsAttrAndChildArray::IndexOfAttr() a bit

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, enhancement)

Description

[(no longer active)]

While profiling Preact in Speedometer I noticed that this loop can get fairly hot: <https://searchfox.org/mozilla-central/rev/b95a1a95838cc6e85ea5d9b9e3ae39b4cd69d8ef/dom/base/nsAttrAndChildArray.cpp#572>.  This can get called from HasAttr() and UnsetAttr().  Also from GetAttrInfo() but that one is less common.

While profiling I noticed that the loop is pretty tight so the cost of checking AttrSlotIsTaken(i) in the loop condition can be pretty high.  It turns out that if we didn't check it the loop would still be correct since Equals() just does a pointer comparison on the atom, so if the atom pointer is null the equality check wouldn't hold, so now we are effectively paying a performance cost in the common case where the entry is present to save some loop iterations in the non-common case of finding a non-existing entry.  Removing this check drops the cost of the function down to the point of making it negligible in the profile I was looking at (0.07% after compared to 0.13% before.)

Comment 1

[(no longer active)]

Attachment: Optimize the loop in nsAttrAndChildArray::IndexOfAttr() a bit

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 8878861 [details] [diff] [review]
Optimize the loop in nsAttrAndChildArray::IndexOfAttr() a bit

Please add a comment to nsAttrName::Equals(nsIAtom*) that some callers (point to this one?) call it on nsAttrName structs with 0 mBits, so no attempt should be made to do anything with mBits here other than direct comparison to the incoming bits.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3794aebfdef8
Optimize the loop in nsAttrAndChildArray::IndexOfAttr() a bit; r=bzbarsky

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/3794aebfdef8