Bug 1440177 - Part 6: Don't dereference display items during AddPaintedDisplayItem for the LAYER_NONE case. r=jnicol
authorMatt Woodrow <mwoodrow@mozilla.com>
Fri, 02 Mar 2018 11:19:17 +1300
changeset 461225 58025bcb77cbc6505af35f4ac4e0cd8a0878841b
parent 461224 4c62cca5f3ecb2ee93b4d50a4c9427b3fa8f8ccd
child 461226 114d99e1c41c8bf49633d331cf3edbe236c2b4a5
push id1683
push usersfraser@mozilla.com
push dateThu, 26 Apr 2018 16:43:40 +0000
treeherdermozilla-release@5af6cb21869d [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjnicol
bugs1440177
milestone60.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 1440177 - Part 6: Don't dereference display items during AddPaintedDisplayItem for the LAYER_NONE case. r=jnicol The display items are almost certainly gone from the cache at this point, so dereferencing them can take a while. This moves the DisplayItemData lookup, and the IsReused/HasMergedFrames checks into the ProcessDisplayItems pass (where we already use the items), and then just uses the AssignedDisplayItem entry for everything. MozReview-Commit-ID: 8udcE0bmyF3
layout/painting/FrameLayerBuilder.cpp
layout/painting/FrameLayerBuilder.h
--- a/layout/painting/FrameLayerBuilder.cpp
+++ b/layout/painting/FrameLayerBuilder.cpp
@@ -192,33 +192,48 @@ DisplayItemData::EndUpdate(nsAutoPtr<nsD
   mItem = nullptr;
   EndUpdate();
 }
 
 void
 DisplayItemData::BeginUpdate(Layer* aLayer, LayerState aState,
                              nsDisplayItem* aItem /* = nullptr */)
 {
+  BeginUpdate(aLayer, aState, aItem,
+              aItem ? aItem->IsReused() : false,
+              aItem ? aItem->HasMergedFrames() : false);
+}
+
+void
+DisplayItemData::BeginUpdate(Layer* aLayer, LayerState aState,
+                             nsDisplayItem* aItem, bool aIsReused,
+                             bool aIsMerged)
+{
   MOZ_RELEASE_ASSERT(mLayer);
   MOZ_RELEASE_ASSERT(aLayer);
   mLayer = aLayer;
   mOptLayer = nullptr;
   mInactiveManager = nullptr;
   mLayerState = aState;
   mUsed = true;
 
   if (aLayer->AsPaintedLayer()) {
     mItem = aItem;
-    mReusedItem = aItem->IsReused();
+    mReusedItem = aIsReused;
   }
 
   if (!aItem) {
     return;
   }
 
+  if (!aIsMerged && mFrameList.Length() == 1) {
+    MOZ_ASSERT(mFrameList[0] == aItem->Frame());
+    return;
+  }
+
   // We avoid adding or removing element unnecessarily
   // since we have to modify userdata each time
   AutoTArray<nsIFrame*, 4> copy(mFrameList);
   if (!copy.RemoveElement(aItem->Frame())) {
     AddFrame(aItem->Frame());
     mChangedFrameInvalidations.Or(mChangedFrameInvalidations,
                                   aItem->Frame()->GetVisualOverflowRect());
   }
@@ -3222,25 +3237,20 @@ void ContainerState::FinishPaintedLayerD
                  "Layer already in list???");
     mNewChildLayers[data->mNewChildLayersIndex].mLayer = paintedLayer.forget();
   }
 
   for (auto& item : data->mAssignedDisplayItems) {
     MOZ_ASSERT(item.mItem->GetType() != DisplayItemType::TYPE_LAYER_EVENT_REGIONS);
     MOZ_ASSERT(item.mItem->GetType() != DisplayItemType::TYPE_COMPOSITOR_HITTEST_INFO);
 
-    DisplayItemData* oldData =
-      mLayerBuilder->GetOldLayerForFrame(item.mItem->Frame(),
-                                         item.mItem->GetPerFrameKey(),
-                                         item.mItem->HasMergedFrames() ? nullptr : item.mItem->GetDisplayItemData());
-    InvalidateForLayerChange(item.mItem, data->mLayer, oldData);
-    mLayerBuilder->AddPaintedDisplayItem(data, item.mItem, item.mClip,
-                                         *this, item.mLayerState,
-                                         data->mAnimatedGeometryRootOffset,
-                                         oldData, item);
+    InvalidateForLayerChange(item.mItem, data->mLayer, item.mDisplayItemData);
+    mLayerBuilder->AddPaintedDisplayItem(data, item, *this,
+                                         data->mAnimatedGeometryRootOffset);
+    item.mDisplayItemData = nullptr;
   }
 
   PaintedDisplayItemLayerUserData* userData = GetPaintedDisplayItemLayerUserData(data->mLayer);
   NS_ASSERTION(userData, "where did our user data go?");
   userData->mLastItemCount = data->mAssignedDisplayItems.Length();
 
   NewLayerEntry* newLayerEntry = &mNewChildLayers[data->mNewChildLayersIndex];
 
@@ -3530,17 +3540,21 @@ PaintedLayerData::Accumulate(ContainerSt
     // Note that the transform (if any) on the PaintedLayer is always an integer translation so
     // we don't have to factor that in here.
     aItem->DisableComponentAlpha();
   }
 
   bool clipMatches = mItemClip == aClip;
   mItemClip = aClip;
 
-  mAssignedDisplayItems.AppendElement(AssignedDisplayItem(aItem, aClip, aLayerState));
+  DisplayItemData* oldData =
+      aState->mLayerBuilder->GetOldLayerForFrame(aItem->Frame(),
+                                         aItem->GetPerFrameKey(),
+                                         aItem->HasMergedFrames() ? nullptr : aItem->GetDisplayItemData());
+  mAssignedDisplayItems.AppendElement(AssignedDisplayItem(aItem, aClip, aLayerState, oldData));
 
   if (aItem->MustPaintOnContentSide()) {
      mShouldPaintOnContentSide = true;
   }
 
   if (!mIsSolidColorInVisibleRegion && mOpaqueRegion.Contains(aVisibleRect) &&
       mVisibleRegion.Contains(aVisibleRect) && !mImage) {
     // A very common case! Most pages have a PaintedLayer with the page
@@ -4638,19 +4652,21 @@ ContainerState::ProcessDisplayItems(nsDi
         newLayerEntry->mBaseScrollMetadata =
           static_cast<nsDisplaySubDocument*>(item)->ComputeScrollMetadata(ownLayer->Manager(), mParameters);
       }
 
       /**
        * No need to allocate geometry for items that aren't
        * part of a PaintedLayer.
        */
-      oldData =
-        mLayerBuilder->GetOldLayerForFrame(item->Frame(), item->GetPerFrameKey());
-      mLayerBuilder->AddLayerDisplayItem(ownLayer, item, layerState, nullptr, oldData);
+      if (ownLayer->Manager() == mLayerBuilder->GetRetainingLayerManager()) {
+        oldData =
+          mLayerBuilder->GetOldLayerForFrame(item->Frame(), item->GetPerFrameKey());
+        mLayerBuilder->StoreDataForFrame(item, ownLayer, layerState, oldData);
+      }
     } else {
       const bool backfaceHidden = item->In3DContextAndBackfaceIsHidden();
       const nsIFrame* referenceFrame = item->ReferenceFrame();
 
       PaintedLayerData* paintedLayerData =
         mPaintedLayerDataTree.FindPaintedLayerFor(animatedGeometryRoot, itemASR, layerClipChain,
                                                   itemVisibleRect, backfaceHidden,
                                                   [&](PaintedLayerData* aData) {
@@ -4831,146 +4847,152 @@ FrameLayerBuilder::ComputeGeometryChange
         layerData->mTranslation);
   }
 
   aData->EndUpdate(geometry);
 }
 
 void
 FrameLayerBuilder::AddPaintedDisplayItem(PaintedLayerData* aLayerData,
-                                        nsDisplayItem* aItem,
-                                        const DisplayItemClip& aClip,
-                                        ContainerState& aContainerState,
-                                        LayerState aLayerState,
-                                        const nsPoint& aTopLeft,
-                                        DisplayItemData* aData,
-                                        AssignedDisplayItem& aAssignedDisplayItem)
+                                         AssignedDisplayItem& aItem,
+                                         ContainerState& aContainerState,
+                                         const nsPoint& aTopLeft)
 {
   PaintedLayer* layer = aLayerData->mLayer;
   PaintedDisplayItemLayerUserData* paintedData =
     static_cast<PaintedDisplayItemLayerUserData*>
       (layer->GetUserData(&gPaintedDisplayItemLayerUserData));
   RefPtr<BasicLayerManager> tempManager;
   nsIntRect intClip;
   bool hasClip = false;
-  if (aLayerState != LAYER_NONE) {
-    if (aData) {
-      tempManager = aData->mInactiveManager;
-
-      // We need to grab these before calling AddLayerDisplayItem because it will overwrite them.
+  if (aItem.mLayerState != LAYER_NONE) {
+    if (aItem.mDisplayItemData) {
+      tempManager = aItem.mDisplayItemData->mInactiveManager;
+
+      // We need to grab these before updating the DisplayItemData because it will overwrite them.
       nsRegion clip;
-      if (aClip.ComputeRegionInClips(&aData->GetClip(),
+      if (aItem.mClip.ComputeRegionInClips(&aItem.mDisplayItemData->GetClip(),
                                      aTopLeft - paintedData->mLastAnimatedGeometryRootOrigin,
                                      &clip)) {
         intClip = clip.GetBounds().ScaleToOutsidePixels(paintedData->mXScale,
                                                         paintedData->mYScale,
                                                         paintedData->mAppUnitsPerDevPixel);
       }
     }
     if (!tempManager) {
       tempManager = new BasicLayerManager(BasicLayerManager::BLM_INACTIVE);
     }
   }
 
-  AddLayerDisplayItem(layer, aItem, aLayerState, tempManager, aData);
+  if (layer->Manager() == mRetainingManager) {
+    DisplayItemData *data = aItem.mDisplayItemData;
+    if (data) {
+      if (!data->mUsed) {
+        data->BeginUpdate(layer, aItem.mLayerState, aItem.mItem, aItem.mReused, aItem.mMerged);
+      }
+    } else {
+      data = StoreDataForFrame(aItem.mItem, layer, aItem.mLayerState, nullptr);
+    }
+    data->mInactiveManager = tempManager;
+  }
 
   if (tempManager) {
-    FLB_LOG_PAINTED_LAYER_DECISION(aLayerData, "Creating nested FLB for item %p\n", aItem);
+    FLB_LOG_PAINTED_LAYER_DECISION(aLayerData, "Creating nested FLB for item %p\n", aItem.mItem);
     FrameLayerBuilder* layerBuilder = new FrameLayerBuilder();
     layerBuilder->Init(mDisplayListBuilder, tempManager, aLayerData, true,
-                       &aClip);
+                       &aItem.mClip);
 
     tempManager->BeginTransaction();
     if (mRetainingManager) {
       layerBuilder->DidBeginRetainedLayerTransaction(tempManager);
     }
 
     UniquePtr<LayerProperties> props(LayerProperties::CloneFrom(tempManager->GetRoot()));
     RefPtr<Layer> tmpLayer =
-      aItem->BuildLayer(mDisplayListBuilder, tempManager, ContainerLayerParameters());
+      aItem.mItem->BuildLayer(mDisplayListBuilder, tempManager, ContainerLayerParameters());
     // We have no easy way of detecting if this transaction will ever actually get finished.
     // For now, I've just silenced the warning with nested transactions in BasicLayers.cpp
     if (!tmpLayer) {
       tempManager->EndTransaction(nullptr, nullptr);
       tempManager->SetUserData(&gLayerManagerLayerBuilder, nullptr);
-      aAssignedDisplayItem.mItem = nullptr;
+      aItem.mItem = nullptr;
       return;
     }
 
     bool snap;
     nsRect visibleRect =
-      aItem->GetVisibleRect().Intersect(aItem->GetBounds(mDisplayListBuilder, &snap));
+      aItem.mItem->GetVisibleRect().Intersect(aItem.mItem->GetBounds(mDisplayListBuilder, &snap));
     nsIntRegion rgn = visibleRect.ToOutsidePixels(paintedData->mAppUnitsPerDevPixel);
 
     // Convert the visible rect to a region and give the item
     // a chance to try restrict it further.
-    nsRegion tightBounds = aItem->GetTightBounds(mDisplayListBuilder, &snap);
+    nsRegion tightBounds = aItem.mItem->GetTightBounds(mDisplayListBuilder, &snap);
     if (!tightBounds.IsEmpty()) {
       rgn.AndWith(tightBounds.ToOutsidePixels(paintedData->mAppUnitsPerDevPixel));
     }
     SetOuterVisibleRegion(tmpLayer, &rgn);
 
     // If BuildLayer didn't call BuildContainerLayerFor, then our new layer won't have been
     // stored in layerBuilder. Manually add it now.
     if (mRetainingManager) {
 #ifdef DEBUG_DISPLAY_ITEM_DATA
       LayerManagerData* parentLmd = static_cast<LayerManagerData*>
         (layer->Manager()->GetUserData(&gLayerManagerUserData));
       LayerManagerData* lmd = static_cast<LayerManagerData*>
         (tempManager->GetUserData(&gLayerManagerUserData));
       lmd->mParent = parentLmd;
 #endif
-      DisplayItemData* data = layerBuilder->GetDisplayItemDataForManager(aItem, tempManager);
-      layerBuilder->StoreDataForFrame(aItem, tmpLayer, LAYER_ACTIVE, data);
+      DisplayItemData* data = layerBuilder->GetDisplayItemDataForManager(aItem.mItem, tempManager);
+      layerBuilder->StoreDataForFrame(aItem.mItem, tmpLayer, LAYER_ACTIVE, data);
     }
 
     tempManager->SetRoot(tmpLayer);
     layerBuilder->WillEndTransaction();
     tempManager->AbortTransaction();
 
 #ifdef MOZ_DUMP_PAINTING
     if (gfxUtils::DumpDisplayList() || gfxEnv::DumpPaint()) {
-      fprintf_stderr(gfxUtils::sDumpPaintFile, "Basic layer tree for painting contents of display item %s(%p):\n", aItem->Name(), aItem->Frame());
+      fprintf_stderr(gfxUtils::sDumpPaintFile, "Basic layer tree for painting contents of display item %s(%p):\n", aItem.mItem->Name(), aItem.mItem->Frame());
       std::stringstream stream;
       tempManager->Dump(stream, "", gfxEnv::DumpPaintToFile());
       fprint_stderr(gfxUtils::sDumpPaintFile, stream);  // not a typo, fprint_stderr declared in LayersLogging.h
     }
 #endif
 
     nsIntPoint offset = GetLastPaintOffset(layer) - GetTranslationForPaintedLayer(layer);
     props->MoveBy(-offset);
     // Effective transforms are needed by ComputeDifferences().
     tmpLayer->ComputeEffectiveTransforms(Matrix4x4());
     nsIntRegion invalid;
     if (!props->ComputeDifferences(tmpLayer, invalid, nullptr)) {
-      nsRect visible = aItem->Frame()->GetVisualOverflowRect();
+      nsRect visible = aItem.mItem->Frame()->GetVisualOverflowRect();
       invalid = visible.ToOutsidePixels(paintedData->mAppUnitsPerDevPixel);
     }
-    if (aLayerState == LAYER_SVG_EFFECTS) {
-      invalid = nsSVGIntegrationUtils::AdjustInvalidAreaForSVGEffects(aItem->Frame(),
-                                                                      aItem->ToReferenceFrame(),
+    if (aItem.mLayerState == LAYER_SVG_EFFECTS) {
+      invalid = nsSVGIntegrationUtils::AdjustInvalidAreaForSVGEffects(aItem.mItem->Frame(),
+                                                                      aItem.mItem->ToReferenceFrame(),
                                                                       invalid);
     }
     if (!invalid.IsEmpty()) {
 #ifdef MOZ_DUMP_PAINTING
       if (nsLayoutUtils::InvalidationDebuggingIsEnabled()) {
-        printf_stderr("Inactive LayerManager(%p) for display item %s(%p) has an invalid region - invalidating layer %p\n", tempManager.get(), aItem->Name(), aItem->Frame(), layer);
+        printf_stderr("Inactive LayerManager(%p) for display item %s(%p) has an invalid region - invalidating layer %p\n", tempManager.get(), aItem.mItem->Name(), aItem.mItem->Frame(), layer);
       }
 #endif
       invalid.ScaleRoundOut(paintedData->mXScale, paintedData->mYScale);
 
       if (hasClip) {
         invalid.And(invalid, intClip);
       }
 
       InvalidatePostTransformRegion(layer, invalid,
                                     GetTranslationForPaintedLayer(layer));
     }
   }
-  aAssignedDisplayItem.mInactiveLayerManager = tempManager;
+  aItem.mInactiveLayerManager = tempManager;
 }
 
 DisplayItemData*
 FrameLayerBuilder::StoreDataForFrame(nsDisplayItem* aItem, Layer* aLayer,
                                      LayerState aState, DisplayItemData* aData)
 {
   if (aData) {
     if (!aData->mUsed) {
@@ -5013,37 +5035,35 @@ FrameLayerBuilder::StoreDataForFrame(nsI
   RefPtr<DisplayItemData> data =
     new (aFrame->PresContext()) DisplayItemData(lmd, aDisplayItemKey, aLayer, aFrame);
 
   data->BeginUpdate(aLayer, aState);
 
   lmd->mDisplayItems.PutEntry(data);
 }
 
+AssignedDisplayItem::AssignedDisplayItem(nsDisplayItem* aItem,
+                                         const DisplayItemClip& aClip,
+                                         LayerState aLayerState,
+                                         DisplayItemData* aData)
+  : mItem(aItem)
+  , mClip(aClip)
+  , mLayerState(aLayerState)
+  , mDisplayItemData(aData)
+  , mReused(aItem->IsReused())
+  , mMerged(aItem->HasMergedFrames())
+{}
+
 AssignedDisplayItem::~AssignedDisplayItem()
 {
   if (mInactiveLayerManager) {
     mInactiveLayerManager->SetUserData(&gLayerManagerLayerBuilder, nullptr);
   }
 }
 
-void
-FrameLayerBuilder::AddLayerDisplayItem(Layer* aLayer,
-                                       nsDisplayItem* aItem,
-                                       LayerState aLayerState,
-                                       BasicLayerManager* aManager,
-                                       DisplayItemData* aData)
-{
-  if (aLayer->Manager() != mRetainingManager)
-    return;
-
-  DisplayItemData *data = StoreDataForFrame(aItem, aLayer, aLayerState, aData);
-  data->mInactiveManager = aManager;
-}
-
 nsIntPoint
 FrameLayerBuilder::GetLastPaintOffset(PaintedLayer* aLayer)
 {
   PaintedDisplayItemLayerUserData* layerData = GetPaintedDisplayItemLayerUserData(aLayer);
   MOZ_ASSERT(layerData);
   if (layerData->mHasExplicitLastPaintOffset) {
     return layerData->mLastPaintOffset;
   }
--- a/layout/painting/FrameLayerBuilder.h
+++ b/layout/painting/FrameLayerBuilder.h
@@ -151,16 +151,18 @@ private:
     * object.
     * Set the passed in parameters, and clears the opt layer and inactive manager.
     * Parent, and display item key are assumed to be the same.
     *
     * EndUpdate must be called before the end of the transaction to complete the update.
     */
   void BeginUpdate(layers::Layer* aLayer, LayerState aState,
                    nsDisplayItem* aItem = nullptr);
+  void BeginUpdate(layers::Layer* aLayer, LayerState aState,
+                   nsDisplayItem* aItem, bool aIsReused, bool aIsMerged);
 
   /**
     * Completes the update of this, and removes any references to data that won't live
     * longer than the transaction.
     *
     * Updates the geometry, frame list and clip.
     * For items within a PaintedLayer, a geometry object must be specified to retain
     * until the next transaction.
@@ -207,34 +209,34 @@ public:
   nsRegion mRegion;
   bool mIsInfinite;
 };
 
 struct AssignedDisplayItem
 {
   AssignedDisplayItem(nsDisplayItem* aItem,
                       const DisplayItemClip& aClip,
-                      LayerState aLayerState)
-    : mItem(aItem)
-    , mClip(aClip)
-    , mLayerState(aLayerState)
-  {}
-
+                      LayerState aLayerState,
+                      DisplayItemData* aData);
   ~AssignedDisplayItem();
 
   nsDisplayItem* mItem;
   DisplayItemClip mClip;
   LayerState mLayerState;
+  DisplayItemData* mDisplayItemData;
 
   /**
    * If the display item is being rendered as an inactive
    * layer, then this stores the layer manager being
    * used for the inactive transaction.
    */
   RefPtr<layers::LayerManager> mInactiveLayerManager;
+
+  bool mReused;
+  bool mMerged;
 };
 
 
 struct ContainerLayerParameters {
   ContainerLayerParameters()
     : mXScale(1)
     , mYScale(1)
     , mLayerContentsVisibleRect(nullptr)
@@ -497,45 +499,26 @@ public:
 
 
   /******* PRIVATE METHODS to FrameLayerBuilder.cpp ********/
   /* These are only in the public section because they need
    * to be called by file-scope helper functions in FrameLayerBuilder.cpp.
    */
 
   /**
-   * Record aItem as a display item that is rendered by aLayer.
-   *
-   * @param aLayer Layer that the display item will be rendered into
-   * @param aItem Display item to be drawn.
-   * @param aLayerState What LayerState the item is using.
-   * @param aManager If the layer is in the LAYER_INACTIVE state,
-   * then this is the temporary layer manager to draw with.
-   */
-  void AddLayerDisplayItem(Layer* aLayer,
-                           nsDisplayItem* aItem,
-                           LayerState aLayerState,
-                           BasicLayerManager* aManager,
-                           DisplayItemData* aData);
-
-  /**
    * Record aItem as a display item that is rendered by the PaintedLayer
    * aLayer, with aClipRect, where aContainerLayerFrame is the frame
    * for the container layer this ThebesItem belongs to.
    * aItem must have an underlying frame.
    * @param aTopLeft offset from active scrolled root to reference frame
    */
   void AddPaintedDisplayItem(PaintedLayerData* aLayer,
-                            nsDisplayItem* aItem,
-                            const DisplayItemClip& aClip,
-                            ContainerState& aContainerState,
-                            LayerState aLayerState,
-                            const nsPoint& aTopLeft,
-                            DisplayItemData* aData,
-                            AssignedDisplayItem& aAssignedDisplayItem);
+                             AssignedDisplayItem& aAssignedDisplayItem,
+                             ContainerState& aContainerState,
+                             const nsPoint& aTopLeft);
 
   /**
    * Calls GetOldLayerForFrame on the underlying frame of the display item,
    * and each subsequent merged frame if no layer is found for the underlying
    * frame.
    */
   Layer* GetOldLayerFor(nsDisplayItem* aItem,
                         nsDisplayItemGeometry** aOldGeometry = nullptr,
@@ -622,31 +605,30 @@ public:
    * Given a frame and a display item key that uniquely identifies a
    * display item for the frame, find the layer that was last used to
    * render that display item. Returns null if there is no such layer.
    * This could be a dedicated layer for the display item, or a PaintedLayer
    * that renders many display items.
    */
   DisplayItemData* GetOldLayerForFrame(nsIFrame* aFrame, uint32_t aDisplayItemKey, DisplayItemData* aOldData = nullptr);
 
-protected:
-
-  friend class LayerManagerData;
-
   /**
    * Stores DisplayItemData associated with aFrame, stores the data in
    * mNewDisplayItemData.
    */
   DisplayItemData* StoreDataForFrame(nsDisplayItem* aItem, Layer* aLayer,
                                      LayerState aState, DisplayItemData* aData);
   void StoreDataForFrame(nsIFrame* aFrame,
                          uint32_t aDisplayItemKey,
                          Layer* aLayer,
                          LayerState aState);
 
+protected:
+  friend class LayerManagerData;
+
   // Flash the area within the context clip if paint flashing is enabled.
   static void FlashPaint(gfxContext *aContext);
 
   /*
    * Get the DisplayItemData array associated with this frame, or null if one
    * doesn't exist.
    *
    * Note that the pointer returned here is only valid so long as you don't