Bug 1367207 part 2 - Update mFrameToRequestMap/mRequestToFrameMap more efficiently on frame removal. r=dholbert
authorMats Palmgren <mats@mozilla.com>
Wed, 07 Jun 2017 15:22:41 +0200
changeset 410809 6d87a5768954e46ed37af5d46f7ac8fa14a75135
parent 410808 67181c32580ee06fa53f0fdbd8418cf91496ea13
child 410810 7307d037a9227f8441674713dd754ac893d243d8
push id7391
push usermtabara@mozilla.com
push dateMon, 12 Jun 2017 13:08:53 +0000
treeherdermozilla-beta@2191d7f87e2e [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdholbert
bugs1367207
milestone55.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 1367207 part 2 - Update mFrameToRequestMap/mRequestToFrameMap more efficiently on frame removal. r=dholbert Specifically: * Factor out updating mFrameToRequestMap/mRequestToFrameMap to helper functions. * Deal with mFrameToRequestMap directly in DropRequestsForFrame to avoid a second hashtable Get(). * Use RemoveAndForget() to remove the entry upfront and iterate that instead of making an nsTArray copy. And don't remove entries from the array while iterating - the array will be deleted when the nsAutoPtr goes out of scope. MozReview-Commit-ID: 7Z7FMXMvM14
layout/style/ImageLoader.cpp
layout/style/ImageLoader.h
--- a/layout/style/ImageLoader.cpp
+++ b/layout/style/ImageLoader.cpp
@@ -131,72 +131,80 @@ ImageLoader::MaybeRegisterCSSImage(Image
 
 void
 ImageLoader::DeregisterCSSImage(ImageLoader::Image* aImage)
 {
   RemoveImage(aImage);
 }
 
 void
-ImageLoader::DisassociateRequestFromFrame(imgIRequest* aRequest,
-                                          nsIFrame* aFrame)
+ImageLoader::RemoveRequestToFrameMapping(imgIRequest* aRequest,
+                                         nsIFrame*    aFrame)
 {
-  FrameSet* frameSet = nullptr;
-  RequestSet* requestSet = nullptr;
-
 #ifdef DEBUG
   {
     nsCOMPtr<imgINotificationObserver> observer;
     aRequest->GetNotificationObserver(getter_AddRefs(observer));
     MOZ_ASSERT(!observer || observer == this);
   }
 #endif
 
+  FrameSet* frameSet = nullptr;
   mRequestToFrameMap.Get(aRequest, &frameSet);
-  mFrameToRequestMap.Get(aFrame, &requestSet);
-
   if (frameSet) {
     frameSet->RemoveElementSorted(aFrame);
-  }
-  if (requestSet) {
-    requestSet->RemoveElementSorted(aRequest);
-  }
+    if (frameSet->IsEmpty()) {
+      mRequestToFrameMap.Remove(aRequest);
 
-  if (frameSet && !frameSet->Length()) {
-    mRequestToFrameMap.Remove(aRequest);
-
-    nsPresContext* presContext = GetPresContext();
-    if (presContext) {
-      nsLayoutUtils::DeregisterImageRequest(presContext,
-                                            aRequest,
-                                            nullptr);
+      nsPresContext* presContext = GetPresContext();
+      if (presContext) {
+        nsLayoutUtils::DeregisterImageRequest(presContext, aRequest, nullptr);
+      }
     }
   }
+}
 
-  if (requestSet && !requestSet->Length()) {
-    mFrameToRequestMap.Remove(aFrame);
-    aFrame->SetHasImageRequest(false);
+void
+ImageLoader::RemoveFrameToRequestMapping(imgIRequest* aRequest,
+                                         nsIFrame*    aFrame)
+{
+  RequestSet* requestSet = nullptr;
+  mFrameToRequestMap.Get(aFrame, &requestSet);
+  if (requestSet) {
+    MOZ_ASSERT(aFrame->HasImageRequest(), "HasImageRequest is lying");
+    requestSet->RemoveElementSorted(aRequest);
+    if (requestSet->IsEmpty()) {
+      mFrameToRequestMap.Remove(aFrame);
+      aFrame->SetHasImageRequest(false);
+    }
   }
 }
 
 void
+ImageLoader::DisassociateRequestFromFrame(imgIRequest* aRequest,
+                                          nsIFrame*    aFrame)
+{
+  MOZ_ASSERT(aFrame->HasImageRequest(), "why call me?");
+  RemoveRequestToFrameMapping(aRequest, aFrame);
+  RemoveFrameToRequestMapping(aRequest, aFrame);
+}
+
+void
 ImageLoader::DropRequestsForFrame(nsIFrame* aFrame)
 {
-  RequestSet* requestSet = nullptr;
-  if (!mFrameToRequestMap.Get(aFrame, &requestSet)) {
+  MOZ_ASSERT(aFrame->HasImageRequest(), "why call me?");
+  nsAutoPtr<RequestSet> requestSet;
+  mFrameToRequestMap.RemoveAndForget(aFrame, requestSet);
+  aFrame->SetHasImageRequest(false);
+  if (MOZ_UNLIKELY(!requestSet)) {
+    MOZ_ASSERT_UNREACHABLE("HasImageRequest was lying");
     return;
   }
-
-  NS_ASSERTION(requestSet, "This should never be null");
-
-  RequestSet frozenRequestSet(*requestSet);
-  for (RequestSet::size_type i = frozenRequestSet.Length(); i != 0; --i) {
-    imgIRequest* request = frozenRequestSet.ElementAt(i - 1);
-
-    DisassociateRequestFromFrame(request, aFrame);
+  for (imgIRequest* request : *requestSet) {
+    RemoveRequestToFrameMapping(request, aFrame);
   }
 }
 
 void
 ImageLoader::SetAnimationMode(uint16_t aMode)
 {
   NS_ASSERTION(aMode == imgIContainer::kNormalAnimMode ||
                aMode == imgIContainer::kDontAnimMode ||
--- a/layout/style/ImageLoader.h
+++ b/layout/style/ImageLoader.h
@@ -95,16 +95,20 @@ private:
 
   void DoRedraw(FrameSet* aFrameSet, bool aForcePaint);
 
   nsresult OnSizeAvailable(imgIRequest* aRequest, imgIContainer* aImage);
   nsresult OnFrameComplete(imgIRequest* aRequest);
   nsresult OnImageIsAnimated(imgIRequest* aRequest);
   nsresult OnFrameUpdate(imgIRequest* aRequest);
 
+  // Helpers for DropRequestsForFrame / DisassociateRequestFromFrame above.
+  void RemoveRequestToFrameMapping(imgIRequest* aRequest, nsIFrame* aFrame);
+  void RemoveFrameToRequestMapping(imgIRequest* aRequest, nsIFrame* aFrame);
+
   // A map of imgIRequests to the nsIFrames that are using them.
   RequestToFrameMap mRequestToFrameMap;
 
   // A map of nsIFrames to the imgIRequests they use.
   FrameToRequestMap mFrameToRequestMap;
 
   // A weak pointer to our document. Nulled out by DropDocumentReference.
   nsIDocument* mDocument;