Bug 1176124 (Part 2) - Add placeholder support to the SurfaceCache so we can avoid launching redundant decoders. r=dholbert
authorSeth Fowler <mark.seth.fowler@gmail.com>
Sun, 19 Jul 2015 18:39:44 -0700
changeset 253641 f52a8f3b15ed32a3c9084133b0fe44374effeaf7
parent 253640 6747b7f35dcb1e2ce70251e31080a3abba2115bd
child 253642 5834e85fc660b4fb6e466fe13fcead5b8d733545
push id14109
push usercbook@mozilla.com
push dateMon, 20 Jul 2015 10:16:00 +0000
treeherderfx-team@ed1bcfdd4a7c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1176124
milestone42.0a1
Bug 1176124 (Part 2) - Add placeholder support to the SurfaceCache so we can avoid launching redundant decoders. r=dholbert
image/RasterImage.cpp
image/SurfaceCache.cpp
image/SurfaceCache.h
--- a/image/RasterImage.cpp
+++ b/image/RasterImage.cpp
@@ -521,19 +521,22 @@ RasterImage::LookupFrame(uint32_t aFrame
 
   LookupResult result = LookupFrameInternal(aFrameNum, requestedSize, aFlags);
 
   if (!result && !mHasSize) {
     // We can't request a decode without knowing our intrinsic size. Give up.
     return DrawableFrameRef();
   }
 
-  if (!result || result.Type() == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND) {
+  if (result.Type() == MatchType::NOT_FOUND ||
+      result.Type() == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND ||
+      ((aFlags & FLAG_SYNC_DECODE) && !result)) {
     // We don't have a copy of this frame, and there's no decoder working on
-    // one. Trigger decoding so it'll be available next time.
+    // one. (Or we're sync decoding and the existing decoder hasn't even started
+    // yet.) Trigger decoding so it'll be available next time.
     MOZ_ASSERT(!mAnim, "Animated frames should be locked");
 
     Decode(Some(requestedSize), aFlags);
 
     // If we can sync decode, we should already have the frame.
     if (aFlags & FLAG_SYNC_DECODE) {
       result = LookupFrameInternal(aFrameNum, requestedSize, aFlags);
     }
@@ -1495,16 +1498,29 @@ RasterImage::CreateDecoder(const Maybe<I
   }
 
   decoder->Init();
 
   if (NS_FAILED(decoder->GetDecoderError())) {
     return nullptr;
   }
 
+  if (aSize) {
+    // Add a placeholder for the first frame to the SurfaceCache so we won't
+    // trigger any more decoders with the same parameters.
+    InsertOutcome outcome =
+      SurfaceCache::InsertPlaceholder(ImageKey(this),
+                                      RasterSurfaceKey(*aSize,
+                                                       decoder->GetDecodeFlags(),
+                                                       /* aFrameNum = */ 0));
+    if (outcome != InsertOutcome::SUCCESS) {
+      return nullptr;
+    }
+  }
+
   if (!aSize) {
     Telemetry::GetHistogramById(
       Telemetry::IMAGE_DECODE_COUNT)->Subtract(mDecodeCount);
     mDecodeCount++;
     Telemetry::GetHistogramById(
       Telemetry::IMAGE_DECODE_COUNT)->Add(mDecodeCount);
 
     if (mDecodeCount > sMaxDecodeCount) {
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -11,16 +11,17 @@
 
 #include <algorithm>
 #include "mozilla/Assertions.h"
 #include "mozilla/Attributes.h"
 #include "mozilla/DebugOnly.h"
 #include "mozilla/Likely.h"
 #include "mozilla/Move.h"
 #include "mozilla/Mutex.h"
+#include "mozilla/Pair.h"
 #include "mozilla/RefPtr.h"
 #include "mozilla/StaticPtr.h"
 #include "nsIMemoryReporter.h"
 #include "gfx2DGlue.h"
 #include "gfxPattern.h"  // Workaround for flaw in bug 921753 part 2.
 #include "gfxPlatform.h"
 #include "gfxPrefs.h"
 #include "imgFrame.h"
@@ -62,16 +63,20 @@ static StaticRefPtr<SurfaceCacheImpl> sI
 /**
  * Cost models the cost of storing a surface in the cache. Right now, this is
  * simply an estimate of the size of the surface in bytes, but in the future it
  * may be worth taking into account the cost of rematerializing the surface as
  * well.
  */
 typedef size_t Cost;
 
+// Placeholders do not have surfaces, but need to be given a trivial cost for
+// our invariants to hold.
+static const Cost sPlaceholderCost = 1;
+
 static Cost
 ComputeCost(const IntSize& aSize, uint32_t aBytesPerPixel)
 {
   MOZ_ASSERT(aBytesPerPixel == 1 || aBytesPerPixel == 4);
   return aSize.width * aSize.height * aBytesPerPixel;
 }
 
 /**
@@ -132,45 +137,61 @@ public:
                 const SurfaceKey&  aSurfaceKey,
                 const Lifetime     aLifetime)
     : mSurface(aSurface)
     , mCost(aCost)
     , mImageKey(aImageKey)
     , mSurfaceKey(aSurfaceKey)
     , mLifetime(aLifetime)
   {
-    MOZ_ASSERT(mSurface, "Must have a valid surface");
+    MOZ_ASSERT(!IsPlaceholder() ||
+               (mCost == sPlaceholderCost && mLifetime == Lifetime::Transient),
+               "Placeholder should have trivial cost and transient lifetime");
     MOZ_ASSERT(mImageKey, "Must have a valid image key");
   }
 
   DrawableFrameRef DrawableRef() const
   {
+    if (MOZ_UNLIKELY(IsPlaceholder())) {
+      MOZ_ASSERT_UNREACHABLE("Shouldn't call DrawableRef() on a placeholder");
+      return DrawableFrameRef();
+    }
+
     return mSurface->DrawableRef();
   }
 
   void SetLocked(bool aLocked)
   {
+    if (IsPlaceholder()) {
+      return;  // Can't lock a placeholder.
+    }
+
     if (aLocked && mLifetime == Lifetime::Persistent) {
       // This may fail, and that's OK. We make no guarantees about whether
       // locking is successful if you call SurfaceCache::LockImage() after
       // SurfaceCache::Insert().
       mDrawableRef = mSurface->DrawableRef();
     } else {
       mDrawableRef.reset();
     }
   }
 
+  bool IsPlaceholder() const { return !bool(mSurface); }
   bool IsLocked() const { return bool(mDrawableRef); }
 
   ImageKey GetImageKey() const { return mImageKey; }
   SurfaceKey GetSurfaceKey() const { return mSurfaceKey; }
   CostEntry GetCostEntry() { return image::CostEntry(this, mCost); }
   nsExpirationState* GetExpirationState() { return &mExpirationState; }
   Lifetime GetLifetime() const { return mLifetime; }
-  bool IsDecoded() const { return mSurface->IsImageComplete(); }
+
+  bool IsDecoded() const
+  {
+    return !IsPlaceholder() && mSurface->IsImageComplete();
+  }
 
   // A helper type used by SurfaceCacheImpl::CollectSizeOfSurfaces.
   struct MOZ_STACK_CLASS SurfaceMemoryReport
   {
     SurfaceMemoryReport(nsTArray<SurfaceMemoryCounter>& aCounters,
                         MallocSizeOf                    aMallocSizeOf)
       : mCounters(aCounters)
       , mMallocSizeOf(aMallocSizeOf)
@@ -257,31 +278,57 @@ public:
 
   already_AddRefed<CachedSurface> Lookup(const SurfaceKey& aSurfaceKey)
   {
     nsRefPtr<CachedSurface> surface;
     mSurfaces.Get(aSurfaceKey, getter_AddRefs(surface));
     return surface.forget();
   }
 
-  already_AddRefed<CachedSurface>
+  MOZ_WARN_UNUSED_RESULT  // See bug 1185044.
+  Pair<already_AddRefed<CachedSurface>, MatchType>
   LookupBestMatch(const SurfaceKey&      aSurfaceKey,
                   const Maybe<uint32_t>& aAlternateFlags)
   {
-    // Try for a perfect match first.
-    nsRefPtr<CachedSurface> surface;
-    mSurfaces.Get(aSurfaceKey, getter_AddRefs(surface));
-    if (surface && surface->IsDecoded()) {
-      return surface.forget();
+    // Try for an exact match first.
+    nsRefPtr<CachedSurface> exactMatch;
+    mSurfaces.Get(aSurfaceKey, getter_AddRefs(exactMatch));
+    if (exactMatch && exactMatch->IsDecoded()) {
+      return MakePair(exactMatch.forget(), MatchType::EXACT);
     }
 
     // There's no perfect match, so find the best match we can.
     MatchContext matchContext(aSurfaceKey, aAlternateFlags);
     ForEach(TryToImproveMatch, &matchContext);
-    return matchContext.mBestMatch.forget();
+
+    MatchType matchType;
+    if (matchContext.mBestMatch) {
+      if (!exactMatch) {
+        // No exact match, but we found a substitute.
+        matchType = MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND;
+      } else if (exactMatch != matchContext.mBestMatch) {
+        // The exact match is still decoding, but we found a substitute.
+        matchType = MatchType::SUBSTITUTE_BECAUSE_PENDING;
+      } else {
+        // The exact match is still decoding, but it's the best we've got.
+        matchType = MatchType::EXACT;
+      }
+    } else {
+      if (exactMatch) {
+        // We found an "exact match", but since TryToImproveMatch didn't return
+        // it, it must have been a placeholder.
+        MOZ_ASSERT(exactMatch->IsPlaceholder());
+        matchType = MatchType::PENDING;
+      } else {
+        // We couldn't find an exact match *or* a substitute.
+        matchType = MatchType::NOT_FOUND;
+      }
+    }
+
+    return MakePair(matchContext.mBestMatch.forget(), matchType);
   }
 
   void ForEach(SurfaceTable::EnumReadFunction aFunction, void* aData)
   {
     mSurfaces.EnumerateRead(aFunction, aData);
   }
 
   void SetLocked(bool aLocked) { mLocked = aLocked; }
@@ -303,16 +350,21 @@ private:
 
   static PLDHashOperator TryToImproveMatch(const SurfaceKey& aSurfaceKey,
                                            CachedSurface*    aSurface,
                                            void*             aContext)
   {
     auto context = static_cast<MatchContext*>(aContext);
     const SurfaceKey& idealKey = context->mIdealKey;
 
+    // We never match a placeholder.
+    if (aSurface->IsPlaceholder()) {
+      return PL_DHASH_NEXT;
+    }
+
     // Matching the animation time and SVG context is required.
     if (aSurfaceKey.AnimationTime() != idealKey.AnimationTime() ||
         aSurfaceKey.SVGContext() != idealKey.SVGContext()) {
       return PL_DHASH_NEXT;
     }
 
     // Matching the flags is required, but we can match the alternate flags as
     // well if some were provided.
@@ -420,20 +472,31 @@ public:
 
   InsertOutcome Insert(imgFrame*         aSurface,
                        const Cost        aCost,
                        const ImageKey    aImageKey,
                        const SurfaceKey& aSurfaceKey,
                        Lifetime          aLifetime)
   {
     // If this is a duplicate surface, refuse to replace the original.
-    if (MOZ_UNLIKELY(Lookup(aImageKey, aSurfaceKey, /* aMarkUsed = */ false))) {
+    // XXX(seth): Calling Lookup() and then RemoveSurface() does the lookup
+    // twice. We'll make this more efficient in bug 1185137.
+    LookupResult result = Lookup(aImageKey, aSurfaceKey, /* aMarkUsed = */ false);
+    if (MOZ_UNLIKELY(result)) {
       return InsertOutcome::FAILURE_ALREADY_PRESENT;
     }
 
+    if (result.Type() == MatchType::PENDING) {
+      RemoveSurface(aImageKey, aSurfaceKey);
+    }
+
+    MOZ_ASSERT(result.Type() == MatchType::NOT_FOUND ||
+               result.Type() == MatchType::PENDING,
+               "A LookupResult with no surface should be NOT_FOUND or PENDING");
+
     // If this is bigger than we can hold after discarding everything we can,
     // refuse to cache it.
     if (MOZ_UNLIKELY(!CanHoldAfterDiscarding(aCost))) {
       mOverflowCount++;
       return InsertOutcome::FAILURE;
     }
 
     // Remove elements in order of cost until we can fit this in the cache. Note
@@ -453,16 +516,17 @@ public:
     }
 
     nsRefPtr<CachedSurface> surface =
       new CachedSurface(aSurface, aCost, aImageKey, aSurfaceKey, aLifetime);
 
     // We require that locking succeed if the image is locked and the surface is
     // persistent; the caller may need to know this to handle errors correctly.
     if (cache->IsLocked() && aLifetime == Lifetime::Persistent) {
+      MOZ_ASSERT(!surface->IsPlaceholder(), "Placeholders should be transient");
       surface->SetLocked(true);
       if (!surface->IsLocked()) {
         return InsertOutcome::FAILURE;
       }
     }
 
     // Insert.
     MOZ_ASSERT(aCost <= mAvailableCost, "Inserting despite too large a cost");
@@ -555,16 +619,20 @@ public:
     }
 
     nsRefPtr<CachedSurface> surface = cache->Lookup(aSurfaceKey);
     if (!surface) {
       // Lookup in the per-image cache missed.
       return LookupResult(MatchType::NOT_FOUND);
     }
 
+    if (surface->IsPlaceholder()) {
+      return LookupResult(MatchType::PENDING);
+    }
+
     DrawableFrameRef ref = surface->DrawableRef();
     if (!ref) {
       // The surface was released by the operating system. Remove the cache
       // entry as well.
       Remove(surface);
       return LookupResult(MatchType::NOT_FOUND);
     }
 
@@ -590,48 +658,49 @@ public:
     // Repeatedly look up the best match, trying again if the resulting surface
     // has been freed by the operating system, until we can either lock a
     // surface for drawing or there are no matching surfaces left.
     // XXX(seth): This is O(N^2), but N is expected to be very small. If we
     // encounter a performance problem here we can revisit this.
 
     nsRefPtr<CachedSurface> surface;
     DrawableFrameRef ref;
+    MatchType matchType = MatchType::NOT_FOUND;
     while (true) {
-      surface = cache->LookupBestMatch(aSurfaceKey, aAlternateFlags);
+      // XXX(seth): This code is begging for std::tie. See bug 1184385.
+      Pair<already_AddRefed<CachedSurface>, MatchType> lookupResult =
+        cache->LookupBestMatch(aSurfaceKey, aAlternateFlags);
+      surface = lookupResult.first();
+      matchType = lookupResult.second();
+
       if (!surface) {
-        // Lookup in the per-image cache missed.
-        return LookupResult(MatchType::NOT_FOUND);
+        return LookupResult(matchType);  // Lookup in the per-image cache missed.
       }
 
       ref = surface->DrawableRef();
       if (ref) {
         break;
       }
 
       // The surface was released by the operating system. Remove the cache
       // entry as well.
       Remove(surface);
     }
 
-    SurfaceKey key = surface->GetSurfaceKey();
-    const bool isExactMatch = key.Size() == aSurfaceKey.Size();
-
-    MOZ_ASSERT(isExactMatch ==
-      (key == aSurfaceKey ||
-         (aAlternateFlags && key == aSurfaceKey.WithNewFlags(*aAlternateFlags))),
+    MOZ_ASSERT((matchType == MatchType::EXACT) ==
+      (surface->GetSurfaceKey() == aSurfaceKey ||
+         (aAlternateFlags &&
+          surface->GetSurfaceKey() ==
+            aSurfaceKey.WithNewFlags(*aAlternateFlags))),
       "Result differs in a way other than size or alternate flags");
 
-
-    if (isExactMatch) {
+    if (matchType == MatchType::EXACT) {
       MarkUsed(surface, cache);
     }
 
-    MatchType matchType = isExactMatch ? MatchType::EXACT
-                                       : MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND;
     return LookupResult(Move(ref), matchType);
   }
 
   void RemoveSurface(const ImageKey    aImageKey,
                      const SurfaceKey& aSurfaceKey)
   {
     nsRefPtr<ImageSurfaceCache> cache = GetImageCache(aImageKey);
     if (!cache) {
@@ -1043,16 +1112,29 @@ SurfaceCache::Insert(imgFrame*         a
     MOZ_CRASH("Don't pass null surfaces to SurfaceCache::Insert");
   }
 
   MutexAutoLock lock(sInstance->GetMutex());
   Cost cost = ComputeCost(aSurface->GetSize(), aSurface->GetBytesPerPixel());
   return sInstance->Insert(aSurface, cost, aImageKey, aSurfaceKey, aLifetime);
 }
 
+/* static */ InsertOutcome
+SurfaceCache::InsertPlaceholder(const ImageKey    aImageKey,
+                                const SurfaceKey& aSurfaceKey)
+{
+  if (!sInstance) {
+    return InsertOutcome::FAILURE;
+  }
+
+  MutexAutoLock lock(sInstance->GetMutex());
+  return sInstance->Insert(nullptr, sPlaceholderCost, aImageKey, aSurfaceKey,
+                           Lifetime::Transient);
+}
+
 /* static */ bool
 SurfaceCache::CanHold(const IntSize& aSize, uint32_t aBytesPerPixel /* = 4 */)
 {
   if (!sInstance) {
     return false;
   }
 
   Cost cost = ComputeCost(aSize, aBytesPerPixel);
--- a/image/SurfaceCache.h
+++ b/image/SurfaceCache.h
@@ -225,16 +225,17 @@ struct SurfaceCache
   static LookupResult LookupBestMatch(const ImageKey    aImageKey,
                                       const SurfaceKey& aSurfaceKey,
                                       const Maybe<uint32_t>& aAlternateFlags
                                         = Nothing());
 
   /**
    * Insert a surface into the cache. If a surface with the same ImageKey and
    * SurfaceKey is already in the cache, Insert returns FAILURE_ALREADY_PRESENT.
+   * If a matching placeholder is already present, the placeholder is removed.
    *
    * Each surface in the cache has a lifetime, either Transient or Persistent.
    * Transient surfaces can expire from the cache at any time. Persistent
    * surfaces, on the other hand, will never expire as long as they remain
    * locked, but if they become unlocked, can expire just like transient
    * surfaces. When it is first inserted, a persistent surface is locked if its
    * associated image is locked. When that image is later unlocked, the surface
    * becomes unlocked too. To become locked again at that point, two things must
@@ -276,16 +277,43 @@ struct SurfaceCache
    *           SurfaceKey already exists in the cache.
    */
   static InsertOutcome Insert(imgFrame*         aSurface,
                               const ImageKey    aImageKey,
                               const SurfaceKey& aSurfaceKey,
                               Lifetime          aLifetime);
 
   /**
+   * Insert a placeholder for a surface into the cache. If a surface with the
+   * same ImageKey and SurfaceKey is already in the cache, InsertPlaceholder()
+   * returns FAILURE_ALREADY_PRESENT.
+   *
+   * Placeholders exist to allow lazy allocation of surfaces. The Lookup*()
+   * methods will report whether a placeholder for an exactly matching surface
+   * existed by returning a MatchType of PENDING or SUBSTITUTE_BECAUSE_PENDING,
+   * but they will never return a placeholder directly. (They couldn't, since
+   * placeholders don't have an associated surface.)
+   *
+   * Once inserted, placeholders can be removed using RemoveSurface() or
+   * RemoveImage(), just like a surface.  They're automatically removed when a
+   * real surface that matches the placeholder is inserted with Insert().
+   *
+   * @param aImageKey    Key data identifying which image the placeholder
+   *                     belongs to.
+   * @param aSurfaceKey  Key data which uniquely identifies the surface the
+   *                     placeholder stands in for.
+   * @return SUCCESS if the placeholder was inserted successfully.
+   *         FAILURE if the placeholder could not be inserted for some reason.
+   *         FAILURE_ALREADY_PRESENT if a surface with the same ImageKey and
+   *           SurfaceKey already exists in the cache.
+   */
+  static InsertOutcome InsertPlaceholder(const ImageKey    aImageKey,
+                                         const SurfaceKey& aSurfaceKey);
+
+  /**
    * Checks if a surface of a given size could possibly be stored in the cache.
    * If CanHold() returns false, Insert() will always fail to insert the
    * surface, but the inverse is not true: Insert() may take more information
    * into account than just image size when deciding whether to cache the
    * surface, so Insert() may still fail even if CanHold() returns true.
    *
    * Use CanHold() to avoid the need to create a temporary surface when we know
    * for sure the cache can't hold it.
@@ -357,34 +385,35 @@ struct SurfaceCache
    * If the image is unlocked, this has no effect.
    *
    * @param aImageKey    The image which should have its existing surfaces
    *                     unlocked.
    */
   static void UnlockSurfaces(const ImageKey aImageKey);
 
   /**
-   * Removes a surface from the cache, if it's present. If it's not present,
-   * RemoveSurface() has no effect.
+   * Removes a surface or placeholder from the cache, if it's present. If it's
+   * not present, RemoveSurface() has no effect.
    *
    * Use this function to remove individual surfaces that have become invalid.
    * Prefer RemoveImage() or DiscardAll() when they're applicable, as they have
    * much better performance than calling this function repeatedly.
    *
    * @param aImageKey    Key data identifying which image the surface belongs
                          to.
    * @param aSurfaceKey  Key data which uniquely identifies the requested
                          surface.
    */
   static void RemoveSurface(const ImageKey    aImageKey,
                             const SurfaceKey& aSurfaceKey);
 
   /**
-   * Removes all cached surfaces associated with the given image from the cache.
-   * If the image is locked, it is automatically unlocked.
+   * Removes all cached surfaces and placeholders associated with the given
+   * image from the cache.  If the image is locked, it is automatically
+   * unlocked.
    *
    * This MUST be called, at a minimum, when an Image which could be storing
    * surfaces in the surface cache is destroyed. If another image were allocated
    * at the same address it could result in subtle, difficult-to-reproduce bugs.
    *
    * @param aImageKey  The image which should be removed from the cache.
    */
   static void RemoveImage(const ImageKey aImageKey);