Bug 1376594 - Track locally whether an effect is part of an EffectSet to avoid hashmap lookups; r=hiro
authorBrian Birtles <birtles@gmail.com>
Wed, 05 Jul 2017 10:29:58 +0900
changeset 367972 82561d1913cbf0bdaa63e3697e398e8e099b8c54
parent 367971 1af6dfb5f04a3cd6271b5528632290c4b34a2494
child 367973 5dd25eb4898c5592663d3d661efbdd75328178a4
push id46173
push userbbirtles@mozilla.com
push dateMon, 10 Jul 2017 08:20:53 +0000
treeherderautoland@5dd25eb4898c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewershiro
bugs1376594
milestone56.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 1376594 - Track locally whether an effect is part of an EffectSet to avoid hashmap lookups; r=hiro MozReview-Commit-ID: IEeAmyR9ZlS
dom/animation/KeyframeEffectReadOnly.cpp
dom/animation/KeyframeEffectReadOnly.h
--- a/dom/animation/KeyframeEffectReadOnly.cpp
+++ b/dom/animation/KeyframeEffectReadOnly.cpp
@@ -952,31 +952,39 @@ KeyframeEffectReadOnly::UpdateTargetRegi
 
   // Animation::IsRelevant() returns a cached value. It only updates when
   // something calls Animation::UpdateRelevance. Whenever our timing changes,
   // we should be notifying our Animation before calling this, so
   // Animation::IsRelevant() should be up-to-date by the time we get here.
   MOZ_ASSERT(isRelevant == IsCurrent() || IsInEffect(),
              "Out of date Animation::IsRelevant value");
 
-  if (isRelevant) {
+  if (isRelevant && !mInEffectSet) {
     EffectSet* effectSet =
       EffectSet::GetOrCreateEffectSet(mTarget->mElement, mTarget->mPseudoType);
     effectSet->AddEffect(*this);
+    mInEffectSet = true;
     UpdateEffectSet(effectSet);
-  } else {
+  } else if (!isRelevant && mInEffectSet) {
     UnregisterTarget();
   }
 }
 
 void
 KeyframeEffectReadOnly::UnregisterTarget()
 {
+  if (!mInEffectSet) {
+    return;
+  }
+
   EffectSet* effectSet =
     EffectSet::GetEffectSet(mTarget->mElement, mTarget->mPseudoType);
+  MOZ_ASSERT(effectSet, "If mInEffectSet is true, there must be an EffectSet"
+                        " on the target element");
+  mInEffectSet = false;
   if (effectSet) {
     effectSet->RemoveEffect(*this);
 
     if (effectSet->IsEmpty()) {
       EffectSet::DestroyEffectSet(mTarget->mElement, mTarget->mPseudoType);
     }
   }
 }
@@ -1829,16 +1837,20 @@ KeyframeEffectReadOnly::ContainsAnimated
   }
 
   return false;
 }
 
 void
 KeyframeEffectReadOnly::UpdateEffectSet(EffectSet* aEffectSet) const
 {
+  if (!mInEffectSet) {
+    return;
+  }
+
   EffectSet* effectSet =
     aEffectSet ? aEffectSet
                : EffectSet::GetEffectSet(mTarget->mElement,
                                          mTarget->mPseudoType);
   if (!effectSet) {
     return;
   }
 
--- a/dom/animation/KeyframeEffectReadOnly.h
+++ b/dom/animation/KeyframeEffectReadOnly.h
@@ -412,16 +412,21 @@ protected:
 
   // The non-animated values for properties in this effect that contain at
   // least one animation value that is composited with the underlying value
   // (i.e. it uses the additive or accumulate composite mode).
   nsDataHashtable<nsUint32HashKey, StyleAnimationValue> mBaseStyleValues;
   nsRefPtrHashtable<nsUint32HashKey, RawServoAnimationValue>
     mBaseStyleValuesForServo;
 
+  // True if this effect is in the EffectSet for its target element. This is
+  // used as an optimization to avoid unnecessary hashmap lookups on the
+  // EffectSet.
+  bool mInEffectSet = false;
+
   // We only want to record telemetry data for "ContentTooLarge" warnings once
   // per effect:target pair so we use this member to record if we have already
   // reported a "ContentTooLarge" warning for the current target.
   bool mRecordedContentTooLarge = false;
   // Similarly, as a point of comparison we record telemetry whether or not
   // we get a "ContentTooLarge" warning, but again only once per effect:target
   // pair.
   bool mRecordedFrameSize = false;