Chromium

[Empty comment from Monorail migration]

[104347:104433:0523/012156.712554:FATAL:ng_inline_item_result.cc(55)] Check failed: end_offset - start_offset == shape_result->NumCharacters() (55 vs. 56)

[Empty comment from Monorail migration]

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

commit 2d32fab3dd7133b17eb9b4c2efe2f1a9e46dcffe
Author: Koji Ishii <kojii@chromium.org>
Date: Fri May 25 11:31:24 2018

[LayoutNG] Fix ShapingLineBreaker not to produce incorrect range

This patch fixes a case where ShapingLineBreaker may produce
incorrect number of characters in the ShapeResult. It was
probably introduced in r471636 [1]. When searching for the
last safe position, there is a case where last_safe and
line_end_result lost sync. Then ShapingLineBreaker constructs
the final ShapeResult assuming they are in sync.

The fix is done by simplifying the logic for when including
more characters can shorten the width. Two nested loops, one
to find previous then another to find next is changed to
single loop. This also avoids trying opportunities once tried
and failed.

When no break opportunities can fit and that multiple break
points are valid, this patch picks a different break than
before. I think the new break is better, but we may want to
revisit this as we learn more.

Includes following related changes:
1. Add is_overflow state. When it overflows, no need to look
   for previous opportunities.
2. Add BreakOpportunity struct to ensure the offset and
   is_hyphenated are in sync.
3. Add more DCHECKs to ensure start, first_safe, last_safe,
   and break_opportunity are always in this order.
4. Add more DCEHCKs to ensure the offsets above are in sync
   with each corresponding ShapeResult.
5. Uncommented DCHECK that used to fail on Mac.

[1] https://codereview.chromium.org/2875933006

Bug: 636993, 846138
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I8e740f9f59ce4d50f20cf4cb360f460685418583
Reviewed-on: https://chromium-review.googlesource.com/1072208
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561829}
[modify] https://crrev.com/2d32fab3dd7133b17eb9b4c2efe2f1a9e46dcffe/third_party/WebKit/LayoutTests/FlagExpectations/enable-blink-features=LayoutNG
[modify] https://crrev.com/2d32fab3dd7133b17eb9b4c2efe2f1a9e46dcffe/third_party/blink/renderer/core/layout/ng/inline/ng_line_breaker.cc
[modify] https://crrev.com/2d32fab3dd7133b17eb9b4c2efe2f1a9e46dcffe/third_party/blink/renderer/platform/fonts/shaping/shaping_line_breaker.cc
[modify] https://crrev.com/2d32fab3dd7133b17eb9b4c2efe2f1a9e46dcffe/third_party/blink/renderer/platform/fonts/shaping/shaping_line_breaker.h
[modify] https://crrev.com/2d32fab3dd7133b17eb9b4c2efe2f1a9e46dcffe/third_party/blink/renderer/platform/fonts/shaping/shaping_line_breaker_test.cc

DCHECK failures fixed, still very slow but at least runs.

I'm still struggling to run profiler to see what's taking time.

Turns out that DCHECK code is consuming 90% of the time, the fix is on its way.
https://chromium-review.googlesource.com/c/chromium/src/+/1077910

With the fix applied:
* 95% spent in ShapingLineBreaker::ShapeLine
* 66% spent in RunSegmenter::ConsumePast
* 51% spent in ScriptRunIterator::Consume

There are more rooms to improve RunSegmenter, especially for the extremely long text, but most other parts seem to back to normal.

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

commit aab641bf58c9e3b17a2ae28804da290bdfb2c17f
Author: Koji Ishii <kojii@chromium.org>
Date: Wed May 30 18:39:46 2018

Pre-allocate Vector<UScriptCode> in ScriptRunIterator

This patch pre-allocates Vector<UScriptCode> in
ScriptRunIterator as the allocation of this vector turned out
to be one of the hottest code path.

Bug: 645035, 846138
Change-Id: Id424d6424d35a9cb7eaf62f15b809843644bd40a
Reviewed-on: https://chromium-review.googlesource.com/1077916
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562927}
[modify] https://crrev.com/aab641bf58c9e3b17a2ae28804da290bdfb2c17f/third_party/blink/renderer/platform/fonts/script_run_iterator.cc
[modify] https://crrev.com/aab641bf58c9e3b17a2ae28804da290bdfb2c17f/third_party/blink/renderer/platform/fonts/script_run_iterator.h
[modify] https://crrev.com/aab641bf58c9e3b17a2ae28804da290bdfb2c17f/third_party/blink/renderer/platform/fonts/script_run_iterator_test.cc

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

commit 0ec16dc229bed3d18e752d6c5ec8ea95cc3fed21
Author: Koji Ishii <kojii@chromium.org>
Date: Wed May 30 18:44:28 2018

[LayoutNG] Dump ShapeResult only when DCHECK fails

The "DCHECK(...) << ToString()" calls ToString() even when
DCHECK pass. ShapeResult::ToString() is expensive when it's
large that using operator<<() makes debug/DCHECK builds
faster.

Before this fix, 90% of time was consumed in ToString() for
blink/perf_tests/layout/word-break-break-all.html

Bug: 636993, 846138
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I808dc3b53da8a65e89bf7157907977ecf00221a9
Reviewed-on: https://chromium-review.googlesource.com/1077910
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#562933}
[modify] https://crrev.com/0ec16dc229bed3d18e752d6c5ec8ea95cc3fed21/third_party/blink/renderer/platform/fonts/shaping/shape_result.cc
[modify] https://crrev.com/0ec16dc229bed3d18e752d6c5ec8ea95cc3fed21/third_party/blink/renderer/platform/fonts/shaping/shape_result.h

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

commit 9a99dba19ace0afe37014dbb8d7378983fc8c7ab
Author: Koji Ishii <kojii@chromium.org>
Date: Fri Jun 01 14:36:24 2018

Make swapping next/ahead_set faster by making them pointers

Pre-allocating inline vector for UScriptCode in CL:1077916
improved the overall performance of ScriptRunIterator. In
this new code, swapping next_set and ahead_set consumes ~50%
of ScriptRunIterator::Fetch(). This part is slower than
before because these vectors are now inlined.

This patch changes them to pointers to turn the swap
operation of inline vectors to the pointer swap operation.

This improves perf_tests/layout/word-break-break-all.html
running on LayoutNG by ~50%, and reduces
ScriptRunIterator::Fetch() to ~25% of total time, and 80% of
Fetch() is spent in GetScript().

Note, LayoutNG is still slower than legacy by ~10x on the
test, and 63% of the time is in RunSegmenter::ConsumePast().
We will need some more tuning around this. 63% is, though,
down from ~90%, with these fixes and a few other WIP fixes.

Bug: 645035, 846138
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: I45a71009447015d44ec0ae0ab05dd2621f07c262
Reviewed-on: https://chromium-review.googlesource.com/1080467
Reviewed-by: Dominik Röttsches <drott@chromium.org>
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Koji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#563624}
[modify] https://crrev.com/9a99dba19ace0afe37014dbb8d7378983fc8c7ab/third_party/blink/renderer/platform/fonts/script_run_iterator.cc
[modify] https://crrev.com/9a99dba19ace0afe37014dbb8d7378983fc8c7ab/third_party/blink/renderer/platform/fonts/script_run_iterator.h

This issue was migrated from crbug.com/chromium/846138?no_tracker_redirect=1

[Monorail blocking: crbug.com/chromium/591099]
[Monorail components added to Component Tags custom field.]