Bug 1396870 P2 Avoid dirtying when removing front entry or when the queue is empty. r=tnikkel
authorBen Kelly <ben@wanderview.com>
Tue, 05 Sep 2017 16:20:18 -0700
changeset 379030 5ac7748ad699020b6f0cd9637ef281baf0c52558
parent 379029 c3218dfedd68920a8a33479c1a883042e9199d92
child 379031 eb36a2188f6f4eb40f092561f9020ec23bcda712
push id94562
push userbkelly@mozilla.com
push dateTue, 05 Sep 2017 23:20:25 +0000
treeherdermozilla-inbound@eb36a2188f6f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerstnikkel
bugs1396870
milestone57.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1396870 P2 Avoid dirtying when removing front entry or when the queue is empty. r=tnikkel
image/imgLoader.cpp
--- a/image/imgLoader.cpp
+++ b/image/imgLoader.cpp
@@ -971,21 +971,47 @@ imgCacheQueue::GetSize() const
 
 #include <algorithm>
 using namespace std;
 
 void
 imgCacheQueue::Remove(imgCacheEntry* entry)
 {
   auto it = find(mQueue.begin(), mQueue.end(), entry);
-  if (it != mQueue.end()) {
-    mSize -= (*it)->GetDataSize();
-    mQueue.erase(it);
-    MarkDirty();
+  if (it == mQueue.end()) {
+    return;
+  }
+
+  mSize -= (*it)->GetDataSize();
+
+  // If the queue is clean and this is the first entry,
+  // then we can efficiently remove the entry without
+  // dirtying the sort order.
+  if (!IsDirty() && it == mQueue.begin()) {
+    std::pop_heap(mQueue.begin(), mQueue.end(),
+                  imgLoader::CompareCacheEntries);
+    mQueue.pop_back();
+    return;
   }
+
+  // Remove from the middle of the list.  This potentially
+  // breaks the binary heap sort order.
+  mQueue.erase(it);
+
+  // If we only have one entry or the queue is empty, though,
+  // then the sort order is still effectively good.  Simply
+  // refresh the list to clear the dirty flag.
+  if (mQueue.size() <= 1) {
+    Refresh();
+    return;
+  }
+
+  // Otherwise we must mark the queue dirty and potentially
+  // trigger an expensive sort later.
+  MarkDirty();
 }
 
 void
 imgCacheQueue::Push(imgCacheEntry* entry)
 {
   mSize += entry->GetDataSize();
 
   RefPtr<imgCacheEntry> refptr(entry);
@@ -1615,17 +1641,22 @@ imgLoader::SetHasProxies(imgRequest* aRe
 
   return false;
 }
 
 void
 imgLoader::CacheEntriesChanged(bool aForChrome, int32_t aSizeDiff /* = 0 */)
 {
   imgCacheQueue& queue = GetCacheQueue(aForChrome);
-  queue.MarkDirty();
+  // We only need to dirty the queue if there is any sorting
+  // taking place.  Empty or single-entry lists can't become
+  // dirty.
+  if (queue.GetNumElements() > 1) {
+    queue.MarkDirty();
+  }
   queue.UpdateSize(aSizeDiff);
 }
 
 void
 imgLoader::CheckCacheLimits(imgCacheTable& cache, imgCacheQueue& queue)
 {
   if (queue.GetNumElements() == 0) {
     NS_ASSERTION(queue.GetSize() == 0,