Bug 870566. Make dataset gets faster by spending less time messing with strings. r=smaug

content/base/public/Element.h

@@ -503,7 +503,7 @@ private:
 
 protected:
   inline bool GetAttr(int32_t aNameSpaceID, nsIAtom* aName,
-                      mozilla::dom::DOMString& aResult) const
+                      DOMString& aResult) const
   {
     NS_ASSERTION(nullptr != aName, "must have attribute name");
     NS_ASSERTION(aNameSpaceID != kNameSpaceID_Unknown,
@@ -513,12 +513,26 @@ protected:
     const nsAttrValue* val = mAttrsAndChildren.GetAttr(aName, aNameSpaceID);
     if (val) {
       val->ToString(aResult);
+      return true;
     }
     // else DOMString comes pre-emptied.
-    return val != nullptr;
+    return false;
   }
 
 public:
+  inline bool GetAttr(const nsAString& aName, 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);
+      return true;
+    }
+    // else DOMString comes pre-emptied.
+    return false;
+  }
+
   void GetTagName(nsAString& aTagName) const
   {
     aTagName = NodeName();
@@ -527,7 +541,7 @@ public:
   {
     GetAttr(kNameSpaceID_None, nsGkAtoms::id, aId);
   }
-  void GetId(mozilla::dom::DOMString& aId) const
+  void GetId(DOMString& aId) const
   {
     GetAttr(kNameSpaceID_None, nsGkAtoms::id, aId);
   }
@@ -548,12 +562,12 @@ public:
   }
   void GetAttribute(const nsAString& aName, nsString& aReturn)
   {
-    mozilla::dom::DOMString str;
+    DOMString str;
     GetAttribute(aName, str);
     str.ToString(aReturn);
   }
 
-  void GetAttribute(const nsAString& aName, mozilla::dom::DOMString& aReturn);
+  void GetAttribute(const nsAString& aName, DOMString& aReturn);
   void GetAttributeNS(const nsAString& aNamespaceURI,
                       const nsAString& aLocalName,
                       nsAString& aReturn);
@@ -707,7 +721,7 @@ public:
            0;
   }
 
-  virtual already_AddRefed<mozilla::dom::UndoManager> GetUndoManager()
+  virtual already_AddRefed<UndoManager> GetUndoManager()
   {
     return nullptr;
   }
@@ -717,18 +731,16 @@ public:
     return false;
   }
 
-  virtual void SetUndoScope(bool aUndoScope, mozilla::ErrorResult& aError)
+  virtual void SetUndoScope(bool aUndoScope, ErrorResult& aError)
   {
   }
 
-  virtual void GetInnerHTML(nsAString& aInnerHTML,
-                            mozilla::ErrorResult& aError);
-  virtual void SetInnerHTML(const nsAString& aInnerHTML,
-                            mozilla::ErrorResult& aError);
-  void GetOuterHTML(nsAString& aOuterHTML, mozilla::ErrorResult& aError);
-  void SetOuterHTML(const nsAString& aOuterHTML, mozilla::ErrorResult& aError);
+  virtual void GetInnerHTML(nsAString& aInnerHTML, ErrorResult& aError);
+  virtual void SetInnerHTML(const nsAString& aInnerHTML, ErrorResult& aError);
+  void GetOuterHTML(nsAString& aOuterHTML, ErrorResult& aError);
+  void SetOuterHTML(const nsAString& aOuterHTML, ErrorResult& aError);
   void InsertAdjacentHTML(const nsAString& aPosition, const nsAString& aText,
-                          mozilla::ErrorResult& aError);
+                          ErrorResult& aError);
 
   //----------------------------------------
 
@@ -764,7 +776,7 @@ public:
                                      nsInputEvent* aSourceEvent,
                                      nsIContent* aTarget,
                                      bool aFullDispatch,
-                                     const mozilla::widget::EventFlags* aFlags,
+                                     const widget::EventFlags* aFlags,
                                      nsEventStatus* aStatus);
 
   /**

content/base/src/nsAttrAndChildArray.cpp

@@ -318,6 +318,23 @@ nsAttrAndChildArray::GetAttr(nsIAtom* aLocalName, int32_t aNamespaceID) const
   return nullptr;
 }
 
+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;
+}
+
 const nsAttrValue*
 nsAttrAndChildArray::GetAttr(const nsAString& aName,
                              nsCaseTreatment aCaseSensitive) const

content/base/src/nsAttrAndChildArray.h

@@ -71,7 +71,11 @@ public:
   int32_t IndexOfChild(const nsINode* aPossibleChild) const;
 
   uint32_t AttrCount() const;
-  const nsAttrValue* GetAttr(nsIAtom* aLocalName, int32_t aNamespaceID = kNameSpaceID_None) const;
+  const nsAttrValue* GetAttr(nsIAtom* aLocalName,
+                             int32_t aNamespaceID = kNameSpaceID_None) const;
+  // As above but using a string attr name and always using
+  // kNameSpaceID_None.  This is always case-sensitive.
+  const nsAttrValue* GetAttr(const nsAString& aName) const;
   // Get an nsAttrValue by qualified name.  Can optionally do
   // ASCII-case-insensitive name matching.
   const nsAttrValue* GetAttr(const nsAString& aName,

content/base/src/nsAttrName.h

@@ -105,6 +105,12 @@ public:
     return reinterpret_cast<uintptr_t>(aAtom) == mBits;
   }
 
+  // And the same but without forcing callers to atomize
+  bool Equals(const nsAString& aLocalName) const
+  {
+    return IsAtom() && Atom()->Equals(aLocalName);
+  }
+
   bool Equals(nsIAtom* aLocalName, int32_t aNamespaceID) const
   {
     if (aNamespaceID == kNameSpaceID_None) {

content/html/content/src/nsDOMStringMap.cpp

@@ -65,7 +65,7 @@ nsDOMStringMap::WrapObject(JSContext *cx, JS::Handle<JSObject*> scope)
 
 void
 nsDOMStringMap::NamedGetter(const nsAString& aProp, bool& found,
-                            nsString& aResult) const
+                            DOMString& aResult) const
 {
   nsAutoString attr;
 
@@ -74,10 +74,7 @@ nsDOMStringMap::NamedGetter(const nsAString& aProp, bool& found,
     return;
   }
 
-  nsCOMPtr<nsIAtom> attrAtom = do_GetAtom(attr);
-  MOZ_ASSERT(attrAtom, "Should be infallible");
-
-  found = mElement->GetAttr(kNameSpaceID_None, attrAtom, aResult);
+  found = mElement->GetAttr(attr, aResult);
 }
 
 void
@@ -139,16 +136,15 @@ nsDOMStringMap::GetSupportedNames(nsTArray<nsString>& aNames)
   // Iterate through all the attributes and add property
   // names corresponding to data attributes to return array.
   for (uint32_t i = 0; i < attrCount; ++i) {
-    nsAutoString attrString;
     const nsAttrName* attrName = mElement->GetAttrNameAt(i);
     // Skip the ones that are not in the null namespace
     if (attrName->NamespaceID() != kNameSpaceID_None) {
       continue;
     }
-    attrName->LocalName()->ToString(attrString);
 
     nsAutoString prop;
-    if (!AttrToDataProp(attrString, prop)) {
+    if (!AttrToDataProp(nsDependentAtomString(attrName->LocalName()),
+                        prop)) {
       continue;
     }
 
@@ -161,23 +157,21 @@ nsDOMStringMap::GetSupportedNames(nsTArray<nsString>& aNames)
  * (ex. aBigFish to data-a-big-fish).
  */
 bool nsDOMStringMap::DataPropToAttr(const nsAString& aProp,
-                                      nsAString& aResult)
+                                    nsAutoString& aResult)
 {
-  const PRUnichar* cur = aProp.BeginReading();
-  const PRUnichar* end = aProp.EndReading();
-
-  // String corresponding to the data attribute on the element.
-  nsAutoString attr;
-  // Length of attr will be at least the length of the property + 5 for "data-".
-  attr.SetCapacity(aProp.Length() + 5);
-
-  attr.Append(NS_LITERAL_STRING("data-"));
+  // aResult is an autostring, so don't worry about setting its capacity:
+  // SetCapacity is slow even when it's a no-op and we already have enough
+  // storage there for most cases, probably.
+  aResult.AppendLiteral("data-");
 
   // Iterate property by character to form attribute name.
   // Return syntax error if there is a sequence of "-" followed by a character
   // in the range "a" to "z".
   // Replace capital characters with "-" followed by lower case character.
   // Otherwise, simply append character to attribute name.
+  const PRUnichar* start = aProp.BeginReading();
+  const PRUnichar* end = aProp.EndReading();
+  const PRUnichar* cur = start;
   for (; cur < end; ++cur) {
     const PRUnichar* next = cur + 1;
     if (PRUnichar('-') == *cur && next < end &&
@@ -187,15 +181,17 @@ bool nsDOMStringMap::DataPropToAttr(const nsAString& aProp,
     }
 
     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 = next; // We've already appended the thing at *cur
     }
   }
 
-  aResult.Assign(attr);
+  aResult.Append(start, cur - start);
+
   return true;
 }
 
@@ -204,7 +200,7 @@ bool nsDOMStringMap::DataPropToAttr(const nsAString& aProp,
  * (ex. data-a-big-fish to aBigFish).
  */
 bool nsDOMStringMap::AttrToDataProp(const nsAString& aAttr,
-                                      nsAString& aResult)
+                                    nsAutoString& aResult)
 {
   // If the attribute name does not begin with "data-" then it can not be
   // a data attribute.
@@ -216,9 +212,8 @@ bool nsDOMStringMap::AttrToDataProp(const nsAString& aAttr,
   const PRUnichar* cur = aAttr.BeginReading() + 5;
   const PRUnichar* end = aAttr.EndReading();
 
-  // Dataset property name. Ensure that the string is large enough to store
-  // all the characters in the property name.
-  nsAutoString prop;
+  // Don't try to mess with aResult's capacity: the probably-no-op SetCapacity()
+  // call is not that fast.
 
   // Iterate through attrName by character to form property name.
   // If there is a sequence of "-" followed by a character in the range "a" to
@@ -229,15 +224,14 @@ bool nsDOMStringMap::AttrToDataProp(const nsAString& aAttr,
     if (PRUnichar('-') == *cur && next < end && 
         PRUnichar('a') <= *next && *next <= PRUnichar('z')) {
       // Upper case the lower case letters that follow a "-".
-      prop.Append(*next - 'a' + 'A');
+      aResult.Append(*next - 'a' + 'A');
       // Consume character to account for "-" character.
       ++cur;
     } else {
       // Simply append character if camel case is not necessary.
-      prop.Append(*cur);
+      aResult.Append(*cur);
     }
   }
 
-  aResult.Assign(prop);
   return true;
 }

content/html/content/src/nsDOMStringMap.h

@@ -35,7 +35,8 @@ public:
   // WebIDL API
   virtual JSObject* WrapObject(JSContext *cx,
                                JS::Handle<JSObject*> scope) MOZ_OVERRIDE;
-  void NamedGetter(const nsAString& aProp, bool& found, nsString& aResult) const;
+  void NamedGetter(const nsAString& aProp, bool& found,
+                   mozilla::dom::DOMString& aResult) const;
   void NamedSetter(const nsAString& aProp, const nsAString& aValue,
                    mozilla::ErrorResult& rv);
   void NamedDeleter(const nsAString& aProp, bool &found);
@@ -48,8 +49,8 @@ protected:
   nsRefPtr<nsGenericHTMLElement> mElement;
   // Flag to guard against infinite recursion.
   bool mRemovingProp;
-  static bool DataPropToAttr(const nsAString& aProp, nsAString& aResult);
-  static bool AttrToDataProp(const nsAString& aAttr, nsAString& aResult);
+  static bool DataPropToAttr(const nsAString& aProp, nsAutoString& aResult);
+  static bool AttrToDataProp(const nsAString& aAttr, nsAutoString& aResult);
 };
 
 #endif