Bug 1370412 - Part 6. ImageSurfaceCache::LookupBestMatch should enter factor of 2 mode on cache misses. r=tnikkel
authorAndrew Osmond <aosmond@mozilla.com>
Tue, 05 Sep 2017 07:58:45 -0400
changeset 659177 9d1a5ce9c33350d2deb004808e8c65248ac1dc7a
parent 659176 536e6a1d3b4f990917a5d97765b29b25180512b7
child 659178 71f26daac945b65873a2ce99498638a9cdec0bf7
push id78047
push userbmo:francesco.lodolo@gmail.com
push dateTue, 05 Sep 2017 17:25:17 +0000
reviewerstnikkel
bugs1370412
milestone57.0a1
Bug 1370412 - Part 6. ImageSurfaceCache::LookupBestMatch should enter factor of 2 mode on cache misses. r=tnikkel
image/SurfaceCache.cpp
image/SurfaceCache.h
--- a/image/SurfaceCache.cpp
+++ b/image/SurfaceCache.cpp
@@ -278,24 +278,51 @@ public:
 
   already_AddRefed<CachedSurface> Lookup(const SurfaceKey& aSurfaceKey)
   {
     RefPtr<CachedSurface> surface;
     mSurfaces.Get(aSurfaceKey, getter_AddRefs(surface));
     return surface.forget();
   }
 
-  Pair<already_AddRefed<CachedSurface>, MatchType>
+  /**
+   * @returns A tuple containing the best matching CachedSurface if available,
+   *          a MatchType describing how the CachedSurface was selected, and
+   *          an IntSize which is the size the caller should choose to decode
+   *          at should it attempt to do so.
+   */
+  Tuple<already_AddRefed<CachedSurface>, MatchType, IntSize>
   LookupBestMatch(const SurfaceKey& aIdealKey)
   {
     // Try for an exact match first.
     RefPtr<CachedSurface> exactMatch;
     mSurfaces.Get(aIdealKey, getter_AddRefs(exactMatch));
-    if (exactMatch && exactMatch->IsDecoded()) {
-      return MakePair(exactMatch.forget(), MatchType::EXACT);
+    if (exactMatch) {
+      if (exactMatch->IsDecoded()) {
+        return MakeTuple(exactMatch.forget(), MatchType::EXACT, IntSize());
+      }
+    } else if (!mFactor2Mode) {
+      // If no exact match is found, and we are not in factor of 2 mode, then
+      // we know that we will trigger a decode because at best we will provide
+      // a substitute. Make sure we switch now to factor of 2 mode if necessary.
+      MaybeSetFactor2Mode();
+    }
+
+    // Try for a best match second, if using compact.
+    IntSize suggestedSize = SuggestedSize(aIdealKey.Size());
+    if (mFactor2Mode) {
+      if (!exactMatch) {
+        SurfaceKey compactKey = aIdealKey.CloneWithSize(suggestedSize);
+        mSurfaces.Get(compactKey, getter_AddRefs(exactMatch));
+        if (exactMatch && exactMatch->IsDecoded()) {
+          return MakeTuple(exactMatch.forget(),
+                           MatchType::SUBSTITUTE_BECAUSE_BEST,
+                           suggestedSize);
+        }
+      }
     }
 
     // There's no perfect match, so find the best match we can.
     RefPtr<CachedSurface> bestMatch;
     for (auto iter = ConstIter(); !iter.Done(); iter.Next()) {
       NotNull<CachedSurface*> current = WrapNotNull(iter.UserData());
       const SurfaceKey& currentKey = current->GetSurfaceKey();
 
@@ -336,37 +363,49 @@ public:
                       currentKey.Size())) {
         bestMatch = current;
       }
     }
 
     MatchType matchType;
     if (bestMatch) {
       if (!exactMatch) {
-        // No exact match, but we found a substitute.
-        matchType = MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND;
+        const IntSize& bestMatchSize = bestMatch->GetSurfaceKey().Size();
+        if (mFactor2Mode && suggestedSize == bestMatchSize) {
+          // No exact match, but this is the best we are willing to decode.
+          matchType = MatchType::SUBSTITUTE_BECAUSE_BEST;
+        } else {
+          // No exact match, but we found a substitute.
+          matchType = MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND;
+        }
       } else if (exactMatch != bestMatch) {
         // 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;
+        const IntSize& bestMatchSize = bestMatch->GetSurfaceKey().Size();
+        if (mFactor2Mode && aIdealKey.Size() != bestMatchSize) {
+          // The best factor of 2 match is still decoding.
+          matchType = MatchType::SUBSTITUTE_BECAUSE_BEST;
+        } 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"; 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(bestMatch.forget(), matchType);
+    return MakeTuple(bestMatch.forget(), matchType, suggestedSize);
   }
 
   void MaybeSetFactor2Mode()
   {
     MOZ_ASSERT(!mFactor2Mode);
 
     // Typically an image cache will not have too many size-varying surfaces, so
     // if we exceed the given threshold, we should consider using a subset.
@@ -624,17 +663,17 @@ public:
       }
       return InsertOutcome::FAILURE;
     }
 
     StartTracking(surface, aAutoLock);
     return InsertOutcome::SUCCESS;
   }
 
-  void Remove(NotNull<CachedSurface*> aSurface,
+  bool Remove(NotNull<CachedSurface*> aSurface,
               const StaticMutexAutoLock& aAutoLock)
   {
     ImageKey imageKey = aSurface->GetImageKey();
 
     RefPtr<ImageSurfaceCache> cache = GetImageCache(imageKey);
     MOZ_ASSERT(cache, "Shouldn't try to remove a surface with no image cache");
 
     // If the surface was not a placeholder, tell its image that we discarded it.
@@ -648,17 +687,20 @@ public:
     mCachedSurfacesDiscard.AppendElement(cache->Remove(aSurface));
 
     // Remove the per-image cache if it's unneeded now. Keep it if the image is
     // locked, since the per-image cache is where we store that state. Note that
     // we don't push it into mImageCachesDiscard because all of its surfaces
     // have been removed, so it is safe to free while holding the lock.
     if (cache->IsEmpty() && !cache->IsLocked()) {
       mImageCaches.Remove(imageKey);
+      return true;
     }
+
+    return false;
   }
 
   void StartTracking(NotNull<CachedSurface*> aSurface,
                      const StaticMutexAutoLock& aAutoLock)
   {
     CostEntry costEntry = aSurface->GetCostEntry();
     MOZ_ASSERT(costEntry.GetCost() <= mAvailableCost,
                "Cost too large and the caller didn't catch it");
@@ -758,45 +800,58 @@ public:
     // 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.
 
     RefPtr<CachedSurface> surface;
     DrawableSurface drawableSurface;
     MatchType matchType = MatchType::NOT_FOUND;
+    IntSize suggestedSize;
     while (true) {
-      Tie(surface, matchType) = cache->LookupBestMatch(aSurfaceKey);
+      Tie(surface, matchType, suggestedSize)
+        = cache->LookupBestMatch(aSurfaceKey);
 
       if (!surface) {
         return LookupResult(matchType);  // Lookup in the per-image cache missed.
       }
 
       drawableSurface = surface->GetDrawableSurface();
       if (drawableSurface) {
         break;
       }
 
       // The surface was released by the operating system. Remove the cache
       // entry as well.
-      Remove(WrapNotNull(surface), aAutoLock);
+      if (Remove(WrapNotNull(surface), aAutoLock)) {
+        break;
+      }
     }
 
     MOZ_ASSERT_IF(matchType == MatchType::EXACT,
                   surface->GetSurfaceKey() == aSurfaceKey);
     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) {
+    if (matchType == MatchType::EXACT ||
+        matchType == MatchType::SUBSTITUTE_BECAUSE_BEST) {
       MarkUsed(WrapNotNull(surface), WrapNotNull(cache), aAutoLock);
     }
 
+    // When the caller may choose to decode at an uncached size because there is
+    // no pending decode at the requested size, we should give it the alternative
+    // size it should decode at.
+    if (matchType == MatchType::SUBSTITUTE_BECAUSE_NOT_FOUND ||
+        matchType == MatchType::NOT_FOUND) {
+      return LookupResult(Move(drawableSurface), matchType, suggestedSize);
+    }
+
     return LookupResult(Move(drawableSurface), matchType);
   }
 
   bool CanHold(const Cost aCost) const
   {
     return aCost <= mMaxCost;
   }
 
--- a/image/SurfaceCache.h
+++ b/image/SurfaceCache.h
@@ -63,16 +63,21 @@ public:
   PLDHashNumber Hash() const
   {
     PLDHashNumber hash = HashGeneric(mSize.width, mSize.height);
     hash = AddToHash(hash, mSVGContext.map(HashSIC).valueOr(0));
     hash = AddToHash(hash, uint8_t(mPlayback), uint32_t(mFlags));
     return hash;
   }
 
+  SurfaceKey CloneWithSize(const IntSize& aSize) const
+  {
+    return SurfaceKey(aSize, mSVGContext, mPlayback, mFlags);
+  }
+
   const IntSize& Size() const { return mSize; }
   Maybe<SVGImageContext> SVGContext() const { return mSVGContext; }
   PlaybackType Playback() const { return mPlayback; }
   SurfaceFlags Flags() const { return mFlags; }
 
 private:
   SurfaceKey(const IntSize& aSize,
              const Maybe<SVGImageContext>& aSVGContext,