New issue
Advanced search Search tips

Issue 808100 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Task


Participants' hotlists:
layoutng


Sign in to add a comment

Improve MarkContainerNeedsCollectInlines logic

Project Member Reported by e...@chromium.org, Feb 1 2018

Issue description

Currently LayoutObject::MarkContainerNeedsCollectInlines is set any time LayoutObject::SetNeedsLayout is triggered. This cuases PrepareLayout in NGInlineNode to be invalidated. In most cases this is unnecesarry, it only needs to be invalidated if the content has changed (subtree modified) or if a font-related style property is changed.

All other changes (element is resized/moved, color, border change etc) allows the PrepareLayout step to be skipped.
 

Comment 1 by kojii@chromium.org, Mar 19 2018

Cc: atotic@chromium.org
atotic@ found:
> fast/css/large-numbers.html takes 24s in ng, almost instant in legacy

I ran perf analysis on this today, found 78% of time is spent in ShapeText. Because this test repeats measuring offsetLeft and adding a line to log, we're probably shaping the whole log text on each test iteration.

Comment 2 by e...@chromium.org, Mar 19 2018

Ah yes, we could be a lot smarter in how we handle this. For cases where there is pure addition we should be able to "append" thew new content to the items list and only reshape from last safe-to-break.

Project Member

Comment 3 by bugdroid1@chromium.org, Mar 29 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/122b676b6cb0e0b319d3c1199a622e214eeb5f8e

commit 122b676b6cb0e0b319d3c1199a622e214eeb5f8e
Author: Koji Ishii <kojii@chromium.org>
Date: Thu Mar 29 20:49:58 2018

[LayoutNG] Make NGInlineItemsBuilder stateless

In part of re-using once-created NGInlineItem to reduce the shaping,
this patch refactors NGInlineItemsBuilder to be stateless.

Instead, the collapsing type at the end of NGInlineItem is stored in
each NGInlineItem. NGInlineItemsBuilder uses it to determine
whitespace collapsing across NGInlineItems.

Because the cross-item collapsing can make the re-use more complex,
the algorithm is taken out from the main loop. We may need some
more iterations on this part to ensure cross-item whitespace
collapsing works correctly when we start re-using NGInlineItems.

Bug: 808100
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I3092729a7fcefe87a9e01f57ca74e5b402572a29
Reviewed-on: https://chromium-review.googlesource.com/984915
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546946}
[modify] https://crrev.com/122b676b6cb0e0b319d3c1199a622e214eeb5f8e/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_item.cc
[modify] https://crrev.com/122b676b6cb0e0b319d3c1199a622e214eeb5f8e/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_item.h
[modify] https://crrev.com/122b676b6cb0e0b319d3c1199a622e214eeb5f8e/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_items_builder.cc
[modify] https://crrev.com/122b676b6cb0e0b319d3c1199a622e214eeb5f8e/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_items_builder.h
[modify] https://crrev.com/122b676b6cb0e0b319d3c1199a622e214eeb5f8e/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_items_builder_test.cc

Sign in to add a comment