Commit 3f20dc0f authored by Emilio Cobos Álvarez's avatar Emilio Cobos Álvarez
Browse files

Bug 1830986 - Avoid a bunch of allocations in innerHTML / outerHTML getter. r=smaug 

One thing that stands out of the profiles in bug 1827132 are a ton of
allocations and attribute lookups.

While the root cause of that performance issue is likely to be a
DOMSubtreeModified event that Chrome doesn't fire or so, those are
rather easy to optimize and might be more generally useful.

I removed handling of `<br type="">`, because editor no longer uses it.
Maybe we can also remove some of the other of the prefixed attribute
shenanigans.

Differential Revision: https://phabricator.services.mozilla.com/D176956
parent cffddeed

dom/base/nsAttrValue.h

+3 −3
Original line number Diff line number Diff line
@@ -484,6 +484,9 @@ class nsAttrValue { 


  size_t SizeOfExcludingThis(mozilla::MallocSizeOf aMallocSizeOf) const;



  nsAtom* GetStoredAtom() const;

  nsStringBuffer* GetStoredStringBuffer() const;



 private:

  // These have to be the same as in ValueType

  enum ValueBaseType {
@@ -496,9 +499,6 @@ class nsAttrValue { 
  inline ValueBaseType BaseType() const;

  inline bool IsSVGType(ValueType aType) const;



  nsAtom* GetStoredAtom() const;

  nsStringBuffer* GetStoredStringBuffer() const;



  /**

   * Get the index of an EnumTable in the sEnumTableArray.

   * If the EnumTable is not in the sEnumTableArray, it is added.

dom/base/nsContentUtils.cpp

+61 −64
Original line number Diff line number Diff line
@@ -8802,7 +8802,7 @@ class StringBuilder { 
    }

    ~Unit() {

      if (mType == eString || mType == eStringWithEncode) {

        delete mString;

        mString.~nsString();

      }

      MOZ_COUNT_DTOR(StringBuilder::Unit);

    }
@@ -8820,7 +8820,7 @@ class StringBuilder { 
    union {

      nsAtom* mAtom;

      const char16_t* mLiteral;

      nsAutoString* mString;

      nsString mString;

      const nsTextFragment* mTextFragment;

    };

    Type mType;
@@ -8851,27 +8851,18 @@ class StringBuilder { 
    mLength += len;

  }



  void Append(const nsAString& aString) {

  void Append(nsString&& aString) {

    Unit* u = AddUnit();

    u->mString = new nsAutoString(aString);

    u->mType = Unit::eString;

    uint32_t len = aString.Length();

    u->mLength = len;

    mLength += len;

  }



  void Append(nsAutoString* aString) {

    Unit* u = AddUnit();

    u->mString = aString;

    new (&u->mString) nsString(std::move(aString));

    u->mType = Unit::eString;

    uint32_t len = aString->Length();

    u->mLength = len;

    mLength += len;

  }



  void AppendWithAttrEncode(nsAutoString* aString, uint32_t aLen) {

  void AppendWithAttrEncode(nsString&& aString, uint32_t aLen) {

    Unit* u = AddUnit();

    u->mString = aString;

    new (&u->mString) nsString(std::move(aString));

    u->mType = Unit::eStringWithEncode;

    u->mLength = aLen;

    mLength += aLen;
@@ -8915,10 +8906,10 @@ class StringBuilder { 
            appender.Append(*(u.mAtom));

            break;

          case Unit::eString:

            appender.Append(*(u.mString));

            appender.Append(u.mString);

            break;

          case Unit::eStringWithEncode:

            EncodeAttrString(*(u.mString), appender);

            EncodeAttrString(u.mString, appender);

            break;

          case Unit::eLiteral:

            appender.Append(Span(u.mLiteral, u.mLength));
@@ -9096,10 +9087,9 @@ static void AppendEncodedCharacters(const nsTextFragment* aText, 
  }

}



static void AppendEncodedAttributeValue(nsAutoString* aValue,

                                        StringBuilder& aBuilder) {

  const char16_t* c = aValue->BeginReading();

  const char16_t* end = aValue->EndReading();

static uint32_t ExtraSpaceNeededForAttrEncoding(const nsAString& aValue) {

  const char16_t* c = aValue.BeginReading();

  const char16_t* end = aValue.EndReading();



  uint32_t extraSpaceNeeded = 0;

  while (c < end) {
@@ -9119,60 +9109,70 @@ static void AppendEncodedAttributeValue(nsAutoString* aValue, 
    ++c;

  }



  if (extraSpaceNeeded) {

    aBuilder.AppendWithAttrEncode(aValue, aValue->Length() + extraSpaceNeeded);

  return extraSpaceNeeded;

}



static void AppendEncodedAttributeValue(const nsAttrValue& aValue,

                                        StringBuilder& aBuilder) {

  if (nsAtom* atom = aValue.GetStoredAtom()) {

    nsDependentAtomString atomStr(atom);

    uint32_t space = ExtraSpaceNeededForAttrEncoding(atomStr);

    if (!space) {

      aBuilder.Append(atom);

    } else {

    aBuilder.Append(aValue);

      aBuilder.AppendWithAttrEncode(nsString(atomStr),

                                    atomStr.Length() + space);

    }

    return;

  }

  // NOTE(emilio): In most cases this will just be a reference to the stored

  // nsStringBuffer.

  nsString str;

  aValue.ToString(str);

  uint32_t space = ExtraSpaceNeededForAttrEncoding(str);

  if (space) {

    aBuilder.AppendWithAttrEncode(std::move(str), str.Length() + space);

  } else {

    aBuilder.Append(std::move(str));

  }

}



static void StartElement(Element* aContent, StringBuilder& aBuilder) {

  nsAtom* localName = aContent->NodeInfo()->NameAtom();

  int32_t tagNS = aContent->GetNameSpaceID();

static void StartElement(Element* aElement, StringBuilder& aBuilder) {

  nsAtom* localName = aElement->NodeInfo()->NameAtom();

  const int32_t tagNS = aElement->GetNameSpaceID();



  aBuilder.Append(u"<");

  if (aContent->IsHTMLElement() || aContent->IsSVGElement() ||

      aContent->IsMathMLElement()) {

  if (tagNS == kNameSpaceID_XHTML || tagNS == kNameSpaceID_SVG ||

      tagNS == kNameSpaceID_MathML) {

    aBuilder.Append(localName);

  } else {

    aBuilder.Append(aContent->NodeName());

    aBuilder.Append(nsString(aElement->NodeName()));

  }



  CustomElementData* ceData = aContent->GetCustomElementData();

  if (ceData) {

    nsAtom* isAttr = ceData->GetIs(aContent);

    if (isAttr && !aContent->HasAttr(kNameSpaceID_None, nsGkAtoms::is)) {

  if (CustomElementData* ceData = aElement->GetCustomElementData()) {

    nsAtom* isAttr = ceData->GetIs(aElement);

    if (isAttr && !aElement->HasAttr(nsGkAtoms::is)) {

      aBuilder.Append(uR"( is=")");

      aBuilder.Append(nsDependentAtomString(isAttr));

      aBuilder.Append(isAttr);

      aBuilder.Append(uR"(")");

    }

  }



  int32_t count = aContent->GetAttrCount();

  for (int32_t i = 0; i < count; i++) {

    const nsAttrName* name = aContent->GetAttrNameAt(i);

  uint32_t i = 0;

  while (BorrowedAttrInfo info = aElement->GetAttrInfoAt(i++)) {

    const nsAttrName* name = info.mName;



    int32_t attNs = name->NamespaceID();

    nsAtom* attName = name->LocalName();



    // Filter out any attribute starting with [-|_]moz

    // FIXME(emilio): Do we still need this?

    nsDependentAtomString attrNameStr(attName);

    if (StringBeginsWith(attrNameStr, u"_moz"_ns) ||

        StringBeginsWith(attrNameStr, u"-moz"_ns)) {

      continue;

    }



    auto* attValue = new nsAutoString();

    aContent->GetAttr(attNs, attName, *attValue);



    // Filter out special case of <br type="_moz*"> used by the editor.

    // Bug 16988.  Yuck.

    if (localName == nsGkAtoms::br && tagNS == kNameSpaceID_XHTML &&

        attName == nsGkAtoms::type && attNs == kNameSpaceID_None &&

        StringBeginsWith(*attValue, u"_moz"_ns)) {

      delete attValue;

      continue;

    }



    aBuilder.Append(u" ");



    if (MOZ_LIKELY(attNs == kNameSpaceID_None) ||
@@ -9184,17 +9184,14 @@ static void StartElement(Element* aContent, StringBuilder& aBuilder) { 
      aBuilder.Append(u"xmlns:");

    } else if (attNs == kNameSpaceID_XLink) {

      aBuilder.Append(u"xlink:");

    } else {

      nsAtom* prefix = name->GetPrefix();

      if (prefix) {

    } else if (nsAtom* prefix = name->GetPrefix()) {

      aBuilder.Append(prefix);

      aBuilder.Append(u":");

    }

    }



    aBuilder.Append(attName);

    aBuilder.Append(uR"(=")");

    AppendEncodedAttributeValue(attValue, aBuilder);

    AppendEncodedAttributeValue(*info.mValue, aBuilder);

    aBuilder.Append(uR"(")");

  }
@@ -9262,13 +9259,13 @@ static inline bool IsVoidTag(Element* aElement) { 
}



bool nsContentUtils::SerializeNodeToMarkup(nsINode* aRoot,

                                           bool aDescendentsOnly,

                                           bool aDescendantsOnly,

                                           nsAString& aOut) {

  // If you pass in a DOCUMENT_NODE, you must pass aDescendentsOnly as true

  MOZ_ASSERT(aDescendentsOnly || aRoot->NodeType() != nsINode::DOCUMENT_NODE);

  MOZ_ASSERT(aDescendantsOnly || aRoot->NodeType() != nsINode::DOCUMENT_NODE);



  nsINode* current =

      aDescendentsOnly ? aRoot->GetFirstChildOfTemplateOrNode() : aRoot;

      aDescendantsOnly ? aRoot->GetFirstChildOfTemplateOrNode() : aRoot;



  if (!current) {

    return true;
@@ -9311,14 +9308,14 @@ bool nsContentUtils::SerializeNodeToMarkup(nsINode* aRoot, 


      case nsINode::DOCUMENT_TYPE_NODE: {

        builder.Append(u"<!DOCTYPE ");

        builder.Append(current->NodeName());

        builder.Append(nsString(current->NodeName()));

        builder.Append(u">");

        break;

      }



      case nsINode::PROCESSING_INSTRUCTION_NODE: {

        builder.Append(u"<?");

        builder.Append(current->NodeName());

        builder.Append(nsString(current->NodeName()));

        builder.Append(u" ");

        builder.Append(static_cast<nsIContent*>(current)->GetText());

        builder.Append(u">");
@@ -9334,7 +9331,7 @@ bool nsContentUtils::SerializeNodeToMarkup(nsINode* aRoot, 
            elem->IsMathMLElement()) {

          builder.Append(elem->NodeInfo()->NameAtom());

        } else {

          builder.Append(current->NodeName());

          builder.Append(nsString(current->NodeName()));

        }

        builder.Append(u">");

      }
@@ -9362,7 +9359,7 @@ bool nsContentUtils::SerializeNodeToMarkup(nsINode* aRoot, 
        }

      }



      if (aDescendentsOnly && current == aRoot) {

      if (aDescendantsOnly && current == aRoot) {

        return builder.ToString(aOut);

      }

    }