Bug 1340958 - Put computed values into AnimationValueMap instead of hashtable in gecko. r?heycam draft
authorHiroyuki Ikezoe <hikezoe@mozilla.com>
Fri, 17 Mar 2017 13:23:21 +0900
changeset 500413 d1252f6b56cc3faff6029fddb61d806011929bc5
parent 500412 65829f2ad612553196750081c512523ad777208e
child 500414 6facc8359e3b6561903b60ecac99da9dcc8e7921
push id49724
push userhikezoe@mozilla.com
push dateFri, 17 Mar 2017 06:26:10 +0000
reviewersheycam
bugs1340958
milestone55.0a1
Bug 1340958 - Put computed values into AnimationValueMap instead of hashtable in gecko. r?heycam Before this patch, we store each computed values in a hashtable, nsRefPtrHashtable<nsUint32HashKey, RawServoAnimationValue>, for all KeyframeEffectReadOnly on an element, and convert the ServoAnimationValues of the hashtable into an nsTArray<ServoAnimationValue*> and then convert the ServoAnimationValues of the nsTArray into PropertyDeclarationBlock in rust. This way was really inefficient. In this patch, we store the computed values into AnimationValueMap and convert all AnimationValue in the map into PropertyDeclarationBlock after EffectCompositor::GetAnimationRule. MozReview-Commit-ID: EJ2Kl65fVeF
dom/animation/Animation.cpp
dom/animation/EffectCompositor.cpp
dom/animation/EffectCompositor.h
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
layout/style/ServoBindingList.h
layout/style/ServoBindings.cpp
layout/style/ServoBindings.h
--- a/dom/animation/Animation.cpp
+++ b/dom/animation/Animation.cpp
@@ -1512,14 +1512,14 @@ Animation::IsRunningOnCompositor() const
 template
 void
 Animation::ComposeStyle<RefPtr<AnimValuesStyleRule>&>(
   RefPtr<AnimValuesStyleRule>& aAnimationRule,
   const nsCSSPropertyIDSet& aPropertiesToSkip);
 
 template
 void
-Animation::ComposeStyle<RefPtr<ServoAnimationRule>&>(
-  RefPtr<ServoAnimationRule>& aAnimationRule,
+Animation::ComposeStyle<const RawServoAnimationValueMap&>(
+  const RawServoAnimationValueMap& aAnimationValues,
   const nsCSSPropertyIDSet& aPropertiesToSkip);
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/animation/EffectCompositor.cpp
+++ b/dom/animation/EffectCompositor.cpp
@@ -472,54 +472,54 @@ namespace {
         Equals(a, b) ||
         a->GetAnimation()->HasLowerCompositeOrderThan(*b->GetAnimation()) !=
           b->GetAnimation()->HasLowerCompositeOrderThan(*a->GetAnimation()));
       return a->GetAnimation()->HasLowerCompositeOrderThan(*b->GetAnimation());
     }
   };
 }
 
-ServoAnimationRule*
-EffectCompositor::GetServoAnimationRule(const dom::Element* aElement,
-                                        CSSPseudoElementType aPseudoType,
-                                        CascadeLevel aCascadeLevel)
+bool
+EffectCompositor::GetServoAnimationRule(
+  const dom::Element* aElement,
+  CSSPseudoElementType aPseudoType,
+  CascadeLevel aCascadeLevel,
+  RawServoAnimationValueMapBorrowed aAnimationValues)
 {
+  MOZ_ASSERT(aAnimationValues);
   MOZ_ASSERT(mPresContext && mPresContext->IsDynamic(),
              "Should not be in print preview");
 
   EffectSet* effectSet = EffectSet::GetEffectSet(aElement, aPseudoType);
   if (!effectSet) {
-    return nullptr;
+    return false;
   }
 
   // Get a list of effects sorted by composite order.
   nsTArray<KeyframeEffectReadOnly*> sortedEffectList(effectSet->Count());
   for (KeyframeEffectReadOnly* effect : *effectSet) {
     sortedEffectList.AppendElement(effect);
   }
   sortedEffectList.Sort(EffectCompositeOrderComparator());
 
-  AnimationRule& animRule = effectSet->AnimationRule(aCascadeLevel);
-  animRule.mServo = nullptr;
-
   // If multiple animations affect the same property, animations with higher
   // composite order (priority) override or add or animations with lower
   // priority.
   const nsCSSPropertyIDSet propertiesToSkip =
     aCascadeLevel == CascadeLevel::Animations
       ? effectSet->PropertiesForAnimationsLevel().Inverse()
       : effectSet->PropertiesForAnimationsLevel();
   for (KeyframeEffectReadOnly* effect : sortedEffectList) {
-    effect->GetAnimation()->ComposeStyle(animRule.mServo, propertiesToSkip);
+    effect->GetAnimation()->ComposeStyle(*aAnimationValues, propertiesToSkip);
   }
 
   MOZ_ASSERT(effectSet == EffectSet::GetEffectSet(aElement, aPseudoType),
              "EffectSet should not change while composing style");
 
-  return animRule.mServo;
+  return true;
 }
 
 /* static */ dom::Element*
 EffectCompositor::GetElementToRestyle(dom::Element* aElement,
                                       CSSPseudoElementType aPseudoType)
 {
   if (aPseudoType == CSSPseudoElementType::NotPseudo) {
     return aElement;
--- a/dom/animation/EffectCompositor.h
+++ b/dom/animation/EffectCompositor.h
@@ -19,16 +19,18 @@
 #include "nsTArray.h"
 
 class nsCSSPropertyIDSet;
 class nsIAtom;
 class nsIFrame;
 class nsIStyleRule;
 class nsPresContext;
 class nsStyleContext;
+struct RawServoAnimationValueMap;
+typedef RawServoAnimationValueMap const* RawServoAnimationValueMapBorrowed;
 
 namespace mozilla {
 
 class EffectSet;
 class RestyleTracker;
 class StyleAnimationValue;
 class ServoAnimationRule;
 struct AnimationPerformanceWarning;
@@ -148,21 +150,25 @@ public:
   // will use a style context associated with the primary frame of the specified
   // (pseudo-)element.
   nsIStyleRule* GetAnimationRule(dom::Element* aElement,
                                  CSSPseudoElementType aPseudoType,
                                  CascadeLevel aCascadeLevel,
                                  nsStyleContext* aStyleContext);
 
   // Get animation rule for stylo. This is an equivalent of GetAnimationRule
-  // and will be called from servo side. We need to be careful while doing any
-  // modification because it may case some thread-safe issues.
-  ServoAnimationRule* GetServoAnimationRule(const dom::Element* aElement,
-                                            CSSPseudoElementType aPseudoType,
-                                            CascadeLevel aCascadeLevel);
+  // and will be called from servo side.
+  // The animation rule is stored in |RawServoAnimationValueMapBorrowed|.
+  // We need to be careful while doing any modification because it may cause
+  // some thread-safe issues.
+  bool GetServoAnimationRule(
+    const dom::Element* aElement,
+    CSSPseudoElementType aPseudoType,
+    CascadeLevel aCascadeLevel,
+    RawServoAnimationValueMapBorrowed aAnimationValues);
 
   bool HasPendingStyleUpdates() const;
   bool HasThrottledStyleUpdates() const;
 
   // Tell the restyle tracker about all the animated styles that have
   // pending updates so that it can update the animation rule for these
   // elements.
   void AddStyleUpdatesTo(RestyleTracker& aTracker);
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -588,44 +588,43 @@ KeyframeEffectReadOnly::ComposeStyleRule
     aStyleRule->AddValue(aProperty.mProperty, Move(toValue));
   }
 }
 
 // Bug 1333311 - We use two branches for Gecko and Stylo. However, it's
 // better to remove the duplicated code.
 void
 KeyframeEffectReadOnly::ComposeStyleRule(
-  RefPtr<ServoAnimationRule>& aAnimationRule,
+  const RawServoAnimationValueMap& aAnimationValues,
   const AnimationProperty& aProperty,
   const AnimationPropertySegment& aSegment,
   const ComputedTiming& aComputedTiming)
 {
   // Bug 1329878 - Stylo: Implement accumulate and addition on Servo
   // AnimationValue.
   RawServoAnimationValue* servoFromValue = aSegment.mFromValue.mServo;
   RawServoAnimationValue* servoToValue = aSegment.mToValue.mServo;
 
   // For unsupported or non-animatable animation types, we get nullptrs.
   if (!servoFromValue || !servoToValue) {
     NS_ERROR("Compose style for unsupported or non-animatable property, "
              "so get invalid RawServoAnimationValues");
     return;
   }
 
-  if (!aAnimationRule) {
-    // Allocate the style rule now that we know we have animation data.
-    aAnimationRule = new ServoAnimationRule();
-  }
-
   // Special handling for zero-length segments
   if (aSegment.mToKey == aSegment.mFromKey) {
     if (aComputedTiming.mProgress.Value() < 0) {
-      aAnimationRule->AddValue(aProperty.mProperty, servoFromValue);
+      Servo_AnimationValueMap_Push(&aAnimationValues,
+                                   aProperty.mProperty,
+                                   servoFromValue);
     } else {
-      aAnimationRule->AddValue(aProperty.mProperty, servoToValue);
+      Servo_AnimationValueMap_Push(&aAnimationValues,
+                                   aProperty.mProperty,
+                                   servoToValue);
     }
     return;
   }
 
   double positionInSegment =
     (aComputedTiming.mProgress.Value() - aSegment.mFromKey) /
     (aSegment.mToKey - aSegment.mFromKey);
   double valuePosition =
@@ -636,21 +635,27 @@ KeyframeEffectReadOnly::ComposeStyleRule
   MOZ_ASSERT(IsFinite(valuePosition), "Position value should be finite");
 
   RefPtr<RawServoAnimationValue> interpolated =
     Servo_AnimationValues_Interpolate(servoFromValue,
                                       servoToValue,
                                       valuePosition).Consume();
 
   if (interpolated) {
-    aAnimationRule->AddValue(aProperty.mProperty, interpolated);
+    Servo_AnimationValueMap_Push(&aAnimationValues,
+                                 aProperty.mProperty,
+                                 interpolated);
   } else if (valuePosition < 0.5) {
-    aAnimationRule->AddValue(aProperty.mProperty, servoFromValue);
+    Servo_AnimationValueMap_Push(&aAnimationValues,
+                                 aProperty.mProperty,
+                                 servoFromValue);
   } else {
-    aAnimationRule->AddValue(aProperty.mProperty, servoToValue);
+    Servo_AnimationValueMap_Push(&aAnimationValues,
+                                 aProperty.mProperty,
+                                 servoToValue);
   }
 }
 
 template<typename ComposeAnimationResult>
 void
 KeyframeEffectReadOnly::ComposeStyle(
   ComposeAnimationResult&& aComposeResult,
   const nsCSSPropertyIDSet& aPropertiesToSkip)
@@ -1816,14 +1821,14 @@ KeyframeEffectReadOnly::ContainsAnimated
 template
 void
 KeyframeEffectReadOnly::ComposeStyle<RefPtr<AnimValuesStyleRule>&>(
   RefPtr<AnimValuesStyleRule>& aAnimationRule,
   const nsCSSPropertyIDSet& aPropertiesToSkip);
 
 template
 void
-KeyframeEffectReadOnly::ComposeStyle<RefPtr<ServoAnimationRule>&>(
-  RefPtr<ServoAnimationRule>& aAnimationRule,
+KeyframeEffectReadOnly::ComposeStyle<const RawServoAnimationValueMap&>(
+  const RawServoAnimationValueMap& aAnimationValues,
   const nsCSSPropertyIDSet& aPropertiesToSkip);
 
 } // namespace dom
 } // namespace mozilla
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -459,17 +459,17 @@ private:
   template<typename StyleType>
   void DoUpdateProperties(StyleType&& aStyle);
 
   void ComposeStyleRule(RefPtr<AnimValuesStyleRule>& aStyleRule,
                         const AnimationProperty& aProperty,
                         const AnimationPropertySegment& aSegment,
                         const ComputedTiming& aComputedTiming);
 
-  void ComposeStyleRule(RefPtr<ServoAnimationRule>& aStyleRule,
+  void ComposeStyleRule(const RawServoAnimationValueMap& aAnimationValues,
                         const AnimationProperty& aProperty,
                         const AnimationPropertySegment& aSegment,
                         const ComputedTiming& aComputedTiming);
 
   nsIFrame* GetAnimationFrame() const;
 
   bool CanThrottle() const;
   bool CanThrottleTransformChanges(nsIFrame& aFrame) const;
--- a/layout/style/ServoBindingList.h
+++ b/layout/style/ServoBindingList.h
@@ -119,16 +119,20 @@ SERVO_BINDING_FUNC(Servo_ParseProperty,
                    const nsACString* base,
                    const GeckoParserExtraData* data)
 SERVO_BINDING_FUNC(Servo_GetComputedKeyframeValues, void,
                    RawGeckoKeyframeListBorrowed keyframes,
                    ServoComputedValuesBorrowed style,
                    ServoComputedValuesBorrowedOrNull parent_style,
                    RawServoStyleSetBorrowed set,
                    RawGeckoComputedKeyframeValuesListBorrowedMut result)
+SERVO_BINDING_FUNC(Servo_AnimationValueMap_Push, void,
+                   RawServoAnimationValueMapBorrowed,
+                   nsCSSPropertyID property,
+                   RawServoAnimationValueBorrowed value)
 
 // AnimationValues handling
 SERVO_BINDING_FUNC(Servo_AnimationValues_Interpolate,
                    RawServoAnimationValueStrong,
                    RawServoAnimationValueBorrowed from,
                    RawServoAnimationValueBorrowed to,
                    double progress)
 SERVO_BINDING_FUNC(Servo_AnimationValues_Uncompute,
--- a/layout/style/ServoBindings.cpp
+++ b/layout/style/ServoBindings.cpp
@@ -383,49 +383,46 @@ Gecko_GetHTMLPresentationAttrDeclaration
     }
     return nullptr;
   }
 
   const RefPtr<RawServoDeclarationBlock>& servo = attrs->GetServoStyle();
   return reinterpret_cast<const RawServoDeclarationBlockStrong*>(&servo);
 }
 
-RawServoDeclarationBlockStrong
+bool
 Gecko_GetAnimationRule(RawGeckoElementBorrowed aElement,
                        nsIAtom* aPseudoTag,
-                       EffectCompositor::CascadeLevel aCascadeLevel)
+                       EffectCompositor::CascadeLevel aCascadeLevel,
+                       RawServoAnimationValueMapBorrowed aAnimationValues)
 {
   MOZ_ASSERT(aElement, "Invalid GeckoElement");
   MOZ_ASSERT(!aPseudoTag ||
              aPseudoTag == nsCSSPseudoElements::before ||
              aPseudoTag == nsCSSPseudoElements::after);
 
-  const RawServoDeclarationBlockStrong emptyDeclarationBlock{ nullptr };
   nsIDocument* doc = aElement->GetComposedDoc();
   if (!doc || !doc->GetShell()) {
-    return emptyDeclarationBlock;
+    return false;
   }
   nsPresContext* presContext = doc->GetShell()->GetPresContext();
   if (!presContext || !presContext->IsDynamic()) {
     // For print or print preview, ignore animations.
-    return emptyDeclarationBlock;
+    return false;
   }
 
   CSSPseudoElementType pseudoType =
     nsCSSPseudoElements::GetPseudoType(
       aPseudoTag,
       nsCSSProps::EnabledState::eIgnoreEnabledState);
 
-  ServoAnimationRule* rule =
-    presContext->EffectCompositor()
-               ->GetServoAnimationRule(aElement, pseudoType, aCascadeLevel);
-  if (!rule) {
-    return emptyDeclarationBlock;
-  }
-  return rule->GetValues();
+  return presContext->EffectCompositor()
+    ->GetServoAnimationRule(aElement, pseudoType,
+                            aCascadeLevel,
+                            aAnimationValues);
 }
 
 bool
 Gecko_StyleAnimationsEquals(RawGeckoStyleAnimationListBorrowed aA,
                             RawGeckoStyleAnimationListBorrowed aB)
 {
   return *aA == *aB;
 }
--- a/layout/style/ServoBindings.h
+++ b/layout/style/ServoBindings.h
@@ -182,20 +182,21 @@ SERVO_DECLARE_ELEMENT_ATTR_MATCHING_FUNC
 
 // Style attributes.
 RawServoDeclarationBlockStrongBorrowedOrNull
 Gecko_GetStyleAttrDeclarationBlock(RawGeckoElementBorrowed element);
 RawServoDeclarationBlockStrongBorrowedOrNull
 Gecko_GetHTMLPresentationAttrDeclarationBlock(RawGeckoElementBorrowed element);
 
 // Animations
-RawServoDeclarationBlockStrong
+bool
 Gecko_GetAnimationRule(RawGeckoElementBorrowed aElement,
                        nsIAtom* aPseudoTag,
-                       mozilla::EffectCompositor::CascadeLevel aCascadeLevel);
+                       mozilla::EffectCompositor::CascadeLevel aCascadeLevel,
+                       RawServoAnimationValueMapBorrowed aAnimationValues);
 bool Gecko_StyleAnimationsEquals(RawGeckoStyleAnimationListBorrowed,
                                  RawGeckoStyleAnimationListBorrowed);
 void Gecko_UpdateAnimations(RawGeckoElementBorrowed aElement,
                             nsIAtom* aPseudoTagOrNull,
                             ServoComputedValuesBorrowedOrNull aComputedValues,
                             ServoComputedValuesBorrowedOrNull aParentComputedValues);
 bool Gecko_ElementHasCSSAnimations(RawGeckoElementBorrowed aElement,
                                    nsIAtom* aPseudoTagOrNull);