There's a bunch of string stuff in here that can be faster...

Comment on attachment 747634 [details] [diff] [review] Make dataset gets faster by spending less time messing with strings. Review of attachment 747634 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/base/public/Element.h @@ +519,5 @@ > } > > public: > + inline bool GetAttr(const nsAString& aName, > + mozilla::dom::DOMString& aResult) const No need for mozilla::dom

Comment on attachment 747634 [details] [diff] [review] Make dataset gets faster by spending less time messing with strings. > public: >+ inline bool GetAttr(const nsAString& aName, >+ mozilla::dom::DOMString& aResult) const >+ { >+ MOZ_ASSERT(aResult.HasStringBuffer() && aResult.StringBufferLength() == 0, >+ "Should have empty string coming in"); >+ const nsAttrValue* val = mAttrsAndChildren.GetAttr(aName); >+ if (val) { >+ val->ToString(aResult); >+ } >+ // else DOMString comes pre-emptied. >+ return val != nullptr; >+ } I'd return true inside if, and false outside. Double null-check looks a bit odd. > const nsAttrValue* >+nsAttrAndChildArray::GetAttr(const nsAString& aLocalName) const >+{ >+ uint32_t i, slotCount = AttrSlotCount(); >+ for (i = 0; i < slotCount && AttrSlotIsTaken(i); ++i) { >+ if (ATTRS(mImpl)[i].mName.Equals(aLocalName)) { >+ return &ATTRS(mImpl)[i].mValue; >+ } >+ } >+ >+ if (mImpl && mImpl->mMappedAttrs) { >+ return mImpl->mMappedAttrs->GetAttr(aLocalName); >+ } >+ >+ return nullptr; >+} Could you make the other GetAttr, the one which takes also namespaceid to call this method when aNamespaceID == kNameSpaceID_None > if (PRUnichar('A') <= *cur && *cur <= PRUnichar('Z')) { >+ // Append the characters in the range [start, cur) >+ aResult.Append(start, cur - start); > // Uncamel-case characters in the range of "A" to "Z". >- attr.Append(PRUnichar('-')); >- attr.Append(*cur - 'A' + 'a'); >- } else { >- attr.Append(*cur); >+ aResult.Append(PRUnichar('-')); >+ aResult.Append(*cur - 'A' + 'a'); >+ start = cur+1; // We've already appended the thing at *cur start = next

> No need for mozilla::dom Good catch. Done, and for the place I copied from and a few others in this file. > I'd return true inside if, and false outside. Double null-check looks a bit odd. OK, done. Again, fixed the other GetAttr as well. > Could you make the other GetAttr, the one which takes also namespaceid to call this > method when aNamespaceID == kNameSpaceID_None There is no GetAttr that takes a string and a namespace id. We have a GetAttr taking string and case-handling (which matches on qualified name, not local name), and we have a GetAttr that takes an _atom_ and a namespace id... > start = next Done.