Bug 1285867 (Part 6) - Record Decoder telemetry outside of the loop. r=edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Mon, 11 Jul 2016 00:44:39 -0700
changeset 345201 9b650c8855c2a64e6194680eee53c53b113e4798
parent 345200 4840df0f7046913f4582487522b4ff73ceef96c0
child 345202 401978e000d8e8819c0ef3a3b5b081e408dc711c
push id6389
push userraliiev@mozilla.com
push dateMon, 19 Sep 2016 13:38:22 +0000
treeherdermozilla-beta@01d67bfe6c81 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin
bugs1285867
milestone50.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 1285867 (Part 6) - Record Decoder telemetry outside of the loop. r=edwin
image/Decoder.cpp
image/Decoder.h
image/SourceBuffer.h
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -20,28 +20,23 @@ using mozilla::gfx::IntSize;
 using mozilla::gfx::SurfaceFormat;
 
 namespace mozilla {
 namespace image {
 
 class MOZ_STACK_CLASS AutoRecordDecoderTelemetry final
 {
 public:
-  AutoRecordDecoderTelemetry(Decoder* aDecoder, uint32_t aByteCount)
+  explicit AutoRecordDecoderTelemetry(Decoder* aDecoder)
     : mDecoder(aDecoder)
   {
     MOZ_ASSERT(mDecoder);
 
     // Begin recording telemetry data.
     mStartTime = TimeStamp::Now();
-    mDecoder->mChunkCount++;
-
-    // Keep track of the total number of bytes written.
-    mDecoder->mBytesDecoded += aByteCount;
-
   }
 
   ~AutoRecordDecoderTelemetry()
   {
     // Finish telemetry.
     mDecoder->mDecodeTime += (TimeStamp::Now() - mStartTime);
   }
 
@@ -53,20 +48,18 @@ private:
 Decoder::Decoder(RasterImage* aImage)
   : mImageData(nullptr)
   , mImageDataLength(0)
   , mColormap(nullptr)
   , mColormapSize(0)
   , mImage(aImage)
   , mProgress(NoProgress)
   , mFrameCount(0)
-  , mChunkCount(0)
   , mDecoderFlags(DefaultDecoderFlags())
   , mSurfaceFlags(DefaultSurfaceFlags())
-  , mBytesDecoded(0)
   , mInitialized(false)
   , mMetadataDecode(false)
   , mInFrame(false)
   , mReachedTerminalState(false)
   , mDecodeDone(false)
   , mDataError(false)
   , mDecodeAborted(false)
   , mShouldReportError(false)
@@ -119,63 +112,63 @@ Decoder::Decode(NotNull<IResumable*> aOn
   MOZ_ASSERT(mInitialized, "Should be initialized here");
   MOZ_ASSERT(mIterator, "Should have a SourceBufferIterator");
 
   // If we're already done, don't attempt to keep decoding.
   if (GetDecodeDone()) {
     return HasError() ? NS_ERROR_FAILURE : NS_OK;
   }
 
+  Maybe<TerminalState> terminalState;
+
   // We keep decoding chunks until the decode completes (i.e., we reach a
   // terminal state) or there are no more chunks available.
-  Maybe<TerminalState> terminalState;
-  do {
-    if (GetDecodeDone()) {
-      MOZ_ASSERT_UNREACHABLE("Finished decode without reaching terminal state?");
-      terminalState = Some(TerminalState::SUCCESS);
-      break;
-    }
-
-    switch (mIterator->AdvanceOrScheduleResume(aOnResume.get())) {
-      case SourceBufferIterator::WAITING:
-        // We can't continue because the rest of the data hasn't arrived from
-        // the network yet. We don't have to do anything special; the
-        // SourceBufferIterator will ensure that Decode() gets called again on a
-        // DecodePool thread when more data is available.
-        return NS_OK;
+  {
+    PROFILER_LABEL("ImageDecoder", "Decode",
+                   js::ProfileEntry::Category::GRAPHICS);
+    AutoRecordDecoderTelemetry telemetry(this);
 
-      case SourceBufferIterator::COMPLETE:
-        // Normally even if the data is truncated, we want decoding to
-        // succeed so we can display whatever we got. However, if the
-        // SourceBuffer was completed with a failing status, we want to fail.
-        // This happens only in exceptional situations like SourceBuffer
-        // itself encountering a failure due to OOM.
-        terminalState = NS_SUCCEEDED(mIterator->CompletionStatus())
-                      ? Some(TerminalState::SUCCESS)
-                      : Some(TerminalState::FAILURE);
-
-        break;
-
-      case SourceBufferIterator::READY: {
-        PROFILER_LABEL("ImageDecoder", "Decode",
-                       js::ProfileEntry::Category::GRAPHICS);
-
-        AutoRecordDecoderTelemetry telemetry(this, mIterator->Length());
-
-        // Pass the data along to the implementation.
-        terminalState = DoDecode(*mIterator);
-
+    do {
+      if (GetDecodeDone()) {
+        MOZ_ASSERT_UNREACHABLE("Finished decode without reaching terminal state?");
+        terminalState = Some(TerminalState::SUCCESS);
         break;
       }
 
-      default:
-        MOZ_ASSERT_UNREACHABLE("Unknown SourceBufferIterator state");
-        terminalState = Some(TerminalState::FAILURE);
-    }
-  } while (!terminalState);
+      switch (mIterator->AdvanceOrScheduleResume(aOnResume.get())) {
+        case SourceBufferIterator::WAITING:
+          // We can't continue because the rest of the data hasn't arrived from
+          // the network yet. We don't have to do anything special; the
+          // SourceBufferIterator will ensure that Decode() gets called again on a
+          // DecodePool thread when more data is available.
+          return NS_OK;
+
+        case SourceBufferIterator::COMPLETE:
+          // Normally even if the data is truncated, we want decoding to
+          // succeed so we can display whatever we got. However, if the
+          // SourceBuffer was completed with a failing status, we want to fail.
+          // This happens only in exceptional situations like SourceBuffer
+          // itself encountering a failure due to OOM.
+          terminalState = NS_SUCCEEDED(mIterator->CompletionStatus())
+                        ? Some(TerminalState::SUCCESS)
+                        : Some(TerminalState::FAILURE);
+
+          break;
+
+        case SourceBufferIterator::READY:
+          // Pass the data along to the implementation.
+          terminalState = DoDecode(*mIterator);
+          break;
+
+        default:
+          MOZ_ASSERT_UNREACHABLE("Unknown SourceBufferIterator state");
+          terminalState = Some(TerminalState::FAILURE);
+      }
+    } while (!terminalState);
+  }
 
   MOZ_ASSERT(terminalState);
   mReachedTerminalState = true;
 
   // If decoding failed, record that fact.
   if (terminalState == Some(TerminalState::FAILURE)) {
     PostDataError();
   }
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -159,23 +159,31 @@ public:
   /**
    * Should we stop decoding after the first frame?
    */
   bool IsFirstFrameDecode() const
   {
     return bool(mDecoderFlags & DecoderFlags::FIRST_FRAME_ONLY);
   }
 
-  size_t BytesDecoded() const { return mBytesDecoded; }
+  size_t BytesDecoded() const
+  {
+    MOZ_ASSERT(mIterator);
+    return mIterator->ByteCount();
+  }
 
   // The amount of time we've spent inside DoDecode() so far for this decoder.
   TimeDuration DecodeTime() const { return mDecodeTime; }
 
   // The number of chunks this decoder's input was divided into.
-  uint32_t ChunkCount() const { return mChunkCount; }
+  uint32_t ChunkCount() const
+  {
+    MOZ_ASSERT(mIterator);
+    return mIterator->ChunkCount();
+  }
 
   // The number of frames we have, including anything in-progress. Thus, this
   // is only 0 if we haven't begun any frames.
   uint32_t GetFrameCount() { return mFrameCount; }
 
   // The number of complete frames we have (ie, not including anything
   // in-progress).
   uint32_t GetCompleteFrameCount() {
@@ -411,21 +419,19 @@ private:
   ImageMetadata mImageMetadata;
   nsIntRect mInvalidRect; // Tracks an invalidation region in the current frame.
   Progress mProgress;
 
   uint32_t mFrameCount; // Number of frames, including anything in-progress
 
   // Telemetry data for this decoder.
   TimeDuration mDecodeTime;
-  uint32_t mChunkCount;
 
   DecoderFlags mDecoderFlags;
   SurfaceFlags mSurfaceFlags;
-  size_t mBytesDecoded;
 
   bool mInitialized : 1;
   bool mMetadataDecode : 1;
   bool mInFrame : 1;
   bool mReachedTerminalState : 1;
   bool mDecodeDone : 1;
   bool mDataError : 1;
   bool mDecodeAborted : 1;
--- a/image/SourceBuffer.h
+++ b/image/SourceBuffer.h
@@ -75,37 +75,43 @@ public:
     READY,    // The iterator is pointing to new data.
     WAITING,  // The iterator is blocked and the caller must yield.
     COMPLETE  // The iterator is pointing to the end of the buffer.
   };
 
   explicit SourceBufferIterator(SourceBuffer* aOwner)
     : mOwner(aOwner)
     , mState(START)
+    , mChunkCount(0)
+    , mByteCount(0)
   {
     MOZ_ASSERT(aOwner);
     mData.mIterating.mChunk = 0;
     mData.mIterating.mData = nullptr;
     mData.mIterating.mOffset = 0;
     mData.mIterating.mLength = 0;
   }
 
   SourceBufferIterator(SourceBufferIterator&& aOther)
     : mOwner(Move(aOther.mOwner))
     , mState(aOther.mState)
     , mData(aOther.mData)
+    , mChunkCount(aOther.mChunkCount)
+    , mByteCount(aOther.mByteCount)
   { }
 
   ~SourceBufferIterator();
 
   SourceBufferIterator& operator=(SourceBufferIterator&& aOther)
   {
     mOwner = Move(aOther.mOwner);
     mState = aOther.mState;
     mData = aOther.mData;
+    mChunkCount = aOther.mChunkCount;
+    mByteCount = aOther.mByteCount;
     return *this;
   }
 
   /**
    * Returns true if there are no more than @aBytes remaining in the
    * SourceBuffer. If the SourceBuffer is not yet complete, returns false.
    */
   bool RemainingBytesIsNoMoreThan(size_t aBytes) const;
@@ -135,32 +141,45 @@ public:
 
   /// If we're ready to read, returns the length of the new data.
   size_t Length() const
   {
     MOZ_ASSERT(mState == READY, "Calling Length() in the wrong state");
     return mState == READY ? mData.mIterating.mLength : 0;
   }
 
+  /// @return a count of the chunks we've advanced through.
+  uint32_t ChunkCount() const { return mChunkCount; }
+
+  /// @return a count of the bytes in all chunks we've advanced through.
+  size_t ByteCount() const { return mByteCount; }
+
 private:
   friend class SourceBuffer;
 
   SourceBufferIterator(const SourceBufferIterator&) = delete;
   SourceBufferIterator& operator=(const SourceBufferIterator&) = delete;
 
   bool HasMore() const { return mState != COMPLETE; }
 
   State SetReady(uint32_t aChunk, const char* aData,
                 size_t aOffset, size_t aLength)
   {
     MOZ_ASSERT(mState != COMPLETE);
+
+    // Update state.
     mData.mIterating.mChunk = aChunk;
     mData.mIterating.mData = aData;
     mData.mIterating.mOffset = aOffset;
     mData.mIterating.mLength = aLength;
+
+    // Update metrics.
+    mChunkCount++;
+    mByteCount += aLength;
+
     return mState = READY;
   }
 
   State SetWaiting()
   {
     MOZ_ASSERT(mState != COMPLETE);
     MOZ_ASSERT(mState != WAITING, "Did we get a spurious wakeup somehow?");
     return mState = WAITING;
@@ -187,16 +206,19 @@ private:
       const char* mData;
       size_t mOffset;
       size_t mLength;
     } mIterating;
     struct {
       nsresult mStatus;
     } mAtEnd;
   } mData;
+
+  uint32_t mChunkCount;  // Count of chunks we've advanced through.
+  size_t mByteCount;     // Count of bytes in all chunks we've advanced through.
 };
 
 /**
  * SourceBuffer is a parallel data structure used for storing image source
  * (compressed) data.
  *
  * SourceBuffer is a single producer, multiple consumer data structure. The
  * single producer calls Append() to append data to the buffer. In parallel,