From 0e66b7f2bd7b8afb575531f5c74182392f4c1fb5 Mon Sep 17 00:00:00 2001 From: Shih-Chiang Chien Date: Mon, 16 Dec 2013 19:31:00 +0800 Subject: [PATCH] Bug 878577 - Part 1: Setup hard limit for DiscardTracker. r=seth --- b2g/app/b2g.js | 2 ++ image/src/DiscardTracker.cpp | 42 +++++++++++++++++++++++++-------- image/src/DiscardTracker.h | 21 ++++++++++++----- image/src/imgFrame.cpp | 23 ++++++++++-------- modules/libpref/src/init/all.js | 4 ++++ 5 files changed, 66 insertions(+), 26 deletions(-) diff --git a/b2g/app/b2g.js b/b2g/app/b2g.js index 614d40ba2e93..e242138bed5b 100644 --- a/b2g/app/b2g.js +++ b/b2g/app/b2g.js @@ -305,6 +305,8 @@ pref("image.mem.decodeondraw", true); pref("image.mem.allow_locking_in_content_processes", false); /* don't allow image locking */ pref("image.mem.min_discard_timeout_ms", 86400000); /* 24h, we rely on the out of memory hook */ pref("image.mem.max_decoded_image_kb", 30000); /* 30MB seems reasonable */ +// 65MB seems reasonable and layout/reftests/bugs/370629-1.html requires more than 62MB +pref("image.mem.hard_limit_decoded_image_kb", 66560); pref("image.onload.decode.limit", 24); /* don't decode more than 24 images eagerly */ // XXX this isn't a good check for "are touch events supported", but diff --git a/image/src/DiscardTracker.cpp b/image/src/DiscardTracker.cpp index a10e337699fa..1772948e4314 100644 --- a/image/src/DiscardTracker.cpp +++ b/image/src/DiscardTracker.cpp @@ -19,9 +19,10 @@ static const char* sDiscardTimeoutPref = "image.mem.min_discard_timeout_ms"; /* static */ bool DiscardTracker::sInitialized = false; /* static */ bool DiscardTracker::sTimerOn = false; /* static */ Atomic DiscardTracker::sDiscardRunnablePending(false); -/* static */ int64_t DiscardTracker::sCurrentDecodedImageBytes = 0; +/* static */ uint64_t DiscardTracker::sCurrentDecodedImageBytes = 0; /* static */ uint32_t DiscardTracker::sMinDiscardTimeoutMs = 10000; /* static */ uint32_t DiscardTracker::sMaxDecodedImageKB = 42 * 1024; +/* static */ uint32_t DiscardTracker::sHardLimitDecodedImageKB = 0; /* static */ PRLock * DiscardTracker::sAllocationLock = nullptr; /* static */ mozilla::Mutex* DiscardTracker::sNodeListMutex = nullptr; /* static */ Atomic DiscardTracker::sShutdown(false); @@ -138,21 +139,39 @@ DiscardTracker::DiscardAll() DisableTimer(); } -void -DiscardTracker::InformAllocation(int64_t bytes) +/* static */ bool +DiscardTracker::TryAllocation(uint64_t aBytes) +{ + MOZ_ASSERT(sInitialized); + + PR_Lock(sAllocationLock); + bool enoughSpace = + !sHardLimitDecodedImageKB || + (sHardLimitDecodedImageKB * 1024) - sCurrentDecodedImageBytes >= aBytes; + + if (enoughSpace) { + sCurrentDecodedImageBytes += aBytes; + } + PR_Unlock(sAllocationLock); + + // If we're using too much memory for decoded images, MaybeDiscardSoon will + // enqueue a callback to discard some images. + MaybeDiscardSoon(); + + return enoughSpace; +} + +/* static */ void +DiscardTracker::InformDeallocation(uint64_t aBytes) { // This function is called back e.g. from RasterImage::Discard(); be careful! MOZ_ASSERT(sInitialized); PR_Lock(sAllocationLock); - sCurrentDecodedImageBytes += bytes; - MOZ_ASSERT(sCurrentDecodedImageBytes >= 0); + MOZ_ASSERT(aBytes <= sCurrentDecodedImageBytes); + sCurrentDecodedImageBytes -= aBytes; PR_Unlock(sAllocationLock); - - // If we're using too much memory for decoded images, MaybeDiscardSoon will - // enqueue a callback to discard some images. - MaybeDiscardSoon(); } /** @@ -169,6 +188,9 @@ DiscardTracker::Initialize() "image.mem.max_decoded_image_kb", 50 * 1024); + Preferences::AddUintVarCache(&sHardLimitDecodedImageKB, + "image.mem.hard_limit_decoded_image_kb", + 0); // Create the timer. sTimer = do_CreateInstance("@mozilla.org/timer;1"); @@ -278,7 +300,7 @@ DiscardTracker::DiscardNow() sCurrentDecodedImageBytes > sMaxDecodedImageKB * 1024) { // Discarding the image should cause sCurrentDecodedImageBytes to - // decrease via a call to InformAllocation(). + // decrease via a call to InformDeallocation(). node->img->Discard(); // Careful: Discarding may have caused the node to have been removed diff --git a/image/src/DiscardTracker.h b/image/src/DiscardTracker.h index 70dd78a5d5c4..7b11b9f503a4 100644 --- a/image/src/DiscardTracker.h +++ b/image/src/DiscardTracker.h @@ -82,12 +82,20 @@ class DiscardTracker static void DiscardAll(); /** - * Inform the discard tracker that we've allocated or deallocated some - * memory for a decoded image. We use this to determine when we've - * allocated too much memory and should discard some images. This function - * can be called from any thread and is thread-safe. + * Inform the discard tracker that we are going to allocate some memory + * for a decoded image. We use this to determine when we've allocated + * too much memory and should discard some images. This function can be + * called from any thread and is thread-safe. If this function succeeds, the + * caller is now responsible for ensuring that InformDeallocation is called. */ - static void InformAllocation(int64_t bytes); + static bool TryAllocation(uint64_t aBytes); + + /** + * Inform the discard tracker that we've deallocated some memory for a + * decoded image. This function can be called from any thread and is + * thread-safe. + */ + static void InformDeallocation(uint64_t aBytes); private: /** @@ -116,9 +124,10 @@ class DiscardTracker static bool sInitialized; static bool sTimerOn; static mozilla::Atomic sDiscardRunnablePending; - static int64_t sCurrentDecodedImageBytes; + static uint64_t sCurrentDecodedImageBytes; static uint32_t sMinDiscardTimeoutMs; static uint32_t sMaxDecodedImageKB; + static uint32_t sHardLimitDecodedImageKB; // Lock for safegarding the 64-bit sCurrentDecodedImageBytes static PRLock *sAllocationLock; static mozilla::Mutex* sNodeListMutex; diff --git a/image/src/imgFrame.cpp b/image/src/imgFrame.cpp index f5a3803a524a..a5aa408fd98a 100644 --- a/image/src/imgFrame.cpp +++ b/image/src/imgFrame.cpp @@ -170,7 +170,7 @@ imgFrame::~imgFrame() mPalettedImageData = nullptr; if (mInformedDiscardTracker) { - DiscardTracker::InformAllocation(-4 * mSize.height * mSize.width); + DiscardTracker::InformDeallocation(4 * mSize.height * mSize.width); } } @@ -203,6 +203,11 @@ nsresult imgFrame::Init(int32_t aX, int32_t aY, int32_t aWidth, int32_t aHeight, NS_WARNING("moz_malloc for paletted image data should succeed"); NS_ENSURE_TRUE(mPalettedImageData, NS_ERROR_OUT_OF_MEMORY); } else { + // Inform the discard tracker that we are going to allocate some memory. + if (!DiscardTracker::TryAllocation(4 * mSize.width * mSize.height)) { + NS_WARNING("Exceed the hard limit of decode image size"); + return NS_ERROR_OUT_OF_MEMORY; + } // For Windows, we must create the device surface first (if we're // going to) so that the image surface can wrap it. Can't be done // the other way around. @@ -238,9 +243,15 @@ nsresult imgFrame::Init(int32_t aX, int32_t aY, int32_t aWidth, int32_t aHeight, } else if (!mImageSurface->CairoStatus()) { NS_WARNING("gfxImageSurface should have good CairoStatus"); } + + // Image surface allocation is failed, need to return + // the booked buffer size. + DiscardTracker::InformDeallocation(4 * mSize.width * mSize.height); return NS_ERROR_OUT_OF_MEMORY; } + mInformedDiscardTracker = true; + #ifdef XP_MACOSX if (!ShouldUseImageSurfaces()) { mQuartzSurface = new gfxQuartzImageSurface(mImageSurface); @@ -248,14 +259,6 @@ nsresult imgFrame::Init(int32_t aX, int32_t aY, int32_t aWidth, int32_t aHeight, #endif } - // Inform the discard tracker that we've allocated some memory, but only if - // we're not a paletted image (paletted images are not usually large and are - // used only for animated frames, which we don't discard). - if (!mPalettedImageData) { - DiscardTracker::InformAllocation(4 * mSize.width * mSize.height); - mInformedDiscardTracker = true; - } - return NS_OK; } @@ -314,7 +317,7 @@ nsresult imgFrame::Optimize() // We just dumped most of our allocated memory, so tell the discard // tracker that we're not using any at all. if (mInformedDiscardTracker) { - DiscardTracker::InformAllocation(-4 * mSize.width * mSize.height); + DiscardTracker::InformDeallocation(4 * mSize.width * mSize.height); mInformedDiscardTracker = false; } diff --git a/modules/libpref/src/init/all.js b/modules/libpref/src/init/all.js index 70af6e1edd8f..5bb154ee0984 100644 --- a/modules/libpref/src/init/all.js +++ b/modules/libpref/src/init/all.js @@ -3918,6 +3918,10 @@ pref("image.mem.max_ms_before_yield", 5); // might keep around more than this, but we'll try to get down to this value). pref("image.mem.max_decoded_image_kb", 51200); +// Hard limit for the amount of decoded image data, 0 means we don't have the +// hard limit for it. +pref("image.mem.hard_limit_decoded_image_kb", 0); + // Minimum timeout for expiring unused images from the surface cache, in // milliseconds. This controls how long we store cached temporary surfaces. pref("image.mem.surfacecache.min_expiration_ms", 60000); // 60ms