Bug 1465619 - Part 6. Add support for recycling to imgFrame. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Sun, 03 Jun 2018 19:42:09 -0400
changeset 442376 7a2954981481f2f9f7bae58709dd50f67dab7cba
parent 442375 ddaa812c31e4589b269dc622c6f6a1bd049f81ec
child 442377 6ef29bb729309b6c5fefa615a72cdcb897c7c473
push id34906
push userdluca@mozilla.com
push dateTue, 23 Oct 2018 04:49:21 +0000
treeherdermozilla-central@77f4c84bebf0 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1465619
milestone65.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 1465619 - Part 6. Add support for recycling to imgFrame. r=tnikkel Beyond the necessary reinitialization methods, we need to protect ourselves from recycling a frame that some other entity in the browser is still using. Generally speaking the animated surface will only be used in imgFrame::Draw since we don't layerize animated images, which will be safe. However with OMTP or blob recordings, we could retain a reference to the surface outside the current stack context. Additional if something calls RasterImage::GetImageContainer(AtSize) or RasterImage::GetFrame(AtSize), it may also have a reference to the surface for an indetermine period of time. As such, if an imgFrame is a candidate for recycling, it will wrap imgFrame::mLockedSurface in a RecyclingSourceSurface. Its job is to track how many consumers there are still of the surface, so that after we advance the animation, the decoder will know if there are still outstanding consumers. If the surface is still in use, it will block for a finite period of time (the refresh interval) before giving up on reclaiming the surface, and will allocate a new surface. The old surface can then remain in circulation for as long as necessary without further blocking the animation progression, since we stop recycling that surface/imgFrame. Differential Revision: https://phabricator.services.mozilla.com/D7511
image/RecyclingSourceSurface.h
image/imgFrame.cpp
image/imgFrame.h
image/moz.build
new file mode 100644
--- /dev/null
+++ b/image/RecyclingSourceSurface.h
@@ -0,0 +1,66 @@
+/* -*- Mode: C++; tab-width: 2; indent-tabs-mode: nil; c-basic-offset: 2 -*- */
+/* vim: set ts=2 et sw=2 tw=80: */
+/* This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+#ifndef mozilla_image_RecyclingSourceSurface_h
+#define mozilla_image_RecyclingSourceSurface_h
+
+#include "mozilla/gfx/2D.h"
+
+namespace mozilla {
+namespace image {
+
+class imgFrame;
+
+/**
+ * This surface subclass will prevent the underlying surface from being recycled
+ * as long as it is still alive. We will create this surface to wrap imgFrame's
+ * mLockedSurface, if we are accessing it on a path that will keep the surface
+ * alive for an indeterminate period of time (e.g. imgFrame::GetSourceSurface,
+ * imgFrame::Draw with a recording or capture DrawTarget).
+ */
+class RecyclingSourceSurface final : public gfx::DataSourceSurface
+{
+public:
+  RecyclingSourceSurface(imgFrame* aParent, gfx::DataSourceSurface* aSurface);
+
+  MOZ_DECLARE_REFCOUNTED_VIRTUAL_TYPENAME(RecyclingSourceSurface, override);
+
+  uint8_t* GetData() override { return mSurface->GetData(); }
+  int32_t Stride() override { return mSurface->Stride(); }
+  gfx::SurfaceType GetType() const override { return mType; }
+  gfx::IntSize GetSize() const override { return mSurface->GetSize(); }
+  gfx::SurfaceFormat GetFormat() const override { return mSurface->GetFormat(); }
+
+  void AddSizeOfExcludingThis(MallocSizeOf aMallocSizeOf,
+                              size_t& aHeapSizeOut,
+                              size_t& aNonHeapSizeOut,
+                              size_t& aExtHandlesOut,
+                              uint64_t& aExtIdOut) const override
+  { }
+
+  bool OnHeap() const override { return mSurface->OnHeap(); }
+  bool Map(MapType aType, MappedSurface* aMappedSurface) override
+  {
+    return mSurface->Map(aType, aMappedSurface);
+  }
+  void Unmap() override { mSurface->Unmap(); }
+
+  gfx::DataSourceSurface* GetChildSurface() const { return mSurface; }
+
+protected:
+  void GuaranteePersistance() override { }
+
+  ~RecyclingSourceSurface() override;
+
+  RefPtr<imgFrame> mParent;
+  RefPtr<DataSourceSurface> mSurface;
+  gfx::SurfaceType mType;
+};
+
+} // namespace image
+} // namespace mozilla
+
+#endif //mozilla_image_RecyclingSourceSurface_h
--- a/image/imgFrame.cpp
+++ b/image/imgFrame.cpp
@@ -17,21 +17,23 @@
 #include "gfxUtils.h"
 
 #include "GeckoProfiler.h"
 #include "MainThreadUtils.h"
 #include "mozilla/CheckedInt.h"
 #include "mozilla/gfx/gfxVars.h"
 #include "mozilla/gfx/Tools.h"
 #include "mozilla/gfx/SourceSurfaceRawData.h"
+#include "mozilla/image/RecyclingSourceSurface.h"
 #include "mozilla/layers/SourceSurfaceSharedData.h"
 #include "mozilla/layers/SourceSurfaceVolatileData.h"
 #include "mozilla/Likely.h"
 #include "mozilla/MemoryReporting.h"
 #include "nsMargin.h"
+#include "nsRefreshDriver.h"
 #include "nsThreadUtils.h"
 
 namespace mozilla {
 
 using namespace gfx;
 
 namespace image {
 
@@ -173,19 +175,21 @@ static bool AllowedImageAndFrameDimensio
   }
   return true;
 }
 
 imgFrame::imgFrame()
   : mMonitor("imgFrame")
   , mDecoded(0, 0, 0, 0)
   , mLockCount(0)
+  , mRecycleLockCount(0)
   , mAborted(false)
   , mFinished(false)
   , mOptimizable(false)
+  , mShouldRecycle(false)
   , mTimeout(FrameTimeout::FromRawMilliseconds(100))
   , mDisposalMethod(DisposalMethod::NOT_SPECIFIED)
   , mBlendMethod(BlendMethod::OVER)
   , mFormat(SurfaceFormat::UNKNOWN)
   , mPalettedImageData(nullptr)
   , mPaletteDepth(0)
   , mNonPremult(false)
   , mIsFullFrame(false)
@@ -204,20 +208,21 @@ imgFrame::~imgFrame()
   free(mPalettedImageData);
   mPalettedImageData = nullptr;
 }
 
 nsresult
 imgFrame::InitForDecoder(const nsIntSize& aImageSize,
                          const nsIntRect& aRect,
                          SurfaceFormat aFormat,
-                         uint8_t aPaletteDepth /* = 0 */,
-                         bool aNonPremult /* = false */,
-                         const Maybe<AnimationParams>& aAnimParams /* = Nothing() */,
-                         bool aIsFullFrame /* = false */)
+                         uint8_t aPaletteDepth,
+                         bool aNonPremult,
+                         const Maybe<AnimationParams>& aAnimParams,
+                         bool aIsFullFrame,
+                         bool aShouldRecycle)
 {
   // Assert for properties that should be verified by decoders,
   // warn for properties related to bad content.
   if (!AllowedImageAndFrameDimensions(aImageSize, aRect)) {
     NS_WARNING("Should have legal image size");
     mAborted = true;
     return NS_ERROR_FAILURE;
   }
@@ -249,19 +254,31 @@ imgFrame::InitForDecoder(const nsIntSize
   // frames, never has to deal with a non-trivial frame rect.
   if (aPaletteDepth == 0 &&
       !mFrameRect.IsEqualEdges(IntRect(IntPoint(), mImageSize))) {
     MOZ_ASSERT_UNREACHABLE("Creating a non-paletted imgFrame with a "
                            "non-trivial frame rect");
     return NS_ERROR_FAILURE;
   }
 
-  mFormat = aFormat;
+  if (aShouldRecycle) {
+    // If we are recycling then we should always use BGRA for the underlying
+    // surface because if we use BGRX, the next frame composited into the
+    // surface could be BGRA and cause rendering problems.
+    MOZ_ASSERT(mIsFullFrame);
+    MOZ_ASSERT(aPaletteDepth == 0);
+    MOZ_ASSERT(aAnimParams);
+    mFormat = SurfaceFormat::B8G8R8A8;
+  } else {
+    mFormat = aFormat;
+  }
+
   mPaletteDepth = aPaletteDepth;
   mNonPremult = aNonPremult;
+  mShouldRecycle = aShouldRecycle;
 
   if (aPaletteDepth != 0) {
     // We're creating for a paletted image.
     if (aPaletteDepth > 8) {
       NS_WARNING("Should have legal palette depth");
       NS_ERROR("This Depth is not supported");
       mAborted = true;
       return NS_ERROR_FAILURE;
@@ -299,16 +316,79 @@ imgFrame::InitForDecoder(const nsIntSize
       return NS_ERROR_OUT_OF_MEMORY;
     }
   }
 
   return NS_OK;
 }
 
 nsresult
+imgFrame::InitForDecoderRecycle(const AnimationParams& aAnimParams)
+{
+  // We want to recycle this frame, but there is no guarantee that consumers are
+  // done with it in a timely manner. Let's ensure they are done with it first.
+  MonitorAutoLock lock(mMonitor);
+
+  MOZ_ASSERT(mIsFullFrame);
+  MOZ_ASSERT(mLockCount > 0);
+  MOZ_ASSERT(mLockedSurface);
+  MOZ_ASSERT(mShouldRecycle);
+
+  if (mRecycleLockCount > 0) {
+    if (NS_IsMainThread()) {
+      // We should never be both decoding and recycling on the main thread. Sync
+      // decoding can only be used to produce the first set of frames. Those
+      // either never use recycling because advancing was blocked (main thread
+      // is busy) or we were auto-advancing (to seek to a frame) and the frames
+      // were never accessed (and thus cannot have recycle locks).
+      MOZ_ASSERT_UNREACHABLE("Recycling/decoding on the main thread?");
+      return NS_ERROR_NOT_AVAILABLE;
+    }
+
+    // We don't want to wait forever to reclaim the frame because we have no
+    // idea why it is still held. It is possibly due to OMTP. Since we are off
+    // the main thread, and we generally have frames already buffered for the
+    // animation, we can afford to wait a short period of time to hopefully
+    // complete the transaction and reclaim the buffer.
+    //
+    // We choose to wait for, at most, the refresh driver interval, so that we
+    // won't skip more than one frame. If the frame is still in use due to
+    // outstanding transactions, we are already skipping frames. If the frame
+    // is still in use for some other purpose, it won't be returned to the pool
+    // and its owner can hold onto it forever without additional impact here.
+    TimeDuration timeout =
+      TimeDuration::FromMilliseconds(nsRefreshDriver::DefaultInterval());
+    while (true) {
+      TimeStamp start = TimeStamp::Now();
+      mMonitor.Wait(timeout);
+      if (mRecycleLockCount == 0) {
+        break;
+      }
+
+      TimeDuration delta = TimeStamp::Now() - start;
+      if (delta >= timeout) {
+        // We couldn't secure the frame for recycling. It will allocate a new
+        // frame instead.
+        return NS_ERROR_NOT_AVAILABLE;
+      }
+
+      timeout -= delta;
+    }
+  }
+
+  mBlendRect = aAnimParams.mBlendRect;
+  mTimeout = aAnimParams.mTimeout;
+  mBlendMethod = aAnimParams.mBlendMethod;
+  mDisposalMethod = aAnimParams.mDisposalMethod;
+  mDirtyRect = mFrameRect;
+
+  return NS_OK;
+}
+
+nsresult
 imgFrame::InitWithDrawable(gfxDrawable* aDrawable,
                            const nsIntSize& aSize,
                            const SurfaceFormat aFormat,
                            SamplingFilter aSamplingFilter,
                            uint32_t aImageFlags,
                            gfx::BackendType aBackend)
 {
   // Assert for properties that should be verified by decoders,
@@ -558,32 +638,42 @@ bool imgFrame::Draw(gfxContext* aContext
              "Directly drawing an image with a non-trivial frame rect!");
 
   if (mPalettedImageData) {
     MOZ_ASSERT_UNREACHABLE("Directly drawing a paletted image!");
     return false;
   }
 
   // Perform the draw and freeing of the surface outside the lock. We want to
-  // avoid contention with the decoder if we can.
+  // avoid contention with the decoder if we can. The surface may also attempt
+  // to relock the monitor if it is freed (e.g. RecyclingSourceSurface).
   RefPtr<SourceSurface> surf;
   SurfaceWithFormat surfaceResult;
   ImageRegion region(aRegion);
   gfxRect imageRect(0, 0, mImageSize.width, mImageSize.height);
 
   {
     MonitorAutoLock lock(mMonitor);
 
     // Possibly convert this image into a GPU texture, this may also cause our
     // mLockedSurface to be released and the OS to release the underlying memory.
     Optimize(aContext->GetDrawTarget());
 
     bool doPartialDecode = !AreAllPixelsWritten();
 
-    surf = GetSourceSurfaceInternal();
+    // Most draw targets will just use the surface only during DrawPixelSnapped
+    // but captures/recordings will retain a reference outside this stack
+    // context. While in theory a decoder thread could be trying to recycle this
+    // frame at this very moment, in practice the only way we can get here is if
+    // this frame is the current frame of the animation. Since we can only
+    // advance on the main thread, we know nothing else will try to use it.
+    DrawTarget* drawTarget = aContext->GetDrawTarget();
+    bool temporary = !drawTarget->IsCaptureDT() &&
+                    drawTarget->GetBackendType() != BackendType::RECORDING;
+    RefPtr<SourceSurface> surf = GetSourceSurfaceInternal(temporary);
     if (!surf) {
       return false;
     }
 
     bool doTile = !imageRect.Contains(aRegion.Rect()) &&
                   !(aImageFlags & imgIContainer::FLAG_CLAMP);
 
     surfaceResult =
@@ -852,38 +942,48 @@ imgFrame::FinalizeSurfaceInternal()
   auto sharedSurf = static_cast<SourceSurfaceSharedData*>(mRawSurface.get());
   sharedSurf->Finalize();
 }
 
 already_AddRefed<SourceSurface>
 imgFrame::GetSourceSurface()
 {
   MonitorAutoLock lock(mMonitor);
-  return GetSourceSurfaceInternal();
+  return GetSourceSurfaceInternal(/* aTemporary */ false);
 }
 
 already_AddRefed<SourceSurface>
-imgFrame::GetSourceSurfaceInternal()
+imgFrame::GetSourceSurfaceInternal(bool aTemporary)
 {
   mMonitor.AssertCurrentThreadOwns();
 
   if (mOptSurface) {
     if (mOptSurface->IsValid()) {
       RefPtr<SourceSurface> surf(mOptSurface);
       return surf.forget();
     } else {
       mOptSurface = nullptr;
     }
   }
 
   if (mLockedSurface) {
+    // We don't need to create recycling wrapper for some callers because they
+    // promise to release the surface immediately after.
+    if (!aTemporary && mShouldRecycle) {
+      RefPtr<SourceSurface> surf =
+        new RecyclingSourceSurface(this, mLockedSurface);
+      return surf.forget();
+    }
+
     RefPtr<SourceSurface> surf(mLockedSurface);
     return surf.forget();
   }
 
+  MOZ_ASSERT(!mShouldRecycle, "Should recycle but no locked surface!");
+
   if (!mRawSurface) {
     return nullptr;
   }
 
   return CreateLockedSurface(mRawSurface, mFrameRect.Size(), mFormat);
 }
 
 void
@@ -968,10 +1068,28 @@ imgFrame::AddSizeOfExcludingThis(MallocS
     mRawSurface->AddSizeOfExcludingThis(aMallocSizeOf, metadata.heap,
                                         metadata.nonHeap, metadata.handles,
                                         metadata.externalId);
   }
 
   aCallback(metadata);
 }
 
+RecyclingSourceSurface::RecyclingSourceSurface(imgFrame* aParent, DataSourceSurface* aSurface)
+  : mParent(aParent)
+  , mSurface(aSurface)
+  , mType(SurfaceType::DATA)
+{
+  mParent->mMonitor.AssertCurrentThreadOwns();
+  ++mParent->mRecycleLockCount;
+}
+
+RecyclingSourceSurface::~RecyclingSourceSurface()
+{
+  MonitorAutoLock lock(mParent->mMonitor);
+  MOZ_ASSERT(mParent->mRecycleLockCount > 0);
+  if (--mParent->mRecycleLockCount == 0) {
+    mParent->mMonitor.NotifyAll();
+  }
+}
+
 } // namespace image
 } // namespace mozilla
--- a/image/imgFrame.h
+++ b/image/imgFrame.h
@@ -52,36 +52,46 @@ public:
    *
    * This is appropriate for use with decoded images, but it should not be used
    * when drawing content into an imgFrame, as it may use a different graphics
    * backend than normal content drawing.
    */
   nsresult InitForDecoder(const nsIntSize& aImageSize,
                           const nsIntRect& aRect,
                           SurfaceFormat aFormat,
-                          uint8_t aPaletteDepth = 0,
-                          bool aNonPremult = false,
-                          const Maybe<AnimationParams>& aAnimParams = Nothing(),
-                          bool aIsFullFrame = false);
+                          uint8_t aPaletteDepth,
+                          bool aNonPremult,
+                          const Maybe<AnimationParams>& aAnimParams,
+                          bool aIsFullFrame,
+                          bool aShouldRecycle);
 
   nsresult InitForAnimator(const nsIntSize& aSize,
                            SurfaceFormat aFormat)
   {
     nsIntRect frameRect(0, 0, aSize.width, aSize.height);
     AnimationParams animParams { frameRect, FrameTimeout::Forever(),
                                  /* aFrameNum */ 1, BlendMethod::OVER,
                                  DisposalMethod::NOT_SPECIFIED };
     // We set aIsFullFrame to false because we don't want the compositing frame
     // to be allocated into shared memory for WebRender. mIsFullFrame is only
     // otherwise used for frames produced by Decoder, so it isn't relevant.
     return InitForDecoder(aSize, frameRect, aFormat, /* aPaletteDepth */ 0,
                           /* aNonPremult */ false, Some(animParams),
-                          /* aIsFullFrame */ false);
+                          /* aIsFullFrame */ false, /* aShouldRecycle */ false);
   }
 
+  /**
+   * Reinitialize this imgFrame with the new parameters, but otherwise retain
+   * the underlying buffer.
+   *
+   * This is appropriate for use with animated images, where the decoder was
+   * given an IDecoderFrameRecycler object which may yield a recycled imgFrame
+   * that was discarded to save memory.
+   */
+  nsresult InitForDecoderRecycle(const AnimationParams& aAnimParams);
 
   /**
    * Initialize this imgFrame with a new surface and draw the provided
    * gfxDrawable into it.
    *
    * This is appropriate to use when drawing content into an imgFrame, as it
    * uses the same graphics backend as normal content drawing. The downside is
    * that the underlying surface may not be stored in a volatile buffer on all
@@ -196,16 +206,18 @@ public:
   const IntRect& GetDirtyRect() const { return mDirtyRect; }
   void SetDirtyRect(const IntRect& aDirtyRect) { mDirtyRect = aDirtyRect; }
 
   bool IsFullFrame() const { return mIsFullFrame; }
 
   bool GetCompositingFailed() const;
   void SetCompositingFailed(bool val);
 
+  bool ShouldRecycle() const { return mShouldRecycle; }
+
   void SetOptimizable();
 
   void FinalizeSurface();
   already_AddRefed<SourceSurface> GetSourceSurface();
 
   struct AddSizeOfCbData {
     AddSizeOfCbData()
       : heap(0), nonHeap(0), handles(0), index(0), externalId(0)
@@ -242,17 +254,24 @@ private: // methods
   void AssertImageDataLocked() const;
 
   bool AreAllPixelsWritten() const;
   nsresult ImageUpdatedInternal(const nsIntRect& aUpdateRect);
   void GetImageDataInternal(uint8_t** aData, uint32_t* length) const;
   uint32_t GetImageBytesPerRow() const;
   uint32_t GetImageDataLength() const;
   void FinalizeSurfaceInternal();
-  already_AddRefed<SourceSurface> GetSourceSurfaceInternal();
+
+  /**
+   * @param aTemporary  If true, it will assume the caller does not require a
+   *                    wrapping RecycleSourceSurface to protect the underlying
+   *                    surface from recycling. The reference to the surface
+   *                    must be freed before releasing the main thread context.
+   */
+  already_AddRefed<SourceSurface> GetSourceSurfaceInternal(bool aTemporary);
 
   uint32_t PaletteDataLength() const
   {
     return mPaletteDepth ? (size_t(1) << mPaletteDepth) * sizeof(uint32_t)
                          : 0;
   }
 
   struct SurfaceWithFormat {
@@ -282,16 +301,17 @@ private: // methods
   SurfaceWithFormat SurfaceForDrawing(bool               aDoPartialDecode,
                                       bool               aDoTile,
                                       ImageRegion&       aRegion,
                                       SourceSurface*     aSurface);
 
 private: // data
   friend class DrawableFrameRef;
   friend class RawAccessFrameRef;
+  friend class RecyclingSourceSurface;
   friend class UnlockImageDataRunnable;
 
   //////////////////////////////////////////////////////////////////////////////
   // Thread-safe mutable data, protected by mMonitor.
   //////////////////////////////////////////////////////////////////////////////
 
   mutable Monitor mMonitor;
 
@@ -313,21 +333,25 @@ private: // data
    * is unused if the DrawTarget is able to render DataSourceSurface buffers
    * directly.
    */
   RefPtr<SourceSurface> mOptSurface;
 
   nsIntRect mDecoded;
 
   //! Number of RawAccessFrameRefs currently alive for this imgFrame.
-  int32_t mLockCount;
+  int16_t mLockCount;
+
+  //! Number of RecyclingSourceSurface's currently alive for this imgFrame.
+  int16_t mRecycleLockCount;
 
   bool mAborted;
   bool mFinished;
   bool mOptimizable;
+  bool mShouldRecycle;
 
 
   //////////////////////////////////////////////////////////////////////////////
   // Effectively const data, only mutated in the Init methods.
   //////////////////////////////////////////////////////////////////////////////
 
   //! The size of the buffer we are decoding to.
   IntSize      mImageSize;
--- a/image/moz.build
+++ b/image/moz.build
@@ -50,16 +50,17 @@ EXPORTS += [
     'imgRequestProxy.h',
     'IProgressObserver.h',
     'Orientation.h',
     'SurfaceCacheUtils.h',
 ]
 
 EXPORTS.mozilla.image += [
     'ImageMemoryReporter.h',
+    'RecyclingSourceSurface.h',
 ]
 
 UNIFIED_SOURCES += [
     'AnimationFrameBuffer.cpp',
     'AnimationSurfaceProvider.cpp',
     'ClippedImage.cpp',
     'DecodedSurfaceProvider.cpp',
     'DecodePool.cpp',