Bug 1383650 - Optimize attribute mapping by not parsing same thing twice r=longsonr
☠☠ backed out by 1624c5a31917 ☠ ☠
authorviolet <violet.bugreport@gmail.com>
Thu, 16 May 2019 00:54:46 +0000
changeset 532851 6730776560c028a402e7112569e6ca8bf6cd742b
parent 532850 a7b8e6460fb8b1245507e9e15deadc1e62674102
child 532852 447c9248342b3ab04801d4684013042140085b23
push id11272
push userapavel@mozilla.com
push dateThu, 16 May 2019 15:28:22 +0000
treeherdermozilla-beta@2265bfc5920d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerslongsonr
bugs1383650
milestone68.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");