Bug 1338347 - Refactor to group fields into a per-animation data structure. r?botond draft
authorKartikaya Gupta <kgupta@mozilla.com>
Mon, 20 Mar 2017 11:41:05 -0400
changeset 501574 2f271e4830d90aaab4841be62eb79dab2bd6955e
parent 501573 81dc51357f2bd13a33296b317164233c28088a77
child 501575 53c5f1a8a47af54f152f997fc190670ad83f27ca
push id50031
push userkgupta@mozilla.com
push dateMon, 20 Mar 2017 15:42:18 +0000
reviewersbotond
bugs1338347
milestone55.0a1
Bug 1338347 - Refactor to group fields into a per-animation data structure. r?botond No functional changes here, just creating a struct that we will expand in the next patch. MozReview-Commit-ID: HlSSvtIuYU5
gfx/layers/composite/AnimationMetricsTracker.cpp
gfx/layers/composite/AnimationMetricsTracker.h
--- a/gfx/layers/composite/AnimationMetricsTracker.cpp
+++ b/gfx/layers/composite/AnimationMetricsTracker.cpp
@@ -12,19 +12,16 @@
 #define AMT_LOG(...)
 // #define AMT_LOG(...) printf_stderr("AMT: " __VA_ARGS__)
 
 namespace mozilla {
 namespace layers {
 
 AnimationMetricsTracker::AnimationMetricsTracker()
   : mMaxLayerAreaAnimated(0)
-  , mChromeAnimationFrameCount(0)
-  , mContentAnimationFrameCount(0)
-  , mApzAnimationFrameCount(0)
 {
 }
 
 AnimationMetricsTracker::~AnimationMetricsTracker()
 {
 }
 
 void
@@ -45,36 +42,33 @@ AnimationMetricsTracker::UpdateAnimation
       AnimationStarted();
     } else {
       mMaxLayerAreaAnimated = std::max(mMaxLayerAreaAnimated, aLayerArea);
     }
   }
 
   UpdateAnimationThroughput("chrome",
                             (aActive & AnimationProcessTypes::eChrome) != AnimationProcessTypes::eNone,
-                            mChromeAnimationStart,
-                            mChromeAnimationFrameCount,
+                            mChromeAnimation,
                             aVsyncInterval,
                             Telemetry::COMPOSITOR_ANIMATION_THROUGHPUT_CHROME);
   UpdateAnimationThroughput("content",
                             (aActive & AnimationProcessTypes::eContent) != AnimationProcessTypes::eNone,
-                            mContentAnimationStart,
-                            mContentAnimationFrameCount,
+                            mContentAnimation,
                             aVsyncInterval,
                             Telemetry::COMPOSITOR_ANIMATION_THROUGHPUT_CONTENT);
 }
 
 void
 AnimationMetricsTracker::UpdateApzAnimationInProgress(bool aInProgress,
                                                       TimeDuration aVsyncInterval)
 {
   UpdateAnimationThroughput("apz",
                             aInProgress,
-                            mApzAnimationStart,
-                            mApzAnimationFrameCount,
+                            mApzAnimation,
                             aVsyncInterval,
                             Telemetry::COMPOSITOR_ANIMATION_THROUGHPUT_APZ);
 }
 
 void
 AnimationMetricsTracker::AnimationStarted()
 {
 }
@@ -89,35 +83,34 @@ AnimationMetricsTracker::AnimationEnded(
   AMT_LOG("Ended animation; duration: %f ms, area: %" PRIu64 "\n",
     (TimeStamp::Now() - mCurrentAnimationStart).ToMilliseconds(),
     mMaxLayerAreaAnimated);
 }
 
 void
 AnimationMetricsTracker::UpdateAnimationThroughput(const char* aLabel,
                                                    bool aInProgress,
-                                                   TimeStamp& aStartTime,
-                                                   uint32_t& aFrameCount,
+                                                   AnimationData& aAnimation,
                                                    TimeDuration aVsyncInterval,
                                                    Telemetry::HistogramID aHistogram)
 {
-  if (aInProgress && !aStartTime) {
+  if (aInProgress && !aAnimation.mStart) {
     // the animation just started
-    aStartTime = TimeStamp::Now();
-    aFrameCount = 1;
+    aAnimation.mStart = TimeStamp::Now();
+    aAnimation.mFrameCount = 1;
     AMT_LOG("Compositor animation of type %s just started\n", aLabel);
-  } else if (aInProgress && aStartTime) {
+  } else if (aInProgress && aAnimation.mStart) {
     // the animation continues
-    aFrameCount++;
-  } else if (!aInProgress && aStartTime) {
+    aAnimation.mFrameCount++;
+  } else if (!aInProgress && aAnimation.mStart) {
     // the animation just ended
 
-    // Get the length and clear aStartTime before the early-returns below
-    TimeDuration animationLength = TimeStamp::Now() - aStartTime;
-    aStartTime = TimeStamp();
+    // Get the length and clear aAnimation.mStart before the early-returns below
+    TimeDuration animationLength = TimeStamp::Now() - aAnimation.mStart;
+    aAnimation.mStart = TimeStamp();
 
     if (aVsyncInterval == TimeDuration::Forever()) {
       AMT_LOG("Invalid vsync interval: forever\n");
       return;
     }
     double vsyncIntervalMs = aVsyncInterval.ToMilliseconds();
     if (vsyncIntervalMs < 1.0f) {
       // Guard to avoid division by zero or other crazy results below
@@ -127,26 +120,26 @@ AnimationMetricsTracker::UpdateAnimation
 
     // We round the expectedFrameCount because it's a count and should be an
     // integer. The animationLength might not be an exact vsync multiple because
     // it's taken during the composition process and the amount of work done
     // between the vsync signal and the Timestamp::Now() call may vary slightly
     // from one composite to another.
     uint32_t expectedFrameCount = std::lround(animationLength.ToMilliseconds() / vsyncIntervalMs);
     AMT_LOG("Type %s ran for %fms (interval: %fms), %u frames (expected: %u)\n",
-        aLabel, animationLength.ToMilliseconds(), vsyncIntervalMs, aFrameCount,
-        expectedFrameCount);
+        aLabel, animationLength.ToMilliseconds(), vsyncIntervalMs,
+        aAnimation.mFrameCount, expectedFrameCount);
     if (expectedFrameCount <= 0) {
       // Graceful handling of probably impossible thing, unless the clock
       // changes while running?
       return;
     }
 
     // Scale up by 1000 because telemetry takes ints, truncate intentionally
     // to avoid artificial inflation of the result.
-    uint32_t frameHitRatio = (uint32_t)(1000.0f * aFrameCount / expectedFrameCount);
+    uint32_t frameHitRatio = (uint32_t)(1000.0f * aAnimation.mFrameCount / expectedFrameCount);
     Telemetry::Accumulate(aHistogram, frameHitRatio);
     AMT_LOG("Reported frameHitRatio %u\n", frameHitRatio);
   }
 }
 
 } // namespace layers
 } // namespace mozilla
--- a/gfx/layers/composite/AnimationMetricsTracker.h
+++ b/gfx/layers/composite/AnimationMetricsTracker.h
@@ -40,43 +40,47 @@ public:
 
   /**
    * Similar to UpdateAnimationInProgress, but this is for APZ animations. Again,
    * this should be called per composite.
    */
   void UpdateApzAnimationInProgress(bool aInProgress, TimeDuration aVsyncInterval);
 
 private:
+  // A struct to group data that we need for each type of compositor animation.
+  struct AnimationData {
+    // The start time of the current animation.
+    TimeStamp mStart;
+    // The number of frames composited for the current animation.
+    uint32_t mFrameCount;
+
+    AnimationData()
+      : mFrameCount(0)
+    {
+    }
+  };
+
   void AnimationStarted();
   void AnimationEnded();
   void UpdateAnimationThroughput(const char* aLabel,
                                  bool aInProgress,
-                                 TimeStamp& aStartTime,
-                                 uint32_t& aFrameCount,
+                                 AnimationData& aAnimationData,
                                  TimeDuration aVsyncInterval,
                                  Telemetry::HistogramID aHistogram);
 
   // The start time of the current compositor animation. This just tracks
   // whether the compositor is running an animation, without regard to which
   // process the animation is coming from.
   TimeStamp mCurrentAnimationStart;
   // The max area (in layer pixels) that the current compositor animation
   // has touched on any given animation frame.
   uint64_t mMaxLayerAreaAnimated;
 
-  // The start time of the current chrome-process animation.
-  TimeStamp mChromeAnimationStart;
-  // The number of frames composited for the current chrome-process animation.
-  uint32_t mChromeAnimationFrameCount;
-  // The start time of the current content-process animation.
-  TimeStamp mContentAnimationStart;
-  // The number of frames composited for the current content-process animation.
-  uint32_t mContentAnimationFrameCount;
-  // The start time of the current APZ animation.
-  TimeStamp mApzAnimationStart;
-  // The number of frames composited for the current APZ animation.
-  uint32_t mApzAnimationFrameCount;
+  // We keep an instance of the struct for each type of compositor animation.
+  AnimationData mChromeAnimation;
+  AnimationData mContentAnimation;
+  AnimationData mApzAnimation;
 };
 
 } // namespace layers
 } // namespace mozilla
 
 #endif // mozilla_layers_AnimationMetricsTracker_h