Bug 1079653 (Part 1) - Move decode telemetry data from DecodeRequest to Decoder. r=tn
authorSeth Fowler <seth@mozilla.com>
Tue, 18 Nov 2014 12:06:26 -0800
changeset 216324 ce8cc2d64e042e5b24916391bc70ba58501602bb
parent 216323 33425fc431a530aaa3ef000144a528dfa0997d1e
child 216325 dfdcfd8f603844256f9470fd8ff467bff9f56f30
push id27845
push userkwierso@gmail.com
push dateWed, 19 Nov 2014 02:08:01 +0000
treeherdermozilla-central@64e7a6391916 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstn
bugs1079653
milestone36.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 1079653 (Part 1) - Move decode telemetry data from DecodeRequest to Decoder. r=tn
image/src/Decoder.cpp
image/src/Decoder.h
image/src/RasterImage.cpp
image/src/RasterImage.h
--- a/image/src/Decoder.cpp
+++ b/image/src/Decoder.cpp
@@ -15,16 +15,17 @@ namespace mozilla {
 namespace image {
 
 Decoder::Decoder(RasterImage &aImage)
   : mImage(aImage)
   , mCurrentFrame(nullptr)
   , mProgress(NoProgress)
   , mImageData(nullptr)
   , mColormap(nullptr)
+  , mChunkCount(0)
   , mDecodeFlags(0)
   , mBytesDecoded(0)
   , mDecodeDone(false)
   , mDataError(false)
   , mFrameCount(0)
   , mFailCode(NS_OK)
   , mNeedsNewFrame(false)
   , mInitialized(false)
@@ -95,20 +96,24 @@ Decoder::Write(const char* aBuffer, uint
     js::ProfileEntry::Category::GRAPHICS);
 
   MOZ_ASSERT(NS_IsMainThread() || aStrategy == DECODE_ASYNC);
 
   // We're strict about decoder errors
   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;
 
   if (IsSizeDecode() && HasSize()) {
     // More data came in since we found the size. We have nothing to do here.
     return;
   }
 
@@ -120,16 +125,19 @@ Decoder::Write(const char* aBuffer, uint
   while (aStrategy == DECODE_SYNC && NeedsNewFrame() && !HasDataError()) {
     nsresult rv = AllocateFrame();
 
     if (NS_SUCCEEDED(rv)) {
       // Tell the decoder to use the data it saved when it asked for a new frame.
       WriteInternal(nullptr, 0, aStrategy);
     }
   }
+
+  // Finish telemetry.
+  mDecodeTime += (TimeStamp::Now() - start);
 }
 
 void
 Decoder::Finish(RasterImage::eShutdownIntent aShutdownIntent)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   // Implementation-specific finalization
--- a/image/src/Decoder.h
+++ b/image/src/Decoder.h
@@ -106,16 +106,22 @@ public:
   void SetSizeDecode(bool aSizeDecode)
   {
     NS_ABORT_IF_FALSE(!mInitialized, "Can't set size decode after Init()!");
     mSizeDecode = aSizeDecode;
   }
 
   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; }
 
   // The number of complete frames we have (ie, not including anything in-progress).
   uint32_t GetCompleteFrameCount() { return mInFrame ? mFrameCount - 1 : mFrameCount; }
 
   // Error tracking
@@ -243,16 +249,20 @@ protected:
   nsIntRect mInvalidRect; // Tracks an invalidation region in the current frame.
   Progress mProgress;
 
   uint8_t* mImageData;       // Pointer to image data in either Cairo or 8bit format
   uint32_t mImageDataLength;
   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;
   bool mDataError;
 
 private:
   uint32_t mFrameCount; // Number of frames, including anything in-progress
 
--- a/image/src/RasterImage.cpp
+++ b/image/src/RasterImage.cpp
@@ -2921,18 +2921,19 @@ RasterImage::FinishedSomeDecoding(eShutd
   nsIntRect invalidRect;
   nsresult rv = NS_OK;
   Progress progress = aProgress;
 
   if (image->mDecoder) {
     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()) {
       image->mDecoder->SetSizeOnImage();
     }
 
     // If the decode finished, or we're specifically being told to shut down,
     // tell the image and shut down the decoder.
@@ -2942,24 +2943,24 @@ RasterImage::FinishedSomeDecoding(eShutd
       // Hold on to a reference to the decoder until we're done with it
       nsRefPtr<Decoder> decoder = image->mDecoder;
 
       wasSize = decoder->IsSizeDecode();
 
       // 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);
         }
       }
 
       // We need to shut down the decoder first, in order to ensure all
       // decoding routines have been finished.
       rv = image->ShutdownDecoder(aIntent);
       if (NS_FAILED(rv)) {
@@ -3379,54 +3380,47 @@ RasterImage::DecodePool::DecodeSomeOfIma
     // to read the size from most images.
     maxBytes = gfxPrefs::ImageMemDecodeBytesAtATime();
   }
 
   if (bytesToDecode == 0) {
     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,
   //  * the decode completes,
   //  * we're an UNTIL_SIZE decode and we get the size, or
   //  * we run out of time.
   // We also try to decode at least one "chunk" if we've allocated a new frame,
   // even if we have no more data to send to the decoder.
   while ((aImg->mSourceData.Length() > aImg->mDecoder->BytesDecoded() &&
           bytesToDecode > 0 &&
           !aImg->IsDecodeFinished() &&
           !(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)) {
       aImg->DoError();
       return rv;
     }
 
     bytesToDecode -= chunkSize;
 
     // Yield if we've been decoding for too long. We check this _after_ decoding
     // a chunk to ensure that we don't yield without doing any decoding.
     if (aDecodeType == DECODE_TYPE_UNTIL_TIME && TimeStamp::Now() >= deadline)
       break;
   }
 
-  if (aImg->mDecodeRequest) {
-    aImg->mDecodeRequest->mDecodeTime += (TimeStamp::Now() - start);
-    aImg->mDecodeRequest->mChunkCount += chunkCount;
-  }
-
   return NS_OK;
 }
 
 RasterImage::DecodeDoneWorker::DecodeDoneWorker(RasterImage* image, DecodeRequest* request)
  : mImage(image)
  , mRequest(request)
 {}
 
--- a/image/src/RasterImage.h
+++ b/image/src/RasterImage.h
@@ -316,17 +316,16 @@ private:
    * DecodeRequests.
    */
   struct DecodeRequest
   {
     explicit DecodeRequest(RasterImage* aImage)
       : mImage(aImage)
       , mBytesToDecode(0)
       , mRequestStatus(REQUEST_INACTIVE)
-      , mChunkCount(0)
       , mAllocatedNewFrame(false)
     { }
 
     NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DecodeRequest)
 
     RasterImage* mImage;
 
     size_t mBytesToDecode;
@@ -335,23 +334,16 @@ private:
     {
       REQUEST_INACTIVE,
       REQUEST_PENDING,
       REQUEST_ACTIVE,
       REQUEST_WORK_DONE,
       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;
 
   private:
     ~DecodeRequest() {}
   };