Skip to content
This repository has been archived by the owner on Aug 4, 2022. It is now read-only.

Commit

Permalink
Bug 1176124 (Part 2) - Add placeholder support to the SurfaceCache so…
Browse files Browse the repository at this point in the history 
… we can avoid launching redundant decoders. r=dholbert
  • Loading branch information
@sethfowler
sethfowler committed Jul 20, 2015
1 parent f811aad commit 3584ac3
Show file tree
Hide file tree
Showing 3 changed files with 156 additions and 29 deletions.
20 changes: 18 additions & 2 deletions image/RasterImage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -526,9 +526,12 @@ RasterImage::LookupFrame(uint32_t aFrameNum,
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);
Expand Down Expand Up @@ -1500,6 +1503,19 @@ RasterImage::CreateDecoder(const Maybe<IntSize>& aSize, uint32_t aFlags)
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);
Expand Down
128 changes: 105 additions & 23 deletions image/SurfaceCache.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#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"
Expand Down Expand Up @@ -67,6 +68,10 @@ static StaticRefPtr<SurfaceCacheImpl> sInstance;
*/
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)
{
Expand Down Expand Up @@ -137,17 +142,28 @@ class CachedSurface
, 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
Expand All @@ -158,14 +174,19 @@ class CachedSurface
}
}

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
Expand Down Expand Up @@ -262,21 +283,47 @@ class ImageSurfaceCache
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)
Expand Down Expand Up @@ -308,6 +355,11 @@ class ImageSurfaceCache
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()) {
Expand Down Expand Up @@ -425,10 +477,21 @@ class SurfaceCacheImpl final : public nsIMemoryReporter
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))) {
Expand Down Expand Up @@ -458,6 +521,7 @@ class SurfaceCacheImpl final : public nsIMemoryReporter
// 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;
Expand Down Expand Up @@ -560,6 +624,10 @@ class SurfaceCacheImpl final : public nsIMemoryReporter
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
Expand Down Expand Up @@ -595,11 +663,16 @@ class SurfaceCacheImpl final : public nsIMemoryReporter

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();
Expand All @@ -612,21 +685,17 @@ class SurfaceCacheImpl final : public nsIMemoryReporter
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);
}

Expand Down Expand Up @@ -1048,6 +1117,19 @@ SurfaceCache::Insert(imgFrame* aSurface,
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 */)
{
Expand Down
37 changes: 33 additions & 4 deletions image/SurfaceCache.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ struct SurfaceCache
/**
* 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
Expand Down Expand Up @@ -280,6 +281,33 @@ struct SurfaceCache
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
Expand Down Expand Up @@ -362,8 +390,8 @@ struct SurfaceCache
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
Expand All @@ -378,8 +406,9 @@ struct SurfaceCache
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
Expand Down

0 comments on commit 3584ac3

Please sign in to comment.