Bug 1079653 (Part 1) - Move decode telemetry data from DecodeRequest to Decoder. r=tn

image/src/Decoder.cpp

@@ -20,6 +20,7 @@ Decoder::Decoder(RasterImage &aImage)
   , mProgress(NoProgress)
   , mImageData(nullptr)
   , mColormap(nullptr)
+  , mChunkCount(0)
   , mDecodeFlags(0)
   , mBytesDecoded(0)
   , mDecodeDone(false)
@@ -100,10 +101,14 @@ Decoder::Write(const char* aBuffer, uint32_t aCount, DecodeStrategy aStrategy)
   MOZ_ASSERT(!HasDecoderError(),
              "Not allowed to make more decoder calls after error!");
 
+  // Begin recording telemetry data.
+  TimeStamp start = TimeStamp::Now();
+  mChunkCount++;
+
   // Keep track of the total number of bytes written.
   mBytesDecoded += aCount;
 
-  // If a data error occured, just ignore future data
+  // If a data error occured, just ignore future data.
   if (HasDataError())
     return;
 
@@ -125,6 +130,9 @@ Decoder::Write(const char* aBuffer, uint32_t aCount, DecodeStrategy aStrategy)
       WriteInternal(nullptr, 0, aStrategy);
     }
   }
+
+  // Finish telemetry.
+  mDecodeTime += (TimeStamp::Now() - start);
 }
 
 void

image/src/Decoder.h

@@ -111,6 +111,12 @@ public:
 
   size_t BytesDecoded() const { return mBytesDecoded; }
 
+  // The amount of time we've spent inside Write() so far for this decoder.
+  TimeDuration DecodeTime() const { return mDecodeTime; }
+
+  // The number of times Write() has been called so far for this decoder.
+  uint32_t ChunkCount() const { return mChunkCount; }
+
   // 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; }
@@ -248,6 +254,10 @@ protected:
   uint32_t* mColormap;       // Current colormap to be used in Cairo format
   uint32_t mColormapSize;
 
+  // Telemetry data for this decoder.
+  TimeDuration mDecodeTime;
+  uint32_t mChunkCount;
+
   uint32_t mDecodeFlags;
   size_t mBytesDecoded;
   bool mDecodeDone;

image/src/RasterImage.cpp

@@ -2926,8 +2926,9 @@ RasterImage::FinishedSomeDecoding(eShutdownIntent aIntent /* = eShutdownIntent_D
     invalidRect = image->mDecoder->TakeInvalidRect();
     progress |= image->mDecoder->TakeProgress();
 
-    if (request && request->mChunkCount && !image->mDecoder->IsSizeDecode()) {
-      Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS, request->mChunkCount);
+    if (!image->mDecoder->IsSizeDecode() && image->mDecoder->ChunkCount()) {
+      Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS,
+                            image->mDecoder->ChunkCount());
     }
 
     if (!image->mHasSize && image->mDecoder->HasSize()) {
@@ -2947,14 +2948,14 @@ RasterImage::FinishedSomeDecoding(eShutdownIntent aIntent /* = eShutdownIntent_D
       // Do some telemetry if this isn't a size decode.
       if (request && !wasSize) {
         Telemetry::Accumulate(Telemetry::IMAGE_DECODE_TIME,
-                              int32_t(request->mDecodeTime.ToMicroseconds()));
+                              int32_t(decoder->DecodeTime().ToMicroseconds()));
 
         // We record the speed for only some decoders. The rest have
         // SpeedHistogram return HistogramCount.
         Telemetry::ID id = decoder->SpeedHistogram();
         if (id < Telemetry::HistogramCount) {
           int32_t KBps = int32_t(decoder->BytesDecoded() /
-                                 (1024 * request->mDecodeTime.ToSeconds()));
+                                 (1024 * decoder->DecodeTime().ToSeconds()));
           Telemetry::Accumulate(id, KBps);
         }
       }
@@ -3384,9 +3385,8 @@ RasterImage::DecodePool::DecodeSomeOfImage(RasterImage* aImg,
     bytesToDecode = aImg->mSourceData.Length() - aImg->mDecoder->BytesDecoded();
   }
 
-  int32_t chunkCount = 0;
-  TimeStamp start = TimeStamp::Now();
-  TimeStamp deadline = start + TimeDuration::FromMilliseconds(gfxPrefs::ImageMemMaxMSBeforeYield());
+  TimeStamp deadline = TimeStamp::Now() +
+                       TimeDuration::FromMilliseconds(gfxPrefs::ImageMemMaxMSBeforeYield());
 
   // We keep decoding chunks until:
   //  * we don't have any data left to decode,
@@ -3401,7 +3401,6 @@ RasterImage::DecodePool::DecodeSomeOfImage(RasterImage* aImg,
           !(aDecodeType == DECODE_TYPE_UNTIL_SIZE && aImg->mHasSize) &&
           !aImg->mDecoder->NeedsNewFrame()) ||
          (aImg->mDecodeRequest && aImg->mDecodeRequest->mAllocatedNewFrame)) {
-    chunkCount++;
     uint32_t chunkSize = std::min(bytesToDecode, maxBytes);
     nsresult rv = aImg->DecodeSomeData(chunkSize, aStrategy);
     if (NS_FAILED(rv)) {
@@ -3417,11 +3416,6 @@ RasterImage::DecodePool::DecodeSomeOfImage(RasterImage* aImg,
       break;
   }
 
-  if (aImg->mDecodeRequest) {
-    aImg->mDecodeRequest->mDecodeTime += (TimeStamp::Now() - start);
-    aImg->mDecodeRequest->mChunkCount += chunkCount;
-  }
-
   return NS_OK;
 }
 

image/src/RasterImage.h

@@ -321,7 +321,6 @@ private:
       : mImage(aImage)
       , mBytesToDecode(0)
       , mRequestStatus(REQUEST_INACTIVE)
-      , mChunkCount(0)
       , mAllocatedNewFrame(false)
     { }
 
@@ -340,13 +339,6 @@ private:
       REQUEST_STOPPED
     } mRequestStatus;
 
-    /* Keeps track of how much time we've burned decoding this particular decode
-     * request. */
-    TimeDuration mDecodeTime;
-
-    /* The number of chunks it took to decode this image. */
-    int32_t mChunkCount;
-
     /* True if a new frame has been allocated, but DecodeSomeData hasn't yet
      * been called to flush data to it */
     bool mAllocatedNewFrame;