Bug 1383650 - Optimize attribute mapping by not parsing same thing twice r=longsonr
authorviolet <violet.bugreport@gmail.com>
Thu, 16 May 2019 13:21:21 +0000
changeset 474705 94c0cc0049815467d23d4a778b68438607dc0c98
parent 474704 5514f80c4f48f8dca61cdfc88e2225244c236d19
child 474706 b05458d3fe67c7e8e368c4d415a6774be4c33dcc
push id36044
push userrmaries@mozilla.com
push dateTue, 21 May 2019 15:45:34 +0000
treeherdermozilla-central@78571bb1f20e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslongsonr
bugs1383650
milestone69.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1383650 - Optimize attribute mapping by not parsing same thing twice r=longsonr Geometry properties are the most used SVG attributes. When authors specify them as attributes, we have to parse them in SVG side. So we don't want to parse them in CSS side again, otherwise the introduced performance loss is relatively high. With this optimization, this feature implementation doesn't slow down overall performace even if there are thousands of geometry elements. Differential Revision: https://phabricator.services.mozilla.com/D30778
dom/svg/SVGElement.cpp
--- a/dom/svg/SVGElement.cpp
+++ b/dom/svg/SVGElement.cpp
@@ -1067,16 +1067,19 @@ class MOZ_STACK_CLASS MappedAttrParser {
   MappedAttrParser(css::Loader* aLoader, nsIURI* aDocURI,
                    already_AddRefed<nsIURI> aBaseURI, SVGElement* aElement);
   ~MappedAttrParser();
 
   // Parses a mapped attribute value.
   void ParseMappedAttrValue(nsAtom* aMappedAttrName,
                             const nsAString& aMappedAttrValue);
 
+  void TellStyleAlreadyParsedResult(nsAtom const* aAtom,
+                                    SVGAnimatedLength const& aLength);
+
   // If we've parsed any values for mapped attributes, this method returns the
   // already_AddRefed css::Declaration that incorporates the parsed
   // values. Otherwise, this method returns null.
   already_AddRefed<DeclarationBlock> GetDeclarationBlock();
 
  private:
   // MEMBER DATA
   // -----------
@@ -1154,16 +1157,28 @@ void MappedAttrParser::ParseMappedAttrVa
   // nsCSSParser doesn't know about 'lang', so we need to handle it specially.
   if (aMappedAttrName == nsGkAtoms::lang) {
     propertyID = eCSSProperty__x_lang;
     RefPtr<nsAtom> atom = NS_Atomize(aMappedAttrValue);
     Servo_DeclarationBlock_SetIdentStringValue(mDecl->Raw(), propertyID, atom);
   }
 }
 
+void MappedAttrParser::TellStyleAlreadyParsedResult(
+    nsAtom const* aAtom, SVGAnimatedLength const& aLength) {
+  if (!mDecl) {
+    mDecl = new DeclarationBlock();
+  }
+  nsCSSPropertyID propertyID =
+      nsCSSProps::LookupProperty(nsDependentAtomString(aAtom));
+
+  SVGElement::UpdateDeclarationBlockFromLength(*mDecl, propertyID, aLength,
+                                               SVGElement::ValToUse::Base);
+}
+
 already_AddRefed<DeclarationBlock> MappedAttrParser::GetDeclarationBlock() {
   return mDecl.forget();
 }
 
 }  // namespace
 
 //----------------------------------------------------------------------
 // Implementation Helpers:
@@ -1177,16 +1192,19 @@ void SVGElement::UpdateContentDeclaratio
     // nothing to do
     return;
   }
 
   Document* doc = OwnerDoc();
   MappedAttrParser mappedAttrParser(doc->CSSLoader(), doc->GetDocumentURI(),
                                     GetBaseURI(), this);
 
+  bool lengthAffectsStyle =
+      SVGGeometryProperty::ElementMapsLengthsToStyle(this);
+
   for (uint32_t i = 0; i < attrCount; ++i) {
     const nsAttrName* attrName = mAttrs.AttrNameAt(i);
     if (!attrName->IsAtom() || !IsAttributeMapped(attrName->Atom())) continue;
 
     if (attrName->NamespaceID() != kNameSpaceID_None &&
         !attrName->Equals(nsGkAtoms::lang, kNameSpaceID_XML)) {
       continue;
     }
@@ -1209,16 +1227,30 @@ void SVGElement::UpdateContentDeclaratio
         continue;
       }
       if (attrName->Atom() == nsGkAtoms::height &&
           !GetAnimatedLength(nsGkAtoms::height)->HasBaseVal()) {
         continue;
       }
     }
 
+    if (lengthAffectsStyle) {
+      auto const* length = GetAnimatedLength(attrName->Atom());
+
+      if (length && length->HasBaseVal()) {
+        // This is an element with geometry property set via SVG attribute,
+        // and the attribute is already successfully parsed. We want to go
+        // through the optimized path to tell the style system the result
+        // directly, rather than let it parse the same thing again.
+        mappedAttrParser.TellStyleAlreadyParsedResult(attrName->Atom(),
+                                                      *length);
+        continue;
+      }
+    }
+
     nsAutoString value;
     mAttrs.AttrAt(i)->ToString(value);
     mappedAttrParser.ParseMappedAttrValue(attrName->Atom(), value);
   }
   mContentDeclarationBlock = mappedAttrParser.GetDeclarationBlock();
 }
 
 const DeclarationBlock* SVGElement::GetContentDeclarationBlock() const {
@@ -1452,17 +1484,16 @@ void SVGElement::DidAnimateLength(uint8_
 SVGAnimatedLength* SVGElement::GetAnimatedLength(const nsAtom* aAttrName) {
   LengthAttributesInfo lengthInfo = GetLengthInfo();
 
   for (uint32_t i = 0; i < lengthInfo.mLengthCount; i++) {
     if (aAttrName == lengthInfo.mLengthInfo[i].mName) {
       return &lengthInfo.mLengths[i];
     }
   }
-  MOZ_ASSERT(false, "no matching length found");
   return nullptr;
 }
 
 void SVGElement::GetAnimatedLengthValues(float* aFirst, ...) {
   LengthAttributesInfo info = GetLengthInfo();
 
   NS_ASSERTION(info.mLengthCount > 0,
                "GetAnimatedLengthValues on element with no length attribs");