Don't miss cache for orthogonal flow ancestor on subsequent layout.

If a node has already been laid out at least once in the current lifecycle update, there's no need to miss the layout cache just because there's an orthogonal writing mode root somewhere in there, since the engine has had an opportunity to update things based on the new containing block size. For flex layout, missing the cache every time caused problematic performance complexity (didn't check, but possibly O(2^n), 'n' being the nesting level). The performance test included is about 25000 times faster than before, from about 0.065 runs per second, to about 1600 times per second (when testing locally). Bug: 1472660 Change-Id: I5c5c813c745d75ebcd5d6d82a1b96354c64bc502 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4793751 Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org> Commit-Queue: Morten Stenshorne <mstensho@chromium.org> Cr-Commit-Position: refs/heads/main@{#1185431}
chromium · Aug 18, 2023 · 1d88a42 · 1d88a42
1 parent 9fadc10
commit 1d88a42
Show file tree

Hide file tree

Showing 5 changed files with 79 additions and 12 deletions.
diff --git a/third_party/blink/perf_tests/layout/flexbox-deeply-nested-with-ortho-flow.html b/third_party/blink/perf_tests/layout/flexbox-deeply-nested-with-ortho-flow.html
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<style>
+  body {
+    overflow: scroll;
+  }
+  iframe {
+    display: block;
+  }
+</style>
+<script src="../resources/runner.js"></script>
+<iframe id="target" xsrc="resources/flexbox-deeply-nested-with-ortho-flow-as-iframe.html">
+</iframe>
+<pre id="log"></pre>
+<script>
+  var target = document.getElementById("target");
+  var style = target.style;
+
+  var parent = target.contentDocument.body;
+  for (var i = 0; i < 20; i++) {
+    var div = target.contentDocument.createElement("div");
+    div.style.display = "flex";
+    parent.appendChild(div);
+    parent = div;
+  }
+  var div = target.contentDocument.createElement("div");
+  div.style.writingMode = "vertical-rl";
+  div.id = "ortho";
+  parent.appendChild(div);
+  parent = div;
+  var text = target.contentDocument.createTextNode("It went sideways");
+  parent.appendChild(text);
+
+  function test() {
+    style.height = "200px";
+    target.contentDocument.documentElement.offsetTop;
+    style.height = "190px";
+    target.contentDocument.documentElement.offsetTop;
+  }
+
+  onload = function() {
+    PerfTestRunner.measureRunsPerSecond({
+      description: "Orthogonal flow root inside deeply nested flex, change initial containing block size",
+      run: test
+    });
+  }
+</script>
diff --git a/third_party/blink/renderer/core/layout/layout_box_hot.cc b/third_party/blink/renderer/core/layout/layout_box_hot.cc
@@ -162,7 +162,7 @@ const NGLayoutResult* LayoutBox::CachedLayoutResult(
     return nullptr;
 
   if (cached_layout_result->HasOrthogonalFallbackSizeDescendant() &&
-      View()->IsResizingInitialContainingBlock()) {
+      View()->AffectedByResizedInitialContainingBlock(*cached_layout_result)) {
     // There's an orthogonal writing-mode root somewhere inside that depends on
     // the size of the initial containing block, and the initial containing
     // block size is changing.

diff --git a/third_party/blink/renderer/core/layout/layout_view.cc b/third_party/blink/renderer/core/layout/layout_view.cc
@@ -50,6 +50,7 @@
 #include "third_party/blink/renderer/core/layout/ng/list/layout_ng_list_item.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_block_node.h"
 #include "third_party/blink/renderer/core/layout/ng/ng_constraint_space_builder.h"
+#include "third_party/blink/renderer/core/layout/ng/ng_layout_result.h"
 #include "third_party/blink/renderer/core/layout/svg/layout_svg_root.h"
 #include "third_party/blink/renderer/core/layout/view_fragmentation_context.h"
 #include "third_party/blink/renderer/core/page/chrome_client.h"
@@ -130,6 +131,7 @@ void LayoutView::Trace(Visitor* visitor) const {
   visitor->Trace(fragmentation_context_);
   visitor->Trace(svg_text_descendants_);
   visitor->Trace(hit_test_cache_);
+  visitor->Trace(initial_containing_block_resize_handled_list_);
   LayoutBlockFlow::Trace(visitor);
 }
 
@@ -895,6 +897,20 @@ CompositingReasons LayoutView::AdditionalCompositingReasons() const {
   return CompositingReason::kNone;
 }
 
+bool LayoutView::AffectedByResizedInitialContainingBlock(
+    const NGLayoutResult& layout_result) {
+  NOT_DESTROYED();
+  if (!initial_containing_block_resize_handled_list_) {
+    return false;
+  }
+  const LayoutObject* layout_object =
+      layout_result.PhysicalFragment().GetLayoutObject();
+  DCHECK(layout_object);
+  auto add_result =
+      initial_containing_block_resize_handled_list_->insert(layout_object);
+  return add_result.is_new_entry;
+}
+
 void LayoutView::UpdateMarkersAndCountersAfterStyleChange(
     LayoutObject* container) {
   NOT_DESTROYED();

diff --git a/third_party/blink/renderer/core/layout/layout_view.h b/third_party/blink/renderer/core/layout/layout_view.h
@@ -33,6 +33,7 @@
 #include "third_party/blink/renderer/core/layout/layout_quote.h"
 #include "third_party/blink/renderer/core/scroll/scrollable_area.h"
 #include "third_party/blink/renderer/platform/graphics/overlay_scrollbar_clip_behavior.h"
+#include "third_party/blink/renderer/platform/heap/collection_support/heap_hash_set.h"
 #include "third_party/blink/renderer/platform/heap/garbage_collected.h"
 #include "third_party/blink/renderer/platform/wtf/casting.h"
 
@@ -268,11 +269,8 @@ class CORE_EXPORT LayoutView : public LayoutBlockFlow {
     needs_marker_counter_update_ = true;
   }
 
-  // Return true if laying out with a new initial containing block size.
-  bool IsResizingInitialContainingBlock() const {
-    NOT_DESTROYED();
-    return is_resizing_initial_containing_block_;
-  }
+  // Return true if laying out with a new initial containing block size. lala.
+  bool AffectedByResizedInitialContainingBlock(const NGLayoutResult&);
 
   // Update generated markers and counters after style and layout tree update.
   // container - The container for container queries, otherwise nullptr.
@@ -367,9 +365,10 @@ class CORE_EXPORT LayoutView : public LayoutBlockFlow {
     return ViewLogicalHeight(kIncludeScrollbars);
   }
 
-  // Set to true if laying out with a new initial containing block size. Always
-  // set back to false after layout.
-  bool is_resizing_initial_containing_block_ = false;
+  // Set if laying out with a new initial containing block size, and populated
+  // as we handle nodes that may have been affected by that.
+  Member<HeapHashSet<Member<const LayoutObject>>>
+      initial_containing_block_resize_handled_list_;
 
  private:
   bool CanHaveChildren() const override;

diff --git a/third_party/blink/renderer/core/layout/ng/layout_ng_view.cc b/third_party/blink/renderer/core/layout/ng/layout_ng_view.cc
@@ -67,18 +67,24 @@ void LayoutNGView::UpdateLayout() {
       chrome_client.GetScreenInfo(frame).device_scale_factor);
 #endif
 
-  is_resizing_initial_containing_block_ =
+  bool is_resizing_initial_containing_block =
       LogicalWidth() != ViewLogicalWidthForBoxSizing() ||
       LogicalHeight() != ViewLogicalHeightForBoxSizing();
   bool invalidate_svg_roots =
       GetDocument().SvgExtensions() && !ShouldUsePrintingLayout() &&
-      (!GetFrameView() || is_resizing_initial_containing_block_);
+      (!GetFrameView() || is_resizing_initial_containing_block);
   if (invalidate_svg_roots) {
     GetDocument()
         .AccessSVGExtensions()
         .InvalidateSVGRootsWithRelativeLengthDescendents();
   }
 
+  DCHECK(!initial_containing_block_resize_handled_list_);
+  if (is_resizing_initial_containing_block) {
+    initial_containing_block_resize_handled_list_ =
+        MakeGarbageCollected<HeapHashSet<Member<const LayoutObject>>>();
+  }
+
   const auto& style = StyleRef();
   NGConstraintSpaceBuilder builder(
       style.GetWritingMode(), style.GetWritingDirection(),
@@ -88,7 +94,7 @@ void LayoutNGView::UpdateLayout() {
   builder.SetIsFixedBlockSize(true);
 
   NGBlockNode(this).Layout(builder.ToConstraintSpace());
-  is_resizing_initial_containing_block_ = false;
+  initial_containing_block_resize_handled_list_ = nullptr;
 }
 
 AtomicString LayoutNGView::NamedPageAtIndex(wtf_size_t page_index) const {