Bug 1413833 - Don't use WeakFrame for the modified frame list since get slow with large numbers of frames. r=miko
authorMatt Woodrow <mwoodrow@mozilla.com>
Wed, 08 Nov 2017 15:25:44 +1300
changeset 441499 8bd69298b447dc9c5cb11a77f0d14afdb8f9a990
parent 441498 e05d3e232865dbb2533966879f50db54daa7340e
child 441500 7c87b438511359c8792273d261d3bed546c9b530
push id8130
push userryanvm@gmail.com
push dateThu, 09 Nov 2017 00:28:20 +0000
treeherdermozilla-beta@a49a6fa54363 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmiko
bugs1413833
milestone58.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 1413833 - Don't use WeakFrame for the modified frame list since get slow with large numbers of frames. r=miko
layout/generic/nsFrame.cpp
layout/generic/nsIFrame.h
layout/painting/RetainedDisplayListBuilder.cpp
--- a/layout/generic/nsFrame.cpp
+++ b/layout/generic/nsFrame.cpp
@@ -964,16 +964,32 @@ nsIFrame::RemoveDisplayItemDataForDeleti
       if (i->GetDependentFrame() == this &&
           !i->HasDeletedFrame()) {
         i->Frame()->MarkNeedsDisplayItemRebuild();
       }
       i->RemoveFrame(this);
     }
     delete items;
   }
+
+  if (IsFrameModified()) {
+    nsIFrame* rootFrame = PresContext()->PresShell()->GetRootFrame();
+    MOZ_ASSERT(rootFrame);
+
+    nsTArray<nsIFrame*>* modifiedFrames =
+      rootFrame->GetProperty(nsIFrame::ModifiedFrameList());
+    MOZ_ASSERT(modifiedFrames);
+
+    for (auto& frame : *modifiedFrames) {
+      if (frame == this) {
+        frame = nullptr;
+        break;
+      }
+    }
+  }
 }
 
 void
 nsIFrame::MarkNeedsDisplayItemRebuild()
 {
   if (!gfxPrefs::LayoutRetainDisplayList() || !XRE_IsContentProcess() ||
       IsFrameModified()) {
     // Skip non-content processes and frames that are already marked modified.
@@ -992,49 +1008,49 @@ nsIFrame::MarkNeedsDisplayItemRebuild()
 
   nsIFrame* rootFrame = PresContext()->PresShell()->GetRootFrame();
   MOZ_ASSERT(rootFrame);
 
   if (rootFrame->IsFrameModified()) {
     return;
   }
 
-  std::vector<WeakFrame>* modifiedFrames =
+  nsTArray<nsIFrame*>* modifiedFrames =
     rootFrame->GetProperty(nsIFrame::ModifiedFrameList());
 
   if (!modifiedFrames) {
-    modifiedFrames = new std::vector<WeakFrame>();
+    modifiedFrames = new nsTArray<nsIFrame*>();
     rootFrame->SetProperty(nsIFrame::ModifiedFrameList(), modifiedFrames);
   }
 
   if (this == rootFrame) {
     // If this is the root frame, then marking us as needing a display
     // item rebuild implies the same for all our descendents. Clear them
-    // all out to reduce the number of WeakFrames we keep around.
+    // all out to reduce the number of modified frames we keep around.
     for (nsIFrame* f : *modifiedFrames) {
       if (f) {
         f->SetFrameIsModified(false);
       }
     }
-    modifiedFrames->clear();
-  } else if (modifiedFrames->size() > gfxPrefs::LayoutRebuildFrameLimit()) {
+    modifiedFrames->Clear();
+  } else if (modifiedFrames->Length() > gfxPrefs::LayoutRebuildFrameLimit()) {
     // If the list starts getting too big, then just mark the root frame
     // as needing a rebuild.
     rootFrame->MarkNeedsDisplayItemRebuild();
     return;
   }
 
-  modifiedFrames->emplace_back(this);
+  modifiedFrames->AppendElement(this);
 
   // TODO: this is a bit of a hack. We are using ModifiedFrameList property to
   // decide whether we are trying to reuse the display list.
   if (displayRoot != rootFrame &&
       !displayRoot->HasProperty(nsIFrame::ModifiedFrameList())) {
     displayRoot->SetProperty(nsIFrame::ModifiedFrameList(),
-                             new std::vector<WeakFrame>());
+                             new nsTArray<nsIFrame*>());
   }
 
   MOZ_ASSERT(PresContext()->LayoutPhaseCount(eLayoutPhase_DisplayListBuilding) == 0);
   SetFrameIsModified(true);
 
   // Hopefully this is cheap, but we could use a frame state bit to note
   // the presence of dependencies to speed it up.
   DisplayItemArray* items = GetProperty(DisplayItems());
--- a/layout/generic/nsIFrame.h
+++ b/layout/generic/nsIFrame.h
@@ -1252,17 +1252,17 @@ public:
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(FragStretchBSizeProperty, nscoord)
 
   // The block-axis margin-box size associated with eBClampMarginBoxMinSize.
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(BClampMarginBoxMinSizeProperty, nscoord)
 
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(IBaselinePadProperty, nscoord)
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(BBaselinePadProperty, nscoord)
 
-  NS_DECLARE_FRAME_PROPERTY_DELETABLE(ModifiedFrameList, std::vector<WeakFrame>)
+  NS_DECLARE_FRAME_PROPERTY_DELETABLE(ModifiedFrameList, nsTArray<nsIFrame*>)
   NS_DECLARE_FRAME_PROPERTY_DELETABLE(DisplayItems, DisplayItemArray)
 
   NS_DECLARE_FRAME_PROPERTY_SMALL_VALUE(BidiDataProperty, mozilla::FrameBidiData)
 
   NS_DECLARE_FRAME_PROPERTY_WITHOUT_DTOR(PlaceholderFrameProperty, nsPlaceholderFrame)
   NS_DECLARE_FRAME_PROPERTY_WITH_DTOR(WebRenderUserDataProperty, WebRenderUserDataTable, DestroyWebRenderUserDataTable)
 
   static void DestroyWebRenderUserDataTable(WebRenderUserDataTable* aTable);
--- a/layout/painting/RetainedDisplayListBuilder.cpp
+++ b/layout/painting/RetainedDisplayListBuilder.cpp
@@ -467,32 +467,30 @@ RetainedDisplayListBuilder::MergeDisplay
 }
 
 static void
 TakeAndAddModifiedFramesFromRootFrame(nsTArray<nsIFrame*>& aFrames,
                                       nsIFrame* aRootFrame)
 {
   MOZ_ASSERT(aRootFrame);
 
-  std::vector<WeakFrame>* frames =
+  nsTArray<nsIFrame*>* frames =
     aRootFrame->GetProperty(nsIFrame::ModifiedFrameList());
 
   if (!frames) {
     return;
   }
 
-  for (WeakFrame& frame : *frames) {
-    nsIFrame* f = frame.GetFrame();
-
+  for (nsIFrame* f : *frames) {
     if (f) {
       aFrames.AppendElement(f);
     }
   }
 
-  frames->clear();
+  frames->Clear();
 }
 
 static bool
 SubDocEnumCb(nsIDocument* aDocument, void* aData)
 {
   MOZ_ASSERT(aDocument);
   MOZ_ASSERT(aData);