Commit eabd51f5 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

Move NGLogicalLineItems to NGInlineChildLayoutContext

This patch moves the ownership of |NGLogicalLineItems| from
|NGInlineLayoutAlgorithm| to |NGInlineChildLayoutContext|.

This change:
* Avoids moving |NGLogicalLineItems| to |NGFragmentItemsBuilder|.
  |NGLogicalLineItems| has an inline size that moving is more
  expensive than regular |Vector|.
* Can reuse the buffer (if expanded) for the next line.

Also, when clearing |NGLogicalLineItems|, make sure it
doesn't release the allocated buffer.

Bug: 982194
Change-Id: I8fa2416769618823876cd2a2669208e60623faeb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2259755
Commit-Queue: Koji Ishii <kojii@chromium.org>
Commit-Queue: Yoshifumi Inoue <yosin@chromium.org>
Reviewed-by: default avatarKent Tamura <tkent@chromium.org>
Reviewed-by: default avatarYoshifumi Inoue <yosin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#781269}
parent 095922ee
...@@ -38,11 +38,13 @@ NGFragmentItemsBuilder::NGFragmentItemsBuilder( ...@@ -38,11 +38,13 @@ NGFragmentItemsBuilder::NGFragmentItemsBuilder(
void NGFragmentItemsBuilder::SetCurrentLine( void NGFragmentItemsBuilder::SetCurrentLine(
const NGPhysicalLineBoxFragment& line, const NGPhysicalLineBoxFragment& line,
NGLogicalLineItems&& children) { NGLogicalLineItems* current_line) {
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
current_line_fragment_ = &line; current_line_fragment_ = &line;
#endif #endif
current_line_ = std::move(children); DCHECK(!current_line_);
DCHECK(current_line);
current_line_ = current_line;
} }
void NGFragmentItemsBuilder::AddLine(const NGPhysicalLineBoxFragment& line, void NGFragmentItemsBuilder::AddLine(const NGPhysicalLineBoxFragment& line,
...@@ -51,10 +53,11 @@ void NGFragmentItemsBuilder::AddLine(const NGPhysicalLineBoxFragment& line, ...@@ -51,10 +53,11 @@ void NGFragmentItemsBuilder::AddLine(const NGPhysicalLineBoxFragment& line,
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
DCHECK_EQ(current_line_fragment_, &line); DCHECK_EQ(current_line_fragment_, &line);
#endif #endif
DCHECK(current_line_);
// Reserve the capacity for (children + line box item). // Reserve the capacity for (children + line box item).
const wtf_size_t size_before = items_.size(); const wtf_size_t size_before = items_.size();
const wtf_size_t estimated_size = size_before + current_line_.size() + 1; const wtf_size_t estimated_size = size_before + current_line_->size() + 1;
const wtf_size_t old_capacity = items_.capacity(); const wtf_size_t old_capacity = items_.capacity();
if (estimated_size > old_capacity) if (estimated_size > old_capacity)
items_.ReserveCapacity(std::max(estimated_size, old_capacity * 2)); items_.ReserveCapacity(std::max(estimated_size, old_capacity * 2));
...@@ -63,7 +66,7 @@ void NGFragmentItemsBuilder::AddLine(const NGPhysicalLineBoxFragment& line, ...@@ -63,7 +66,7 @@ void NGFragmentItemsBuilder::AddLine(const NGPhysicalLineBoxFragment& line,
const wtf_size_t line_start_index = items_.size(); const wtf_size_t line_start_index = items_.size();
items_.emplace_back(offset, line); items_.emplace_back(offset, line);
AddItems(current_line_.begin(), current_line_.end()); AddItems(current_line_->begin(), current_line_->end());
// All children are added. Create an item for the start of the line. // All children are added. Create an item for the start of the line.
NGFragmentItem& line_item = items_[line_start_index].item; NGFragmentItem& line_item = items_[line_start_index].item;
...@@ -74,8 +77,11 @@ void NGFragmentItemsBuilder::AddLine(const NGPhysicalLineBoxFragment& line, ...@@ -74,8 +77,11 @@ void NGFragmentItemsBuilder::AddLine(const NGPhysicalLineBoxFragment& line,
// Keep children's offsets relative to |line|. They will be adjusted later in // Keep children's offsets relative to |line|. They will be adjusted later in
// |ConvertToPhysical()|. // |ConvertToPhysical()|.
current_line_.clear(); // Clear the current line without releasing the buffer. It is likely to be
// reused again.
current_line_->Shrink(0);
#if DCHECK_IS_ON() #if DCHECK_IS_ON()
current_line_ = nullptr;
current_line_fragment_ = nullptr; current_line_fragment_ = nullptr;
#endif #endif
......
...@@ -57,10 +57,9 @@ class CORE_EXPORT NGFragmentItemsBuilder { ...@@ -57,10 +57,9 @@ class CORE_EXPORT NGFragmentItemsBuilder {
// positions the line box. |SetCurrentLine| sets the children, and the next // positions the line box. |SetCurrentLine| sets the children, and the next
// |AddLine| adds them. // |AddLine| adds them.
// //
// TODO(kojii): Moving |NGLogicalLineItems| is not cheap because it has inline // The caller must keep |children| alive until |AddLine| completes.
// capacity. Reconsider the ownership.
void SetCurrentLine(const NGPhysicalLineBoxFragment& line, void SetCurrentLine(const NGPhysicalLineBoxFragment& line,
NGLogicalLineItems&& children); NGLogicalLineItems* current_line);
void AddLine(const NGPhysicalLineBoxFragment& line, void AddLine(const NGPhysicalLineBoxFragment& line,
const LogicalOffset& offset); const LogicalOffset& offset);
...@@ -131,7 +130,7 @@ class CORE_EXPORT NGFragmentItemsBuilder { ...@@ -131,7 +130,7 @@ class CORE_EXPORT NGFragmentItemsBuilder {
String first_line_text_content_; String first_line_text_content_;
// Keeps children of a line until the offset is determined. See |AddLine|. // Keeps children of a line until the offset is determined. See |AddLine|.
NGLogicalLineItems current_line_; NGLogicalLineItems* current_line_ = nullptr;
NGInlineNode node_; NGInlineNode node_;
......
...@@ -9,6 +9,7 @@ namespace blink { ...@@ -9,6 +9,7 @@ namespace blink {
namespace { namespace {
struct SameSizeAsNGInlineChildLayoutContext { struct SameSizeAsNGInlineChildLayoutContext {
NGLogicalLineItems line_items_;
base::Optional<NGInlineLayoutStateStack> box_states_; base::Optional<NGInlineLayoutStateStack> box_states_;
void* pointers[2]; void* pointers[2];
unsigned number; unsigned number;
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
#include "third_party/blink/renderer/core/layout/ng/inline/ng_fragment_items_builder.h" #include "third_party/blink/renderer/core/layout/ng/inline/ng_fragment_items_builder.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_box_state.h" #include "third_party/blink/renderer/core/layout/ng/inline/ng_inline_box_state.h"
#include "third_party/blink/renderer/core/layout/ng/inline/ng_logical_line_item.h"
namespace blink { namespace blink {
...@@ -27,6 +28,10 @@ class NGInlineChildLayoutContext { ...@@ -27,6 +28,10 @@ class NGInlineChildLayoutContext {
items_builder_ = builder; items_builder_ = builder;
} }
// Returns an instance of |NGLogicalLineItems|. This is reused when laying out
// the next line.
NGLogicalLineItems* LogicalLineItems() { return &logical_line_items_; }
// Returns the NGInlineLayoutStateStack in this context. // Returns the NGInlineLayoutStateStack in this context.
bool HasBoxStates() const { return box_states_.has_value(); } bool HasBoxStates() const { return box_states_.has_value(); }
NGInlineLayoutStateStack* BoxStates() { return &*box_states_; } NGInlineLayoutStateStack* BoxStates() { return &*box_states_; }
...@@ -50,6 +55,8 @@ class NGInlineChildLayoutContext { ...@@ -50,6 +55,8 @@ class NGInlineChildLayoutContext {
// transit, allocating separately is easier. // transit, allocating separately is easier.
NGFragmentItemsBuilder* items_builder_ = nullptr; NGFragmentItemsBuilder* items_builder_ = nullptr;
NGLogicalLineItems logical_line_items_;
base::Optional<NGInlineLayoutStateStack> box_states_; base::Optional<NGInlineLayoutStateStack> box_states_;
// The items and its index this context is set up for. // The items and its index this context is set up for.
......
...@@ -64,12 +64,14 @@ NGInlineLayoutAlgorithm::NGInlineLayoutAlgorithm( ...@@ -64,12 +64,14 @@ NGInlineLayoutAlgorithm::NGInlineLayoutAlgorithm(
// lays out in visual order. // lays out in visual order.
TextDirection::kLtr, TextDirection::kLtr,
break_token), break_token),
line_box_(*context->LogicalLineItems()),
box_states_(nullptr), box_states_(nullptr),
context_(context), context_(context),
baseline_type_(container_builder_.Style().GetFontBaseline()), baseline_type_(container_builder_.Style().GetFontBaseline()),
is_horizontal_writing_mode_( is_horizontal_writing_mode_(
blink::IsHorizontalWritingMode(space.GetWritingMode())) { blink::IsHorizontalWritingMode(space.GetWritingMode())) {
DCHECK(context); DCHECK(context);
DCHECK(&line_box_);
quirks_mode_ = inline_node.InLineHeightQuirksMode(); quirks_mode_ = inline_node.InLineHeightQuirksMode();
} }
...@@ -195,7 +197,8 @@ void NGInlineLayoutAlgorithm::CreateLine( ...@@ -195,7 +197,8 @@ void NGInlineLayoutAlgorithm::CreateLine(
NGExclusionSpace* exclusion_space) { NGExclusionSpace* exclusion_space) {
// Needs MutableResults to move ShapeResult out of the NGLineInfo. // Needs MutableResults to move ShapeResult out of the NGLineInfo.
NGInlineItemResults* line_items = line_info->MutableResults(); NGInlineItemResults* line_items = line_info->MutableResults();
line_box_.resize(0); // Clear the current line without releasing the buffer.
line_box_.Shrink(0);
// Apply justification before placing items, because it affects size/position // Apply justification before placing items, because it affects size/position
// of items, which are needed to compute inline static positions. // of items, which are needed to compute inline static positions.
...@@ -1078,7 +1081,7 @@ scoped_refptr<const NGLayoutResult> NGInlineLayoutAlgorithm::Layout() { ...@@ -1078,7 +1081,7 @@ scoped_refptr<const NGLayoutResult> NGInlineLayoutAlgorithm::Layout() {
container_builder_.ToLineBoxFragment(); container_builder_.ToLineBoxFragment();
items_builder->SetCurrentLine( items_builder->SetCurrentLine(
To<NGPhysicalLineBoxFragment>(layout_result->PhysicalFragment()), To<NGPhysicalLineBoxFragment>(layout_result->PhysicalFragment()),
std::move(line_box_)); &line_box_);
return layout_result; return layout_result;
} }
......
...@@ -111,7 +111,7 @@ class CORE_EXPORT NGInlineLayoutAlgorithm final ...@@ -111,7 +111,7 @@ class CORE_EXPORT NGInlineLayoutAlgorithm final
const NGExclusionSpace&, const NGExclusionSpace&,
LayoutUnit line_height); LayoutUnit line_height);
NGLogicalLineItems line_box_; NGLogicalLineItems& line_box_;
NGInlineLayoutStateStack* box_states_; NGInlineLayoutStateStack* box_states_;
NGInlineChildLayoutContext* context_; NGInlineChildLayoutContext* context_;
......
...@@ -241,8 +241,7 @@ class NGLogicalLineItems { ...@@ -241,8 +241,7 @@ class NGLogicalLineItems {
void ReserveInitialCapacity(unsigned capacity) { void ReserveInitialCapacity(unsigned capacity) {
children_.ReserveInitialCapacity(capacity); children_.ReserveInitialCapacity(capacity);
} }
void clear() { children_.resize(0); } void Shrink(wtf_size_t size) { children_.Shrink(size); }
void resize(wtf_size_t size) { children_.resize(size); }
using iterator = Vector<NGLogicalLineItem, 16>::iterator; using iterator = Vector<NGLogicalLineItem, 16>::iterator;
iterator begin() { return children_.begin(); } iterator begin() { return children_.begin(); }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment