Skip to content

GitLab

T
tangled
Commit 179b2705 authored by Ian Kilpatrick's avatar Ian Kilpatrick Committed by Commit Bot
Browse files
Options

[LayoutNG] Improve handling of many floats within one single FC. 

This patch improves the performance of lots of floats within a single
formatting context.

Previously when we built the DerivedGeometry (the data-structure used
for querying information), we naively placed all floats into this
data-structure (which is relatively expensive to build).

This introduced a performance regression in the site within
crbug.com/1081477 .

This patch introduces a "ceiling" for the DerivedGeometry class, that is
the highest block-offset it is used to answer queries for. It is
extremely rare to go "backwards" and query for something higher than
what we've just created.

This is used to only all floats below this ceiling substantially
reducing the cost of building the DerivedGeometry.

Added floats_show_hide.html which has improved 50% locally. On the real
world site there is approx. a 20x improvement.

Bug: 1081477
Change-Id: I7b3ced903e2218f13a852bb6487271afd50d30d8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2313476
Commit-Queue: Ian Kilpatrick <ikilpatrick@chromium.org>
Reviewed-by: default avatarChristian Biesinger <cbiesinger@chromium.org>
Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Cr-Commit-Position: refs/heads/master@{#791655}
parent cdea1e19
Hide whitespace changes
Inline Side-by-side
Showing with 138 additions and 16 deletions
third_party/blink/perf_tests/layout/floats_show_hide.html 0 → 100644
View file @ 179b2705
<!DOCTYPE html>
<style>
.float {
float: left;
width: 20px;
}
</style>
<script src="../resources/runner.js"></script>
<body style="overflow-y: scroll;">
<div id="parent" style="display: flow-root;"></div>
</body>
<script>
const parent = document.getElementById('parent');
const containers = [];
for (let i = 0; i < 100; i++) {
const container = document.createElement('div');
container.style.height = '100px';
const random = Math.random();
container.innerHTML = `
<div class="float" style="height: ${90 * random}px"></div>
<div class="float" style="height: ${80 * random}px"></div>
<div class="float" style="height: ${70 * random}px"></div>
<div class="float" style="height: ${60 * random}px"></div>
<div class="float" style="height: ${50 * random}px"></div>
<div class="float" style="height: ${40 * random}px"></div>
<div class="float" style="height: ${30 * random}px"></div>
<div class="float" style="height: ${20 * random}px"></div>
<div class="float" style="height: ${10 * random}px"></div>
`;
parent.appendChild(container);
containers.push(container);
}
PerfTestRunner.measureRunsPerSecond({
description: 'Measures formance of showing/hiding containers with many floats',
run: () => {
for (let container of containers) {
container.style.display = 'none';
PerfTestRunner.forceLayout();
container.style.display = '';
PerfTestRunner.forceLayout();
}
},
done: () => {
parent.style.display = 'none';
}
});
</script>
third_party/blink/renderer/core/layout/ng/exclusions/ng_exclusion.h
View file @ 179b2705
...@@ -54,6 +54,7 @@ struct CORE_EXPORT NGExclusion : public RefCounted<NGExclusion> { ...@@ -54,6 +54,7 @@ struct CORE_EXPORT NGExclusion : public RefCounted<NGExclusion> {
const NGBfcRect rect; const NGBfcRect rect;
const EFloat type; const EFloat type;
bool is_past_other_exclusions = false;
const std::unique_ptr<NGExclusionShapeData> shape_data; const std::unique_ptr<NGExclusionShapeData> shape_data;
bool operator==(const NGExclusion& other) const; bool operator==(const NGExclusion& other) const;
......
third_party/blink/renderer/core/layout/ng/exclusions/ng_exclusion_space.cc
View file @ 179b2705
...@@ -196,8 +196,10 @@ NGExclusionSpaceInternal& NGExclusionSpaceInternal::operator=( ...@@ -196,8 +196,10 @@ NGExclusionSpaceInternal& NGExclusionSpaceInternal::operator=(
NGExclusionSpaceInternal&&) noexcept = default; NGExclusionSpaceInternal&&) noexcept = default;
NGExclusionSpaceInternal::DerivedGeometry::DerivedGeometry( NGExclusionSpaceInternal::DerivedGeometry::DerivedGeometry(
LayoutUnit block_offset_limit,
bool track_shape_exclusions) bool track_shape_exclusions)
: track_shape_exclusions_(track_shape_exclusions) { : block_offset_limit_(block_offset_limit),
track_shape_exclusions_(track_shape_exclusions) {
// The exclusion space must always have at least one shelf, at -Infinity. // 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_); track_shape_exclusions_);
...@@ -230,8 +232,16 @@ void NGExclusionSpaceInternal::Add(scoped_refptr<const NGExclusion> exclusion) { ...@@ -230,8 +232,16 @@ void NGExclusionSpaceInternal::Add(scoped_refptr<const NGExclusion> exclusion) {
derived_geometry_ = nullptr; derived_geometry_ = nullptr;
} }
if (derived_geometry_) LayoutUnit exclusion_block_offset = exclusion->rect.BlockStartOffset();
derived_geometry_->Add(*exclusion);
// We can safely mutate the exclusion here as an exclusion will never be
// reused if this invariant doesn't hold.
const_cast<NGExclusion*>(exclusion.get())->is_past_other_exclusions =
exclusion_block_offset >= left_clear_offset_ &&
exclusion_block_offset >= right_clear_offset_;
last_float_block_start_ =
std::max(last_float_block_start_, exclusion_block_offset);
// Update the members used for clearance calculations. // Update the members used for clearance calculations.
LayoutUnit clear_offset = exclusion->rect.BlockEndOffset(); LayoutUnit clear_offset = exclusion->rect.BlockEndOffset();
...@@ -240,8 +250,8 @@ void NGExclusionSpaceInternal::Add(scoped_refptr<const NGExclusion> exclusion) { ...@@ -240,8 +250,8 @@ void NGExclusionSpaceInternal::Add(scoped_refptr<const NGExclusion> exclusion) {
else if (exclusion->type == EFloat::kRight) else if (exclusion->type == EFloat::kRight)
right_clear_offset_ = std::max(right_clear_offset_, clear_offset); right_clear_offset_ = std::max(right_clear_offset_, clear_offset);
last_float_block_start_ = if (derived_geometry_)
std::max(last_float_block_start_, exclusion->rect.BlockStartOffset()); derived_geometry_->Add(*exclusion);
if (!already_exists) if (!already_exists)
exclusions_->data.emplace_back(std::move(exclusion)); exclusions_->data.emplace_back(std::move(exclusion));
...@@ -250,6 +260,8 @@ void NGExclusionSpaceInternal::Add(scoped_refptr<const NGExclusion> exclusion) { ...@@ -250,6 +260,8 @@ void NGExclusionSpaceInternal::Add(scoped_refptr<const NGExclusion> exclusion) {
void NGExclusionSpaceInternal::DerivedGeometry::Add( void NGExclusionSpaceInternal::DerivedGeometry::Add(
const NGExclusion& exclusion) { const NGExclusion& exclusion) {
DCHECK_GE(exclusion.rect.BlockStartOffset(), block_offset_limit_);
// If the exclusion takes up no inline space, we shouldn't pay any further // If the exclusion takes up no inline space, we shouldn't pay any further
// attention to it. The only thing it can affect is block-axis positioning of // attention to it. The only thing it can affect is block-axis positioning of
// subsequent floats (dealt with above). // subsequent floats (dealt with above).
...@@ -519,6 +531,7 @@ NGExclusionSpaceInternal::DerivedGeometry::FindLayoutOpportunity( ...@@ -519,6 +531,7 @@ NGExclusionSpaceInternal::DerivedGeometry::FindLayoutOpportunity(
const LayoutUnit available_inline_size, const LayoutUnit available_inline_size,
const LayoutUnit minimum_inline_size) const { const LayoutUnit minimum_inline_size) const {
// TODO(ikilpatrick): Determine what to do for a -ve available_inline_size. // TODO(ikilpatrick): Determine what to do for a -ve available_inline_size.
DCHECK_GE(offset.block_offset, block_offset_limit_);
NGLayoutOpportunity return_opportunity; NGLayoutOpportunity return_opportunity;
IterateAllLayoutOpportunities( IterateAllLayoutOpportunities(
...@@ -548,6 +561,7 @@ LayoutOpportunityVector ...@@ -548,6 +561,7 @@ LayoutOpportunityVector
NGExclusionSpaceInternal::DerivedGeometry::AllLayoutOpportunities( NGExclusionSpaceInternal::DerivedGeometry::AllLayoutOpportunities(
const NGBfcOffset& offset, const NGBfcOffset& offset,
const LayoutUnit available_inline_size) const { const LayoutUnit available_inline_size) const {
DCHECK_GE(offset.block_offset, block_offset_limit_);
LayoutOpportunityVector opportunities; LayoutOpportunityVector opportunities;
// This method is only used for determining the position of line-boxes. // This method is only used for determining the position of line-boxes.
...@@ -626,14 +640,55 @@ void NGExclusionSpaceInternal::DerivedGeometry::IterateAllLayoutOpportunities( ...@@ -626,14 +640,55 @@ void NGExclusionSpaceInternal::DerivedGeometry::IterateAllLayoutOpportunities(
} }
const NGExclusionSpaceInternal::DerivedGeometry& const NGExclusionSpaceInternal::DerivedGeometry&
NGExclusionSpaceInternal::GetDerivedGeometry() const { NGExclusionSpaceInternal::GetDerivedGeometry(
LayoutUnit block_offset_limit) const {
// We might have a geometry, but built at a lower block-offset limit.
if (derived_geometry_ &&
block_offset_limit < derived_geometry_->block_offset_limit_)
derived_geometry_ = nullptr;
// Re-build the geometry if it isn't present. // Re-build the geometry if it isn't present.
if (!derived_geometry_) { if (!derived_geometry_) {
derived_geometry_ =
std::make_unique<DerivedGeometry>(track_shape_exclusions_);
DCHECK_LE(num_exclusions_, exclusions_->data.size()); DCHECK_LE(num_exclusions_, exclusions_->data.size());
for (wtf_size_t i = 0; i < num_exclusions_; ++i) DCHECK_GE(num_exclusions_, 1u);
derived_geometry_->Add(*exclusions_->data[i]);
const auto* begin = exclusions_->data.begin();
const auto* end = begin + num_exclusions_;
DCHECK_LE(end, exclusions_->data.end());
// Find the first exclusion whose block-start offset is "after" the
// |block_offset_limit|.
auto* it = std::lower_bound(
begin, end, block_offset_limit,
[](const auto& exclusion, const auto& block_offset) -> bool {
return exclusion->rect.BlockStartOffset() < block_offset;
});
if (it == begin) {
block_offset_limit = LayoutUnit::Min();
} else {
#if DCHECK_IS_ON()
if (it != end)
DCHECK_GE((*it)->rect.BlockStartOffset(), block_offset_limit);
#endif
// Find the "highest" exclusion possible which itself is past other
// exclusions.
while (--it != begin) {
if ((*it)->is_past_other_exclusions)
break;
}
// This exclusion must be above the given block-offset limit.
DCHECK_LE((*it)->rect.BlockStartOffset(), block_offset_limit);
block_offset_limit = (*it)->rect.BlockStartOffset();
}
// Add all the exclusions below the block-offset limit.
derived_geometry_ = std::make_unique<DerivedGeometry>(
block_offset_limit, track_shape_exclusions_);
for (; it < end; ++it)
derived_geometry_->Add(**it);
} }
return *derived_geometry_; return *derived_geometry_;
......
third_party/blink/renderer/core/layout/ng/exclusions/ng_exclusion_space.h
View file @ 179b2705
...@@ -51,8 +51,9 @@ class CORE_EXPORT NGExclusionSpaceInternal { ...@@ -51,8 +51,9 @@ class CORE_EXPORT NGExclusionSpaceInternal {
return NGLayoutOpportunity(NGBfcRect(offset, end_offset), nullptr); return NGLayoutOpportunity(NGBfcRect(offset, end_offset), nullptr);
} }
return GetDerivedGeometry().FindLayoutOpportunity( return GetDerivedGeometry(offset.block_offset)
offset, available_inline_size, minimum_inline_size); .FindLayoutOpportunity(offset, available_inline_size,
minimum_inline_size);
} }
LayoutOpportunityVector AllLayoutOpportunities( LayoutOpportunityVector AllLayoutOpportunities(
...@@ -69,8 +70,8 @@ class CORE_EXPORT NGExclusionSpaceInternal { ...@@ -69,8 +70,8 @@ class CORE_EXPORT NGExclusionSpaceInternal {
{NGLayoutOpportunity(NGBfcRect(offset, end_offset), nullptr)}); {NGLayoutOpportunity(NGBfcRect(offset, end_offset), nullptr)});
} }
return GetDerivedGeometry().AllLayoutOpportunities(offset, return GetDerivedGeometry(offset.block_offset)
available_inline_size); .AllLayoutOpportunities(offset, available_inline_size);
} }
LayoutUnit ClearanceOffset(EClear clear_type) const { LayoutUnit ClearanceOffset(EClear clear_type) const {
...@@ -343,7 +344,17 @@ class CORE_EXPORT NGExclusionSpaceInternal { ...@@ -343,7 +344,17 @@ class CORE_EXPORT NGExclusionSpaceInternal {
USING_FAST_MALLOC(DerivedGeometry); USING_FAST_MALLOC(DerivedGeometry);
public: public:
explicit DerivedGeometry(bool track_shape_exclusions); // |block_offset_limit| represents the highest block-offset for which the
// geometry is valid. |FindLayoutOpportunity| and |AllLayoutOpportunities|
// should not be called for a block-offset higher than this.
// If |NGExclusionSpaceInternal::GetDerivedGeometry| is called with a
// higher limit the geometry is rebuilt.
//
// |track_shape_exclusions| is used to tell the geometry to track shape
// exclusions. Tracking shape exclusions is expensive, and uncommon, so
// when an exclusion with a shape is added we rebuilt the geometry to track
// this.
DerivedGeometry(LayoutUnit block_offset_limit, bool track_shape_exclusions);
DerivedGeometry(DerivedGeometry&& o) noexcept = default; DerivedGeometry(DerivedGeometry&& o) noexcept = default;
void Add(const NGExclusion& exclusion); void Add(const NGExclusion& exclusion);
...@@ -379,12 +390,18 @@ class CORE_EXPORT NGExclusionSpaceInternal { ...@@ -379,12 +390,18 @@ class CORE_EXPORT NGExclusionSpaceInternal {
// created, it can never change. // created, it can never change.
Vector<NGClosedArea, 4> areas_; Vector<NGClosedArea, 4> areas_;
// This represents the highest block-offset for which the geometry is valid
// for. If |NGExclusionSpaceInternal::GetDerivedGeometry| is called with a
// higher limit it is rebuilt.
LayoutUnit block_offset_limit_;
bool track_shape_exclusions_; bool track_shape_exclusions_;
}; };
// Returns the derived_geometry_ member, potentially re-built from the // Returns the derived_geometry_ member, potentially re-built from the
// exclusions_, and num_exclusions_ members. // exclusions_, and num_exclusions_ members.
const DerivedGeometry& GetDerivedGeometry() const; const DerivedGeometry& GetDerivedGeometry(
LayoutUnit block_offset_limit) const;
// See DerivedGeometry struct description. // See DerivedGeometry struct description.
mutable std::unique_ptr<DerivedGeometry> derived_geometry_; mutable std::unique_ptr<DerivedGeometry> derived_geometry_;
......
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