Bug 1256603. Only mark images as used in the surface cache if we actually use them. r=aosmond
authorTimothy Nikkel <tnikkel@gmail.com>
Sat, 13 Oct 2018 00:31:02 -0500
changeset 441105 8421b16c011ba49a2e495f81853b282413aa7650
parent 441104 0c5f5c2e2a860f9f681e301662aabe2deca8be4b
child 441106 479653ce6d8c31dd5cddc815f9ad19a5b06b7fda
push id34842
push useraciure@mozilla.com
push dateSat, 13 Oct 2018 09:36:47 +0000
treeherdermozilla-central@94a62c1aad52 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersaosmond
bugs1256603
milestone64.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 1256603. Only mark images as used in the surface cache if we actually use them. r=aosmond We were marking them used even if only a decode was requested. This can cause us to hold extra decoded copies of the image around because we have a tendency to request decode at the intrinsic size.
image/FrameAnimator.cpp
image/FrameAnimator.h
image/RasterImage.cpp
image/RasterImage.h
image/SurfaceCache.cpp
image/SurfaceCache.h
--- a/image/FrameAnimator.cpp
+++ b/image/FrameAnimator.cpp
@@ -32,17 +32,18 @@ AnimationState::UpdateState(bool aAnimat
                             RasterImage *aImage,
                             const gfx::IntSize& aSize,
                             bool aAllowInvalidation /* = true */)
 {
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(aImage),
                          RasterSurfaceKey(aSize,
                                           DefaultSurfaceFlags(),
-                                          PlaybackType::eAnimated));
+                                          PlaybackType::eAnimated),
+                         /* aMarkUsed = */ false);
 
   return UpdateStateInternal(result, aAnimationFinished, aSize, aAllowInvalidation);
 }
 
 const gfx::IntRect
 AnimationState::UpdateStateInternal(LookupResult& aResult,
                                     bool aAnimationFinished,
                                     const gfx::IntSize& aSize,
@@ -393,17 +394,18 @@ FrameAnimator::ResetAnimation(AnimationS
   aState.ResetAnimation();
 
   // Our surface provider is synchronized to our state, so we need to reset its
   // state as well, if we still have one.
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(mImage),
                          RasterSurfaceKey(mSize,
                                           DefaultSurfaceFlags(),
-                                          PlaybackType::eAnimated));
+                                          PlaybackType::eAnimated),
+                         /* aMarkUsed = */ false);
   if (!result) {
     return;
   }
 
   result.Surface().Reset();
 }
 
 RefreshResult
@@ -422,17 +424,18 @@ FrameAnimator::RequestRefresh(AnimationS
   // Get the animation frames once now, and pass them down to callees because
   // the surface could be discarded at anytime on a different thread. This is
   // must easier to reason about then trying to write code that is safe to
   // having the surface disappear at anytime.
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(mImage),
                          RasterSurfaceKey(mSize,
                                           DefaultSurfaceFlags(),
-                                          PlaybackType::eAnimated));
+                                          PlaybackType::eAnimated),
+                         /* aMarkUsed = */ true);
 
   ret.mDirtyRect = aState.UpdateStateInternal(result, aAnimationFinished, mSize);
   if (aState.IsDiscarded() || !result) {
     aState.MaybeAdvanceAnimationFrameTime(aTime);
     if (!ret.mDirtyRect.IsEmpty()) {
       ret.mFrameAdvanced = true;
     }
     return ret;
@@ -497,32 +500,33 @@ FrameAnimator::RequestRefresh(AnimationS
   }
 
   MOZ_ASSERT(!aState.mIsCurrentlyDecoded || !aState.mCompositedFrameInvalid);
 
   return ret;
 }
 
 LookupResult
-FrameAnimator::GetCompositedFrame(AnimationState& aState)
+FrameAnimator::GetCompositedFrame(AnimationState& aState, bool aMarkUsed)
 {
   aState.mCompositedFrameRequested = true;
 
   // If we have a composited version of this frame, return that.
   if (!aState.mCompositedFrameInvalid && mLastCompositedFrameIndex >= 0 &&
       (uint32_t(mLastCompositedFrameIndex) == aState.mCurrentAnimationFrameIndex)) {
     return LookupResult(DrawableSurface(mCompositingFrame->DrawableRef()),
                         MatchType::EXACT);
   }
 
   LookupResult result =
     SurfaceCache::Lookup(ImageKey(mImage),
                          RasterSurfaceKey(mSize,
                                           DefaultSurfaceFlags(),
-                                          PlaybackType::eAnimated));
+                                          PlaybackType::eAnimated),
+                         aMarkUsed);
 
   if (aState.mCompositedFrameInvalid) {
     MOZ_ASSERT(gfxPrefs::ImageMemAnimatedDiscardable());
     MOZ_ASSERT(aState.GetHasRequestedDecode());
     MOZ_ASSERT(!aState.GetIsCurrentlyDecoded());
     if (result.Type() == MatchType::NOT_FOUND) {
       return result;
     }
--- a/image/FrameAnimator.h
+++ b/image/FrameAnimator.h
@@ -318,17 +318,18 @@ public:
                                const TimeStamp& aTime,
                                bool aAnimationFinished);
 
   /**
    * Get the full frame for the current frame of the animation (it may or may
    * not have required compositing). It may not be available because it hasn't
    * been decoded yet, in which case we return an empty LookupResult.
    */
-  LookupResult GetCompositedFrame(AnimationState& aState);
+  LookupResult GetCompositedFrame(AnimationState& aState,
+                                  bool aMarkUsed);
 
   /**
    * Collect an accounting of the memory occupied by the compositing surfaces we
    * use during animation playback. All of the actual animation frames are
    * stored in the SurfaceCache, so we don't need to report them here.
    */
   void CollectSizeOfCompositingSurfaces(nsTArray<SurfaceMemoryCounter>& aCounters,
                                         MallocSizeOf aMallocSizeOf) const;
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -293,48 +293,52 @@ RasterImage::GetType(uint16_t* aType)
 
   *aType = imgIContainer::TYPE_RASTER;
   return NS_OK;
 }
 
 LookupResult
 RasterImage::LookupFrameInternal(const IntSize& aSize,
                                  uint32_t aFlags,
-                                 PlaybackType aPlaybackType)
+                                 PlaybackType aPlaybackType,
+                                 bool aMarkUsed)
 {
   if (mAnimationState && aPlaybackType == PlaybackType::eAnimated) {
     MOZ_ASSERT(mFrameAnimator);
     MOZ_ASSERT(ToSurfaceFlags(aFlags) == DefaultSurfaceFlags(),
                "Can't composite frames with non-default surface flags");
-    return mFrameAnimator->GetCompositedFrame(*mAnimationState);
+    return mFrameAnimator->GetCompositedFrame(*mAnimationState, aMarkUsed);
   }
 
   SurfaceFlags surfaceFlags = ToSurfaceFlags(aFlags);
 
   // We don't want any substitution for sync decodes, and substitution would be
   // illegal when high quality downscaling is disabled, so we use
   // SurfaceCache::Lookup in this case.
   if ((aFlags & FLAG_SYNC_DECODE) || !(aFlags & FLAG_HIGH_QUALITY_SCALING)) {
     return SurfaceCache::Lookup(ImageKey(this),
                                 RasterSurfaceKey(aSize,
                                                  surfaceFlags,
-                                                 PlaybackType::eStatic));
+                                                 PlaybackType::eStatic),
+                                aMarkUsed);
   }
 
   // We'll return the best match we can find to the requested frame.
   return SurfaceCache::LookupBestMatch(ImageKey(this),
                                        RasterSurfaceKey(aSize,
                                                         surfaceFlags,
-                                                        PlaybackType::eStatic));
+                                                        PlaybackType::eStatic),
+                                       aMarkUsed);
 }
 
 LookupResult
 RasterImage::LookupFrame(const IntSize& aSize,
                          uint32_t aFlags,
-                         PlaybackType aPlaybackType)
+                         PlaybackType aPlaybackType,
+                         bool aMarkUsed /* = true */)
 {
   MOZ_ASSERT(NS_IsMainThread());
 
   // If we're opaque, we don't need to care about premultiplied alpha, because
   // that can only matter for frames with transparency.
   if (IsOpaque()) {
     aFlags &= ~FLAG_DECODE_NO_PREMULTIPLY_ALPHA;
   }
@@ -342,17 +346,17 @@ RasterImage::LookupFrame(const IntSize& 
   IntSize requestedSize = CanDownscaleDuringDecode(aSize, aFlags)
                         ? aSize : mSize;
   if (requestedSize.IsEmpty()) {
     // Can't decode to a surface of zero size.
     return LookupResult(MatchType::NOT_FOUND);
   }
 
   LookupResult result =
-    LookupFrameInternal(requestedSize, aFlags, aPlaybackType);
+    LookupFrameInternal(requestedSize, aFlags, aPlaybackType, aMarkUsed);
 
   if (!result && !mHasSize) {
     // We can't request a decode without knowing our intrinsic size. Give up.
     return LookupResult(MatchType::NOT_FOUND);
   }
 
   if (result.Type() == MatchType::NOT_FOUND ||
       result.Type() == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND ||
@@ -372,17 +376,17 @@ RasterImage::LookupFrame(const IntSize& 
                   (aFlags & FLAG_HIGH_QUALITY_SCALING));
       requestedSize = result.SuggestedSize();
     }
 
     bool ranSync = Decode(requestedSize, aFlags, aPlaybackType);
 
     // If we can or did sync decode, we should already have the frame.
     if (ranSync || (aFlags & FLAG_SYNC_DECODE)) {
-      result = LookupFrameInternal(requestedSize, aFlags, aPlaybackType);
+      result = LookupFrameInternal(requestedSize, aFlags, aPlaybackType, aMarkUsed);
     }
   }
 
   if (!result) {
     // We still weren't able to get a frame. Give up.
     return result;
   }
 
@@ -457,17 +461,18 @@ RasterImage::WillDrawOpaqueNow()
   if (mLockCount == 0) {
     return false;
   }
 
   LookupResult result =
     SurfaceCache::LookupBestMatch(ImageKey(this),
                                   RasterSurfaceKey(mSize,
                                                    DefaultSurfaceFlags(),
-                                                   PlaybackType::eStatic));
+                                                   PlaybackType::eStatic),
+                                  /* aMarkUsed = */ false);
   MatchType matchType = result.Type();
   if (matchType == MatchType::NOT_FOUND || matchType == MatchType::PENDING ||
       !result.Surface()->IsFinished()) {
     return false;
   }
 
   return true;
 }
@@ -1195,17 +1200,17 @@ RasterImage::RequestDecodeForSizeInterna
 
   uint32_t flags = shouldSyncDecodeIfFast
                  ? aFlags
                  : aFlags & ~FLAG_SYNC_DECODE_IF_FAST;
 
   // Perform a frame lookup, which will implicitly start decoding if needed.
   PlaybackType playbackType = mAnimationState ? PlaybackType::eAnimated
                                               : PlaybackType::eStatic;
-  LookupResult result = LookupFrame(aSize, flags, playbackType);
+  LookupResult result = LookupFrame(aSize, flags, playbackType, /* aMarkUsed = */ false);
   return std::move(result.Surface());
 }
 
 static bool
 LaunchDecodingTask(IDecodingTask* aTask,
                    RasterImage* aImage,
                    uint32_t aFlags,
                    bool aHaveSourceData)
--- a/image/RasterImage.h
+++ b/image/RasterImage.h
@@ -279,29 +279,32 @@ private:
   /**
    * Tries to retrieve a surface for this image with size @aSize, surface flags
    * matching @aFlags, and a playback type of @aPlaybackType.
    *
    * If @aFlags specifies FLAG_SYNC_DECODE and we already have all the image
    * data, we'll attempt a sync decode if no matching surface is found. If
    * FLAG_SYNC_DECODE was not specified and no matching surface was found, we'll
    * kick off an async decode so that the surface is (hopefully) available next
-   * time it's requested.
+   * time it's requested. aMarkUsed determines if we mark the surface used in
+   * the surface cache or not.
    *
    * @return a drawable surface, which may be empty if the requested surface
    *         could not be found.
    */
   LookupResult LookupFrame(const gfx::IntSize& aSize,
                            uint32_t aFlags,
-                           PlaybackType aPlaybackType);
+                           PlaybackType aPlaybackType,
+                           bool aMarkUsed = true);
 
   /// Helper method for LookupFrame().
   LookupResult LookupFrameInternal(const gfx::IntSize& aSize,
                                    uint32_t aFlags,
-                                   PlaybackType aPlaybackType);
+                                   PlaybackType aPlaybackType,
+                                   bool aMarkUsed);
 
   ImgDrawResult DrawInternal(DrawableSurface&& aFrameRef,
                           gfxContext* aContext,
                           const nsIntSize& aSize,
                           const ImageRegion& aRegion,
                           gfx::SamplingFilter aSamplingFilter,
                           uint32_t aFlags,
                           float aOpacity);
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -994,17 +994,18 @@ public:
 
     MOZ_ASSERT(surface->GetSurfaceKey() == aSurfaceKey,
                "Lookup() not returning an exact match?");
     return LookupResult(std::move(drawableSurface), MatchType::EXACT);
   }
 
   LookupResult LookupBestMatch(const ImageKey         aImageKey,
                                const SurfaceKey&      aSurfaceKey,
-                               const StaticMutexAutoLock& aAutoLock)
+                               const StaticMutexAutoLock& aAutoLock,
+                               bool aMarkUsed /* = true */)
   {
     RefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
       // No cached surfaces for this image.
       return LookupResult(MatchType::NOT_FOUND);
     }
 
     // Repeatedly look up the best match, trying again if the resulting surface
@@ -1040,17 +1041,18 @@ public:
     MOZ_ASSERT_IF(matchType == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND ||
                   matchType == MatchType::SUBSTITUTE_BECAUSE_PENDING,
       surface->GetSurfaceKey().SVGContext() == aSurfaceKey.SVGContext() &&
       surface->GetSurfaceKey().Playback() == aSurfaceKey.Playback() &&
       surface->GetSurfaceKey().Flags() == aSurfaceKey.Flags());
 
     if (matchType == MatchType::EXACT ||
         matchType == MatchType::SUBSTITUTE_BECAUSE_BEST) {
-      if (!MarkUsed(WrapNotNull(surface), WrapNotNull(cache), aAutoLock)) {
+      if (aMarkUsed &&
+          !MarkUsed(WrapNotNull(surface), WrapNotNull(cache), aAutoLock)) {
         Remove(WrapNotNull(surface), /* aStopTracking */ false, aAutoLock);
       }
     }
 
     return LookupResult(std::move(drawableSurface), matchType, suggestedSize);
   }
 
   bool CanHold(const Cost aCost) const
@@ -1511,48 +1513,50 @@ SurfaceCache::Shutdown()
     MOZ_ASSERT(NS_IsMainThread());
     MOZ_ASSERT(sInstance, "No singleton - was Shutdown() called twice?");
     cache = sInstance.forget();
   }
 }
 
 /* static */ LookupResult
 SurfaceCache::Lookup(const ImageKey         aImageKey,
-                     const SurfaceKey&      aSurfaceKey)
+                     const SurfaceKey&      aSurfaceKey,
+                     bool aMarkUsed /* = true */)
 {
   nsTArray<RefPtr<CachedSurface>> discard;
   LookupResult rv(MatchType::NOT_FOUND);
 
   {
     StaticMutexAutoLock lock(sInstanceMutex);
     if (!sInstance) {
       return rv;
     }
 
-    rv = sInstance->Lookup(aImageKey, aSurfaceKey, lock);
+    rv = sInstance->Lookup(aImageKey, aSurfaceKey, lock, aMarkUsed);
     sInstance->TakeDiscard(discard, lock);
   }
 
   return rv;
 }
 
 /* static */ LookupResult
 SurfaceCache::LookupBestMatch(const ImageKey         aImageKey,
-                              const SurfaceKey&      aSurfaceKey)
+                              const SurfaceKey&      aSurfaceKey,
+                              bool aMarkUsed /* = true */)
 {
   nsTArray<RefPtr<CachedSurface>> discard;
   LookupResult rv(MatchType::NOT_FOUND);
 
   {
     StaticMutexAutoLock lock(sInstanceMutex);
     if (!sInstance) {
       return rv;
     }
 
-    rv = sInstance->LookupBestMatch(aImageKey, aSurfaceKey, lock);
+    rv = sInstance->LookupBestMatch(aImageKey, aSurfaceKey, lock, aMarkUsed);
     sInstance->TakeDiscard(discard, lock);
   }
 
   return rv;
 }
 
 /* static */ InsertOutcome
 SurfaceCache::Insert(NotNull<ISurfaceProvider*> aProvider)
--- a/image/SurfaceCache.h
+++ b/image/SurfaceCache.h
@@ -227,17 +227,18 @@ struct SurfaceCache
    * @param aImageKey       Key data identifying which image the cache entry
    *                        belongs to.
    * @param aSurfaceKey     Key data which uniquely identifies the requested
    *                        cache entry.
    * @return                a LookupResult which will contain a DrawableSurface
    *                        if the cache entry was found.
    */
   static LookupResult Lookup(const ImageKey    aImageKey,
-                             const SurfaceKey& aSurfaceKey);
+                             const SurfaceKey& aSurfaceKey,
+                             bool aMarkUsed = true);
 
   /**
    * Looks up the best matching cache entry and returns a drawable reference to
    * its associated surface.
    *
    * The result may vary from the requested cache entry only in terms of size.
    *
    * @param aImageKey       Key data identifying which image the cache entry
@@ -246,17 +247,18 @@ struct SurfaceCache
    *                        cache entry.
    * @return                a LookupResult which will contain a DrawableSurface
    *                        if a cache entry similar to the one the caller
    *                        requested could be found. Callers can use
    *                        LookupResult::IsExactMatch() to check whether the
    *                        returned surface exactly matches @aSurfaceKey.
    */
   static LookupResult LookupBestMatch(const ImageKey    aImageKey,
-                                      const SurfaceKey& aSurfaceKey);
+                                      const SurfaceKey& aSurfaceKey,
+                                      bool aMarkUsed = true);
 
   /**
    * Insert an ISurfaceProvider into the cache. If an entry with the same
    * ImageKey and SurfaceKey is already in the cache, Insert returns
    * FAILURE_ALREADY_PRESENT. If a matching placeholder is already present, it
    * is replaced.
    *
    * Cache entries will never expire as long as they remain locked, but if they