Bug 1146663 (Part 2) - Remove the concept of lifetimes from the SurfaceCache. r=dholbert

This commit is contained in:
Seth Fowler
2015-09-19 16:20:59 -07:00
parent 19718f2bc4
commit 1c519d9a7f
5 changed files with 64 additions and 92 deletions

View File

@@ -324,8 +324,7 @@ Decoder::AllocateFrameInternal(uint32_t aFrameNum,
SurfaceCache::Insert(frame, ImageKey(mImage.get()),
RasterSurfaceKey(aTargetSize,
mSurfaceFlags,
aFrameNum),
Lifetime::Persistent);
aFrameNum));
if (outcome == InsertOutcome::FAILURE) {
// We couldn't insert the surface, almost certainly due to low memory. We
// treat this as a permanent error to help the system recover; otherwise,

View File

@@ -217,8 +217,7 @@ public:
bool aLastPart) = 0;
/**
* Called when the SurfaceCache discards a persistent surface belonging to
* this image.
* Called when the SurfaceCache discards a surface belonging to this image.
*/
virtual void OnSurfaceDiscarded() = 0;

View File

@@ -135,17 +135,14 @@ public:
CachedSurface(imgFrame* aSurface,
const Cost aCost,
const ImageKey aImageKey,
const SurfaceKey& aSurfaceKey,
const Lifetime aLifetime)
const SurfaceKey& aSurfaceKey)
: mSurface(aSurface)
, mCost(aCost)
, mImageKey(aImageKey)
, mSurfaceKey(aSurfaceKey)
, mLifetime(aLifetime)
{
MOZ_ASSERT(!IsPlaceholder() ||
(mCost == sPlaceholderCost && mLifetime == Lifetime::Transient),
"Placeholder should have trivial cost and transient lifetime");
MOZ_ASSERT(!IsPlaceholder() || mCost == sPlaceholderCost,
"Placeholder should have trivial cost");
MOZ_ASSERT(mImageKey, "Must have a valid image key");
}
@@ -165,7 +162,7 @@ public:
return; // Can't lock a placeholder.
}
if (aLocked && mLifetime == Lifetime::Persistent) {
if (aLocked) {
// This may fail, and that's OK. We make no guarantees about whether
// locking is successful if you call SurfaceCache::LockImage() after
// SurfaceCache::Insert().
@@ -182,7 +179,6 @@ public:
SurfaceKey GetSurfaceKey() const { return mSurfaceKey; }
CostEntry GetCostEntry() { return image::CostEntry(this, mCost); }
nsExpirationState* GetExpirationState() { return &mExpirationState; }
Lifetime GetLifetime() const { return mLifetime; }
bool IsDecoded() const
{
@@ -230,7 +226,6 @@ private:
const Cost mCost;
const ImageKey mImageKey;
const SurfaceKey mSurfaceKey;
const Lifetime mLifetime;
};
/**
@@ -259,9 +254,8 @@ public:
void Insert(const SurfaceKey& aKey, CachedSurface* aSurface)
{
MOZ_ASSERT(aSurface, "Should have a surface");
MOZ_ASSERT(!mLocked || aSurface->GetLifetime() != Lifetime::Persistent ||
aSurface->IsLocked(),
"Inserting an unlocked persistent surface for a locked image");
MOZ_ASSERT(!mLocked || aSurface->IsPlaceholder() || aSurface->IsLocked(),
"Inserting an unlocked surface for a locked image");
mSurfaces.Put(aKey, aSurface);
}
@@ -470,8 +464,7 @@ public:
InsertOutcome Insert(imgFrame* aSurface,
const Cost aCost,
const ImageKey aImageKey,
const SurfaceKey& aSurfaceKey,
Lifetime aLifetime)
const SurfaceKey& aSurfaceKey)
{
// If this is a duplicate surface, refuse to replace the original.
// XXX(seth): Calling Lookup() and then RemoveSurface() does the lookup
@@ -513,12 +506,12 @@ public:
}
nsRefPtr<CachedSurface> surface =
new CachedSurface(aSurface, aCost, aImageKey, aSurfaceKey, aLifetime);
new CachedSurface(aSurface, aCost, aImageKey, aSurfaceKey);
// 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");
// We require that locking succeed if the image is locked and we're not
// inserting a placeholder; the caller may need to know this to handle
// errors correctly.
if (cache->IsLocked() && !surface->IsPlaceholder()) {
surface->SetLocked(true);
if (!surface->IsLocked()) {
return InsertOutcome::FAILURE;
@@ -541,8 +534,8 @@ public:
nsRefPtr<ImageSurfaceCache> cache = GetImageCache(imageKey);
MOZ_ASSERT(cache, "Shouldn't try to remove a surface with no image cache");
// If the surface was persistent, tell its image that we discarded it.
if (aSurface->GetLifetime() == Lifetime::Persistent) {
// If the surface was not a placeholder, tell its image that we discarded it.
if (!aSurface->IsPlaceholder()) {
static_cast<Image*>(imageKey)->OnSurfaceDiscarded();
}
@@ -777,9 +770,8 @@ public:
void DiscardAll()
{
// Remove in order of cost because mCosts is an array and the other data
// structures are all hash tables. Note that locked surfaces (persistent
// surfaces belonging to locked images) are not removed, since they aren't
// present in mCosts.
// structures are all hash tables. Note that locked surfaces are not
// removed, since they aren't present in mCosts.
while (!mCosts.IsEmpty()) {
Remove(mCosts.LastElement().GetSurface());
}
@@ -813,8 +805,7 @@ public:
void LockSurface(CachedSurface* aSurface)
{
if (aSurface->GetLifetime() == Lifetime::Transient ||
aSurface->IsLocked()) {
if (aSurface->IsPlaceholder() || aSurface->IsLocked()) {
return;
}
@@ -837,8 +828,7 @@ public:
CachedSurface* aSurface,
void* aCache)
{
if (aSurface->GetLifetime() == Lifetime::Transient ||
!aSurface->IsLocked()) {
if (aSurface->IsPlaceholder() || !aSurface->IsLocked()) {
return PL_DHASH_NEXT;
}
@@ -921,9 +911,9 @@ private:
// This is similar to CanHold() except that it takes into account the costs of
// locked surfaces. It's used internally in Insert(), but it's not exposed
// publicly because if we start permitting multithreaded access to the surface
// cache, which seems likely, then the result would be meaningless: another
// thread could insert a persistent surface or lock an image at any time.
// publicly because we permit multithreaded access to the surface cache, which
// means that the result would be meaningless: another thread could insert a
// surface or lock an image at any time.
bool CanHoldAfterDiscarding(const Cost aCost) const
{
return aCost <= mMaxCost - mLockedCost;
@@ -1096,8 +1086,7 @@ SurfaceCache::LookupBestMatch(const ImageKey aImageKey,
/* static */ InsertOutcome
SurfaceCache::Insert(imgFrame* aSurface,
const ImageKey aImageKey,
const SurfaceKey& aSurfaceKey,
Lifetime aLifetime)
const SurfaceKey& aSurfaceKey)
{
if (!sInstance) {
return InsertOutcome::FAILURE;
@@ -1110,7 +1099,7 @@ SurfaceCache::Insert(imgFrame* aSurface,
MutexAutoLock lock(sInstance->GetMutex());
Cost cost = ComputeCost(aSurface->GetSize(), aSurface->GetBytesPerPixel());
return sInstance->Insert(aSurface, cost, aImageKey, aSurfaceKey, aLifetime);
return sInstance->Insert(aSurface, cost, aImageKey, aSurfaceKey);
}
/* static */ InsertOutcome
@@ -1122,8 +1111,7 @@ SurfaceCache::InsertPlaceholder(const ImageKey aImageKey,
}
MutexAutoLock lock(sInstance->GetMutex());
return sInstance->Insert(nullptr, sPlaceholderCost, aImageKey, aSurfaceKey,
Lifetime::Transient);
return sInstance->Insert(nullptr, sPlaceholderCost, aImageKey, aSurfaceKey);
}
/* static */ bool

View File

@@ -121,11 +121,6 @@ VectorSurfaceKey(const gfx::IntSize& aSize,
return SurfaceKey(aSize, aSVGContext, aAnimationTime, DefaultSurfaceFlags());
}
enum class Lifetime : uint8_t {
Transient,
Persistent
};
enum class InsertOutcome : uint8_t {
SUCCESS, // Success (but see Insert documentation).
FAILURE, // Couldn't insert (e.g., for capacity reasons).
@@ -146,9 +141,8 @@ enum class InsertOutcome : uint8_t {
* cache. This is most often because losing the data could harm the user
* experience (for example, we often don't want to allow surfaces that are
* currently visible to expire) or because it's not possible to rematerialize
* the surface. SurfaceCache supports this through the use of image locking and
* surface lifetimes; see the comments for Insert() and LockImage() for more
* details.
* the surface. SurfaceCache supports this through the use of image locking; see
* the comments for Insert() and LockImage() for more details.
*
* Any image which stores surfaces in the SurfaceCache *must* ensure that it
* calls RemoveImage() before it is destroyed. See the comments for
@@ -178,8 +172,8 @@ struct SurfaceCache
* If the imgFrame was found in the cache, but had stored its surface in a
* volatile buffer which was discarded by the OS, then it is automatically
* removed from the cache and an empty LookupResult is returned. Note that
* this will never happen to persistent surfaces associated with a locked
* image; the cache keeps a strong reference to such surfaces internally.
* this will never happen to surfaces associated with a locked image; the
* cache keeps a strong reference to such surfaces internally.
*
* @param aImageKey Key data identifying which image the surface belongs
* to.
@@ -236,32 +230,30 @@ struct SurfaceCache
* 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
* happen: the image must become locked again (via LockImage()), and the
* surface must be touched again (via one of the Lookup() functions).
* Surfaces will never expire as long as they remain locked, but if they
* become unlocked, they can expire either because the SurfaceCache runs out
* of capacity or because they've gone too long without being used. When it
* is first inserted, a 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 happen: the image must
* become locked again (via LockImage()), and the surface must be touched
* again (via one of the Lookup() functions).
*
* All of this means that a very particular procedure has to be followed for
* surfaces which cannot be rematerialized. First, they must be inserted
* with a persistent lifetime *after* the image is locked with LockImage(); if
* you use the other order, the surfaces might expire before LockImage() gets
* called or before the surface is touched again by Lookup(). Second, the
* image they are associated with must never be unlocked.
* *after* the image is locked with LockImage(); if you use the other order,
* the surfaces might expire before LockImage() gets called or before the
* surface is touched again by Lookup(). Second, the image they are associated
* with must never be unlocked.
*
* If a surface cannot be rematerialized, it may be important to know whether
* it was inserted into the cache successfully. Insert() returns FAILURE if it
* failed to insert the surface, which could happen because of capacity
* reasons, or because it was already freed by the OS. If you aren't inserting
* a surface with persistent lifetime, or if the surface isn't associated with
* a locked image, checking for SUCCESS or FAILURE is useless: the surface
* might expire immediately after being inserted, even though Insert()
* returned SUCCESS. Thus, many callers do not need to check the result of
* Insert() at all.
* reasons, or because it was already freed by the OS. If the surface isn't
* associated with a locked image, checking for SUCCESS or FAILURE is useless:
* the surface might expire immediately after being inserted, even though
* Insert() returned SUCCESS. Thus, many callers do not need to check the
* result of Insert() at all.
*
* @param aTarget The new surface (wrapped in an imgFrame) to insert into
* the cache.
@@ -269,9 +261,6 @@ struct SurfaceCache
* to.
* @param aSurfaceKey Key data which uniquely identifies the requested
* surface.
* @param aLifetime Whether this is a transient surface that can always be
* allowed to expire, or a persistent surface that
* shouldn't expire if the image is locked.
* @return SUCCESS if the surface was inserted successfully. (But see above
* for more information about when you should check this.)
* FAILURE if the surface could not be inserted, e.g. for capacity
@@ -282,8 +271,7 @@ struct SurfaceCache
*/
static InsertOutcome Insert(imgFrame* aSurface,
const ImageKey aImageKey,
const SurfaceKey& aSurfaceKey,
Lifetime aLifetime);
const SurfaceKey& aSurfaceKey);
/**
* Insert a placeholder for a surface into the cache. If a surface with the
@@ -333,17 +321,16 @@ struct SurfaceCache
static bool CanHold(size_t aSize);
/**
* Locks an image. Any of the image's persistent surfaces which are either
* inserted or accessed while the image is locked will not expire.
* Locks an image. Any of the image's surfaces which are either inserted or
* accessed while the image is locked will not expire.
*
* Locking an image does not automatically lock that image's existing
* surfaces. A call to LockImage() guarantees that persistent surfaces which
* are inserted afterward will not expire before the next call to
* UnlockImage() or UnlockSurfaces() for that image. Surfaces that are
* accessed via Lookup() or LookupBestMatch() after a LockImage() call will
* also not expire until the next UnlockImage() or UnlockSurfaces() call for
* that image. Any other surfaces owned by the image may expire at any time,
* whether they are persistent or transient.
* surfaces. A call to LockImage() guarantees that surfaces which are inserted
* afterward will not expire before the next call to UnlockImage() or
* UnlockSurfaces() for that image. Surfaces that are accessed via Lookup() or
* LookupBestMatch() after a LockImage() call will also not expire until the
* next UnlockImage() or UnlockSurfaces() call for that image. Any other
* surfaces owned by the image may expire at any time.
*
* Regardless of locking, any of an image's surfaces may be removed using
* RemoveSurface(), and all of an image's surfaces are removed by
@@ -381,10 +368,10 @@ struct SurfaceCache
* expiring.
*
* This is intended to be used in situations where it's no longer clear that
* all of the persistent surfaces owned by an image are needed. Calling
* UnlockSurfaces() and then taking some action that will cause Lookup() to
* touch any surfaces that are still useful will permit the remaining surfaces
* to expire from the cache.
* all of the surfaces owned by an image are needed. Calling UnlockSurfaces()
* and then taking some action that will cause Lookup() to touch any surfaces
* that are still useful will permit the remaining surfaces to expire from the
* cache.
*
* If the image is unlocked, this has no effect.
*
@@ -425,9 +412,9 @@ struct SurfaceCache
/**
* Evicts all evictable surfaces from the cache.
*
* All surfaces are evictable except for persistent surfaces associated with
* locked images. Non-evictable surfaces can only be removed by
* RemoveSurface() or RemoveImage().
* All surfaces are evictable except for surfaces associated with locked
* images. Non-evictable surfaces can only be removed by RemoveSurface() or
* RemoveImage().
*/
static void DiscardAll();

View File

@@ -920,8 +920,7 @@ VectorImage::CreateSurfaceAndShow(const SVGDrawingParameters& aParams)
SurfaceCache::Insert(frame, ImageKey(this),
VectorSurfaceKey(aParams.size,
aParams.svgContext,
aParams.animationTime),
Lifetime::Persistent);
aParams.animationTime));
// Draw.
nsRefPtr<gfxDrawable> drawable =