Bug 1291071 (Part 3) - Pass telemetry explicitly to FinalizeDecoder. r=edwin
authorSeth Fowler <mark.seth.fowler@gmail.com>
Tue, 02 Aug 2016 16:34:23 -0700
changeset 349756 9b9a6dca288397b794f31e52c8a21b1262c72d47
parent 349755 801b6b88b490705e4905b2b83346515e2461c5b7
child 349757 eef2029cae9d33943011306ac593906492368d2b
push id6570
push userraliiev@mozilla.com
push dateMon, 14 Nov 2016 12:26:13 +0000
treeherdermozilla-beta@f455459b2ae5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersedwin
bugs1291071
milestone51.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 1291071 (Part 3) - Pass telemetry explicitly to FinalizeDecoder. r=edwin
image/Decoder.cpp
image/Decoder.h
image/IDecodingTask.cpp
image/RasterImage.cpp
image/RasterImage.h
--- a/image/Decoder.cpp
+++ b/image/Decoder.cpp
@@ -262,16 +262,26 @@ Decoder::ExplicitOutputSize() const
 Maybe<uint32_t>
 Decoder::TakeCompleteFrameCount()
 {
   const bool finishedNewFrame = mFinishedNewFrame;
   mFinishedNewFrame = false;
   return finishedNewFrame ? Some(GetCompleteFrameCount()) : Nothing();
 }
 
+DecoderTelemetry
+Decoder::Telemetry()
+{
+  MOZ_ASSERT(mIterator);
+  return DecoderTelemetry(SpeedHistogram(),
+                          mIterator->ByteCount(),
+                          mIterator->ChunkCount(),
+                          mDecodeTime);
+}
+
 nsresult
 Decoder::AllocateFrame(uint32_t aFrameNum,
                        const gfx::IntSize& aOutputSize,
                        const gfx::IntRect& aFrameRect,
                        gfx::SurfaceFormat aFormat,
                        uint8_t aPaletteDepth)
 {
   mCurrentFrame = AllocateFrameInternal(aFrameNum, aOutputSize, aFrameRect,
--- a/image/Decoder.h
+++ b/image/Decoder.h
@@ -23,16 +23,50 @@
 namespace mozilla {
 
 namespace Telemetry {
   enum ID : uint32_t;
 } // namespace Telemetry
 
 namespace image {
 
+struct DecoderTelemetry final
+{
+  DecoderTelemetry(Telemetry::ID aSpeedHistogram,
+                   size_t aBytesDecoded,
+                   uint32_t aChunkCount,
+                   TimeDuration aDecodeTime)
+    : mSpeedHistogram(aSpeedHistogram)
+    , mBytesDecoded(aBytesDecoded)
+    , mChunkCount(aChunkCount)
+    , mDecodeTime(aDecodeTime)
+  { }
+
+  /// @return our decoder's speed, in KBps.
+  int32_t Speed() const
+  {
+    return mBytesDecoded / (1024 * mDecodeTime.ToSeconds());
+  }
+
+  /// @return our decoder's decode time, in microseconds.
+  int32_t DecodeTimeMicros() { return mDecodeTime.ToMicroseconds(); }
+
+  /// The per-image-format telemetry ID for recording our decoder's speed.
+  const Telemetry::ID mSpeedHistogram;
+
+  /// The number of bytes of input our decoder processed.
+  const size_t mBytesDecoded;
+
+  /// The number of chunks our decoder's input was divided into.
+  const uint32_t mChunkCount;
+
+  /// The amount of time our decoder spent inside DoDecode().
+  const TimeDuration mDecodeTime;
+};
+
 class Decoder
 {
 public:
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(Decoder)
 
   explicit Decoder(RasterImage* aImage);
 
   /**
@@ -186,32 +220,16 @@ public:
   /**
    * Should we stop decoding after the first frame?
    */
   bool IsFirstFrameDecode() const
   {
     return bool(mDecoderFlags & DecoderFlags::FIRST_FRAME_ONLY);
   }
 
-  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
-  {
-    MOZ_ASSERT(mIterator);
-    return mIterator->ChunkCount();
-  }
-
   /**
    * @return the number of complete animation frames which have been decoded so
    * far, if it has changed since the last call to TakeCompleteFrameCount();
    * otherwise, returns Nothing().
    */
   Maybe<uint32_t> TakeCompleteFrameCount();
 
   // The number of frames we have, including anything in-progress. Thus, this
@@ -318,16 +336,19 @@ public:
     return gfx::IntRect(gfx::IntPoint(), OutputSize());
   }
 
   virtual Telemetry::ID SpeedHistogram();
 
   /// @return the metadata we collected about this image while decoding.
   const ImageMetadata& GetImageMetadata() { return mImageMetadata; }
 
+  /// @return performance telemetry we collected while decoding.
+  DecoderTelemetry Telemetry();
+
   /**
    * @return a weak pointer to the image associated with this decoder. Illegal
    * to call if this decoder is not associated with an image.
    */
   NotNull<RasterImage*> GetImage() const { return WrapNotNull(mImage.get()); }
 
   /**
    * @return a possibly-null weak pointer to the image associated with this
--- a/image/IDecodingTask.cpp
+++ b/image/IDecodingTask.cpp
@@ -60,35 +60,36 @@ IDecodingTask::NotifyProgress(NotNull<Ra
 IDecodingTask::NotifyDecodeComplete(NotNull<RasterImage*> aImage,
                                     NotNull<Decoder*> aDecoder)
 {
   MOZ_ASSERT(aDecoder->HasError() || !aDecoder->InFrame(),
              "Decode complete in the middle of a frame?");
 
   // Capture the decoder's state.
   ImageMetadata metadata = aDecoder->GetImageMetadata();
+  DecoderTelemetry telemetry = aDecoder->Telemetry();
   Progress progress = aDecoder->TakeProgress();
   IntRect invalidRect = aDecoder->TakeInvalidRect();
   Maybe<uint32_t> frameCount = aDecoder->TakeCompleteFrameCount();
   SurfaceFlags surfaceFlags = aDecoder->GetSurfaceFlags();
 
   // Synchronously notify if we can.
   if (NS_IsMainThread() &&
       !(aDecoder->GetDecoderFlags() & DecoderFlags::ASYNC_NOTIFY)) {
-    aImage->FinalizeDecoder(aDecoder, metadata, progress, invalidRect,
-                            frameCount, surfaceFlags);
+    aImage->FinalizeDecoder(aDecoder, metadata, telemetry, progress,
+                            invalidRect, frameCount, surfaceFlags);
     return;
   }
 
   // We're forced to notify asynchronously.
   NotNull<RefPtr<RasterImage>> image = aImage;
   NotNull<RefPtr<Decoder>> decoder = aDecoder;
   NS_DispatchToMainThread(NS_NewRunnableFunction([=]() -> void {
-    image->FinalizeDecoder(decoder.get(), metadata, progress, invalidRect,
-                           frameCount, surfaceFlags);
+    image->FinalizeDecoder(decoder.get(), metadata, telemetry, progress,
+                           invalidRect, frameCount, surfaceFlags);
   }));
 }
 
 
 ///////////////////////////////////////////////////////////////////////////////
 // IDecodingTask implementation.
 ///////////////////////////////////////////////////////////////////////////////
 
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -1559,16 +1559,17 @@ RasterImage::NotifyProgress(Progress aPr
 
   // Tell the observers what happened.
   image->mProgressTracker->SyncNotifyProgress(aProgress, aInvalidRect);
 }
 
 void
 RasterImage::FinalizeDecoder(Decoder* aDecoder,
                              const ImageMetadata& aMetadata,
+                             const DecoderTelemetry& aTelemetry,
                              Progress aProgress,
                              const IntRect& aInvalidRect,
                              const Maybe<uint32_t>& aFrameCount,
                              SurfaceFlags aSurfaceFlags)
 {
   MOZ_ASSERT(NS_IsMainThread());
   MOZ_ASSERT(aDecoder);
 
@@ -1603,34 +1604,31 @@ RasterImage::FinalizeDecoder(Decoder* aD
   NotifyProgress(aProgress, aInvalidRect, aFrameCount, aSurfaceFlags);
 
   if (mHasBeenDecoded && mAnimationState) {
     // We're done decoding and our AnimationState has been notified about all
     // our frames, so let it know not to expect anymore.
     mAnimationState->SetDoneDecoding(true);
   }
 
-  if (!wasMetadata && aDecoder->ChunkCount()) {
-    Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS,
-                          aDecoder->ChunkCount());
+  if (!wasMetadata && aTelemetry.mChunkCount) {
+    Telemetry::Accumulate(Telemetry::IMAGE_DECODE_CHUNKS, aTelemetry.mChunkCount);
   }
 
   if (done) {
     // Do some telemetry if this isn't a metadata decode.
     if (!wasMetadata) {
       Telemetry::Accumulate(Telemetry::IMAGE_DECODE_TIME,
-                            int32_t(aDecoder->DecodeTime().ToMicroseconds()));
+                            int32_t(aTelemetry.mDecodeTime.ToMicroseconds()));
 
       // We record the speed for only some decoders. The rest have
       // SpeedHistogram return HistogramCount.
-      Telemetry::ID id = aDecoder->SpeedHistogram();
+      Telemetry::ID id = aTelemetry.mSpeedHistogram;
       if (id < Telemetry::HistogramCount) {
-        int32_t KBps = int32_t(aDecoder->BytesDecoded() /
-                               (1024 * aDecoder->DecodeTime().ToSeconds()));
-        Telemetry::Accumulate(id, KBps);
+        Telemetry::Accumulate(id, aTelemetry.Speed());
       }
     }
 
     // Detect errors.
     if (aDecoder->HasError() && !aDecoder->WasAborted()) {
       DoError();
     } else if (wasMetadata && !mHasSize) {
       DoError();
--- a/image/RasterImage.h
+++ b/image/RasterImage.h
@@ -130,16 +130,17 @@ namespace mozilla {
 namespace layers {
 class ImageContainer;
 class Image;
 } // namespace layers
 
 namespace image {
 
 class Decoder;
+struct DecoderTelemetry;
 class ImageMetadata;
 class SourceBuffer;
 
 class RasterImage final : public ImageResource
                         , public nsIProperties
                         , public SupportsWeakPtr<RasterImage>
 #ifdef DEBUG
                         , public imgIContainerDebug
@@ -198,16 +199,17 @@ public:
 
   /**
    * Records telemetry and does final teardown of the provided decoder.
    *
    * Main-thread only.
    */
   void FinalizeDecoder(Decoder* aDecoder,
                        const ImageMetadata& aMetadata,
+                       const DecoderTelemetry& aTelemetry,
                        Progress aProgress,
                        const gfx::IntRect& aInvalidRect,
                        const Maybe<uint32_t>& aFrameCount,
                        SurfaceFlags aSurfaceFlags);
 
   // Helper method for FinalizeDecoder.
   void ReportDecoderError(Decoder* aDecoder);