[LayoutNG] Avoid bookkeeping shape_exclusions if shapes aren't present.

We spend a lot of time collecting floats. This is for an edge-case
calculation when shape-outside is specified on another float.

Typically this doesn't happen. This change checks if a float is added
with shape outside, and invalidates the derived_geometry_ member to
indicate it needs to track these objects now.

This removes one of the last large allocations in the exclusion space
code, and improves micro-benchmarks by 5-10%.

Bug: 635619
Change-Id: I32abb143959e28c526c16e779f9e2dc9c0992d56
Reviewed-on: https://chromium-review.googlesource.com/c/1309144Reviewed-by: Christian Biesinger <cbiesinger@chromium.org>
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604647}

third_party/blink/renderer/core/layout/ng/exclusions/ng_exclusion_space.cc

@@ -137,8 +137,11 @@ NGLayoutOpportunity CreateLayoutOpportunity(const NGLayoutOpportunity& other,
       std::min(other.rect.LineEndOffset(), offset.line_offset + inline_size),
       other.rect.BlockEndOffset());
 
-  return NGLayoutOpportunity(NGBfcRect(start_offset, end_offset),
-                             other.shape_exclusions);
+  return NGLayoutOpportunity(
+      NGBfcRect(start_offset, end_offset),
+      other.shape_exclusions
+          ? base::AdoptRef(new NGShapeExclusions(*other.shape_exclusions))
+          : nullptr);
 }
 
 // Creates a new layout opportunity. The given shelf *must* intersect with the
@@ -172,6 +175,7 @@ NGExclusionSpaceInternal::NGExclusionSpaceInternal()
     : exclusions_(RefVector<scoped_refptr<const NGExclusion>>::Create()),
       num_exclusions_(0),
       both_clear_offset_(LayoutUnit::Min()),
+      track_shape_exclusions_(false),
       derived_geometry_(nullptr) {}
 
 NGExclusionSpaceInternal::NGExclusionSpaceInternal(
@@ -179,6 +183,7 @@ NGExclusionSpaceInternal::NGExclusionSpaceInternal(
     : exclusions_(other.exclusions_),
       num_exclusions_(other.num_exclusions_),
       both_clear_offset_(other.both_clear_offset_),
+      track_shape_exclusions_(other.track_shape_exclusions_),
       derived_geometry_(std::move(other.derived_geometry_)) {
   // This copy-constructor does fun things. It moves the derived_geometry_ to
   // the newly created exclusion space where it'll more-likely be used.
@@ -193,6 +198,7 @@ NGExclusionSpaceInternal& NGExclusionSpaceInternal::operator=(
   exclusions_ = other.exclusions_;
   num_exclusions_ = other.num_exclusions_;
   both_clear_offset_ = other.both_clear_offset_;
+  track_shape_exclusions_ = other.track_shape_exclusions_;
   derived_geometry_ = std::move(other.derived_geometry_);
   other.derived_geometry_ = nullptr;
   return *this;
@@ -201,12 +207,15 @@ NGExclusionSpaceInternal& NGExclusionSpaceInternal::operator=(
 NGExclusionSpaceInternal& NGExclusionSpaceInternal::operator=(
     NGExclusionSpaceInternal&&) noexcept = default;
 
-NGExclusionSpaceInternal::DerivedGeometry::DerivedGeometry()
-    : last_float_block_start_(LayoutUnit::Min()),
+NGExclusionSpaceInternal::DerivedGeometry::DerivedGeometry(
+    bool track_shape_exclusions)
+    : track_shape_exclusions_(track_shape_exclusions),
+      last_float_block_start_(LayoutUnit::Min()),
       left_float_clear_offset_(LayoutUnit::Min()),
       right_float_clear_offset_(LayoutUnit::Min()) {
   // The exclusion space must always have at least one shelf, at -Infinity.
-  shelves_.emplace_back(/* block_offset */ LayoutUnit::Min());
+  shelves_.emplace_back(/* block_offset */ LayoutUnit::Min(),
+                        track_shape_exclusions_);
 }
 
 void NGExclusionSpaceInternal::Add(scoped_refptr<const NGExclusion> exclusion) {
@@ -221,7 +230,14 @@ void NGExclusionSpaceInternal::Add(scoped_refptr<const NGExclusion> exclusion) {
         exclusions_->GetVector().begin() + num_exclusions_);
     std::swap(exclusions_, exclusions);
 
-    // The derived_geometry_ is now invalid.
+    // The derived_geometry_ member is now invalid.
+    derived_geometry_ = nullptr;
+  }
+
+  // If this is the first exclusion with shape_data, the derived_geometry_
+  // member now needs to perform additional bookkeeping, and is invalid.
+  if (!track_shape_exclusions_ && exclusion->shape_data) {
+    track_shape_exclusions_ = true;
     derived_geometry_ = nullptr;
   }
 
@@ -408,6 +424,7 @@ void NGExclusionSpaceInternal::DerivedGeometry::Add(
                 exclusion.rect.BlockStartOffset(),
                 exclusion.rect.BlockEndOffset());
           }
+          if (shelf.shape_exclusions)
             shelf.shape_exclusions->line_left_shapes.emplace_back(&exclusion);
         } else {
           DCHECK_EQ(exclusion.type, EFloat::kRight);
@@ -420,6 +437,7 @@ void NGExclusionSpaceInternal::DerivedGeometry::Add(
                 exclusion.rect.BlockStartOffset(),
                 exclusion.rect.BlockEndOffset());
           }
+          if (shelf.shape_exclusions)
             shelf.shape_exclusions->line_right_shapes.emplace_back(&exclusion);
         }
 
@@ -468,7 +486,8 @@ void NGExclusionSpaceInternal::DerivedGeometry::Add(
 
       // We only want to add the shelf if it's at a different block offset.
       if (exclusion_end_offset != shelf_copy->block_offset) {
-        NGShelf new_shelf(/* block_offset */ exclusion_end_offset);
+        NGShelf new_shelf(/* block_offset */ exclusion_end_offset,
+                          track_shape_exclusions_);
 
         // shelf_copy->line_{left,right}_edges will not valid after these calls.
         CollectSolidEdges(&shelf_copy->line_left_edges, new_shelf.block_offset,
@@ -527,7 +546,7 @@ NGExclusionSpaceInternal::DerivedGeometry::FindLayoutOpportunity(
         if ((opportunity.rect.InlineSize() >= minimum_size.inline_size ||
              opportunity.rect.InlineSize() == available_inline_size) &&
             opportunity.rect.BlockSize() >= minimum_size.block_size) {
-          return_opportunity = opportunity;
+          return_opportunity = std::move(opportunity);
           return true;
         }
 
@@ -546,7 +565,7 @@ NGExclusionSpaceInternal::DerivedGeometry::AllLayoutOpportunities(
   IterateAllLayoutOpportunities(
       offset, available_inline_size,
       [&opportunities](const NGLayoutOpportunity opportunity) -> bool {
-        opportunities.push_back(opportunity);
+        opportunities.push_back(std::move(opportunity));
         return false;
       });
 
@@ -631,7 +650,8 @@ const NGExclusionSpaceInternal::DerivedGeometry&
 NGExclusionSpaceInternal::GetDerivedGeometry() const {
   // Re-build the geometry if it isn't present.
   if (!derived_geometry_) {
-    derived_geometry_ = std::make_unique<DerivedGeometry>();
+    derived_geometry_ =
+        std::make_unique<DerivedGeometry>(track_shape_exclusions_);
     DCHECK_LE(num_exclusions_, exclusions_->size());
     for (wtf_size_t i = 0; i < num_exclusions_; ++i)
       derived_geometry_->Add(*exclusions_->GetVector()[i]);

third_party/blink/renderer/core/layout/ng/exclusions/ng_exclusion_space.h

@@ -129,23 +129,26 @@ class CORE_EXPORT NGExclusionSpaceInternal {
   //  - The opportunity also holds onto a list of these edges to support
   //    css-shapes.
   struct NGShelf {
-    explicit NGShelf(LayoutUnit block_offset)
+    NGShelf(LayoutUnit block_offset, bool track_shape_exclusions)
         : block_offset(block_offset),
           line_left(LayoutUnit::Min()),
           line_right(LayoutUnit::Max()),
-          shape_exclusions(base::AdoptRef(new NGShapeExclusions)),
+          shape_exclusions(track_shape_exclusions
+                               ? base::AdoptRef(new NGShapeExclusions)
+                               : nullptr),
           has_shape_exclusions(false) {}
 
-    // The copy constructor explicitly copies the shape_exclusions member,
-    // instead of just incrementing the ref.
+    // The copy constructor explicitly copies the shape_exclusions member.
     NGShelf(const NGShelf& other)
         : block_offset(other.block_offset),
           line_left(other.line_left),
           line_right(other.line_right),
           line_left_edges(other.line_left_edges),
           line_right_edges(other.line_right_edges),
-          shape_exclusions(
-              base::AdoptRef(new NGShapeExclusions(*other.shape_exclusions))),
+          shape_exclusions(other.shape_exclusions
+                               ? base::AdoptRef(new NGShapeExclusions(
+                                     *other.shape_exclusions))
+                               : nullptr),
           has_shape_exclusions(other.has_shape_exclusions) {}
 
     NGShelf(NGShelf&& other) noexcept = default;
@@ -180,6 +183,12 @@ class CORE_EXPORT NGExclusionSpaceInternal {
   wtf_size_t num_exclusions_;
   LayoutUnit both_clear_offset_;
 
+  // In order to reduce the amount of copies related to bookkeeping shape data,
+  // we initially ignore exclusions with shape data. When we first see an
+  // exclusion with shape data, we set this flag, and rebuild the
+  // DerivedGeometry data-structure, to perform the additional bookkeeping.
+  bool track_shape_exclusions_;
+
   // The derived geometry struct, is the data-structure which handles all of the
   // queries on the exclusion space. It can always be rebuilt from exclusions_
   // and num_exclusions_. This is mutable as it is passed down a chain of
@@ -202,7 +211,7 @@ class CORE_EXPORT NGExclusionSpaceInternal {
     USING_FAST_MALLOC(DerivedGeometry);
 
    public:
-    DerivedGeometry();
+    explicit DerivedGeometry(bool track_shape_exclusions);
     DerivedGeometry(DerivedGeometry&& o) noexcept = default;
 
     void Add(const NGExclusion& exclusion);
@@ -259,6 +268,8 @@ class CORE_EXPORT NGExclusionSpaceInternal {
     Vector<NGShelf, 4> shelves_;
     Vector<NGLayoutOpportunity, 4> opportunities_;
 
+    bool track_shape_exclusions_;
+
     // This member is used for implementing the "top edge alignment rule" for
     // floats. Floats can be positioned at negative offsets, hence is
     // initialized the minimum value.