Revert "LibWeb: Cache intrinsic sizes across layout runs"

This reverts commit 12c6ac78e2.

Very large performance regression when viewing GitHub repository pages.
This commit is contained in:
Andreas Kling 2025-03-08 12:08:51 +01:00
commit 73a4b176cf
Notes: github-actions[bot] 2025-03-08 11:10:16 +00:00
13 changed files with 102 additions and 108 deletions

View file

@ -962,7 +962,7 @@ void KeyframeEffect::update_computed_properties()
}
if (invalidation.relayout)
target->set_needs_layout_update(DOM::SetNeedsLayoutReason::KeyframeEffect);
document.set_needs_layout(DOM::SetNeedsLayoutReason::KeyframeEffect);
if (invalidation.rebuild_layout_tree)
document.invalidate_layout_tree(DOM::InvalidateLayoutTreeReason::KeyframeEffect);
if (invalidation.repaint) {

View file

@ -147,7 +147,7 @@ WebIDL::ExceptionOr<void> CharacterData::replace_data(size_t offset, size_t coun
static_cast<Layout::TextNode&>(*layout_node).invalidate_text_for_rendering();
// We also need to relayout.
set_needs_layout_update(SetNeedsLayoutReason::CharacterDataReplaceData);
document().set_needs_layout(SetNeedsLayoutReason::CharacterDataReplaceData);
}
document().bump_character_data_version();

View file

@ -1211,6 +1211,20 @@ Optional<String> Document::encoding_parse_and_serialize_url(StringView url) cons
return parsed_url->serialize();
}
void Document::set_needs_layout(SetNeedsLayoutReason reason)
{
if (m_needs_layout)
return;
if constexpr (UPDATE_LAYOUT_DEBUG) {
// NOTE: We check some conditions here to avoid debug spam in documents that don't do layout.
auto navigable = this->navigable();
if (m_layout_root && navigable && navigable->active_document() == this)
dbgln_if(UPDATE_LAYOUT_DEBUG, "NEED LAYOUT {}", to_string(reason));
}
m_needs_layout = true;
schedule_layout_update();
}
void Document::invalidate_layout_tree(InvalidateLayoutTreeReason reason)
{
if (m_layout_root)
@ -1285,7 +1299,7 @@ void Document::update_layout(UpdateLayoutReason reason)
update_style();
if (!m_needs_layout_update && m_layout_root)
if (!m_needs_layout && m_layout_root)
return;
// NOTE: If this is a document hosting <template> contents, layout is unnecessary.
@ -1315,16 +1329,11 @@ void Document::update_layout(UpdateLayoutReason reason)
}
}
// Assign each box that establishes a formatting context a list of absolutely positioned children it should take care of during layout
m_layout_root->for_each_in_inclusive_subtree_of_type<Layout::Box>([&](auto& child) {
bool needs_layout_update = child.dom_node() && child.dom_node()->needs_layout_update();
if (needs_layout_update || child.is_anonymous()) {
child.reset_cached_intrinsic_sizes();
}
child.clear_contained_abspos_children();
return TraversalDecision::Continue;
});
// Assign each box that establishes a formatting context a list of absolutely positioned children it should take care of during layout
m_layout_root->for_each_in_inclusive_subtree([&](auto& child) {
if (!child.is_absolutely_positioned())
return TraversalDecision::Continue;
@ -1387,10 +1396,7 @@ void Document::update_layout(UpdateLayoutReason reason)
paintable()->recompute_selection_states(*range);
}
for_each_shadow_including_inclusive_descendant([](auto& node) {
node.reset_needs_layout_update();
return TraversalDecision::Continue;
});
m_needs_layout = false;
// Scrolling by zero offset will clamp scroll offset back to valid range if it was out of bounds
// after the viewport size change.
@ -1424,9 +1430,6 @@ void Document::update_layout(UpdateLayoutReason reason)
}
is_display_none = static_cast<Element&>(node).computed_properties()->display().is_none();
}
if (node_invalidation.relayout) {
node.set_needs_layout_update(SetNeedsLayoutReason::StyleChange);
}
if (node_invalidation.rebuild_layout_tree) {
// We mark layout tree for rebuild starting from parent element to correctly invalidate
// "display" property change to/from "contents" value.
@ -1438,7 +1441,6 @@ void Document::update_layout(UpdateLayoutReason reason)
}
invalidation |= node_invalidation;
node.set_needs_style_update(false);
invalidation |= node_invalidation;
bool children_need_inherited_style_update = !invalidation.is_none();
if (needs_full_style_update || node.child_needs_style_update() || children_need_inherited_style_update) {
@ -1530,6 +1532,8 @@ void Document::update_style()
auto invalidation = update_style_recursively(*this, style_computer(), false);
if (!invalidation.is_none())
invalidate_display_list();
if (invalidation.relayout)
set_needs_layout(SetNeedsLayoutReason::StyleChange);
if (invalidation.rebuild_stacking_context_tree)
invalidate_stacking_context_tree();
m_needs_full_style_update = false;

View file

@ -50,6 +50,26 @@ enum class QuirksMode {
Yes
};
#define ENUMERATE_SET_NEEDS_LAYOUT_REASONS(X) \
X(CharacterDataReplaceData) \
X(FinalizeACrossDocumentNavigation) \
X(HTMLImageElementReactToChangesInTheEnvironment) \
X(HTMLImageElementUpdateTheImageData) \
X(HTMLVideoElementSetVideoTrack) \
X(KeyframeEffect) \
X(LayoutTreeUpdate) \
X(NavigableSetViewportSize) \
X(SVGImageElementFetchTheDocument) \
X(StyleChange)
enum class SetNeedsLayoutReason {
#define ENUMERATE_SET_NEEDS_LAYOUT_REASON(e) e,
ENUMERATE_SET_NEEDS_LAYOUT_REASONS(ENUMERATE_SET_NEEDS_LAYOUT_REASON)
#undef ENUMERATE_SET_NEEDS_LAYOUT_REASON
};
[[nodiscard]] StringView to_string(SetNeedsLayoutReason);
#define ENUMERATE_INVALIDATE_LAYOUT_TREE_REASONS(X) \
X(DocumentAddAnElementToTheTopLayer) \
X(DocumentRequestAnElementToBeRemovedFromTheTopLayer) \
@ -338,6 +358,8 @@ public:
void update_paint_and_hit_testing_properties_if_needed();
void update_animated_style_if_needed();
void set_needs_layout(SetNeedsLayoutReason);
void invalidate_layout_tree(InvalidateLayoutTreeReason);
void invalidate_stacking_context_tree();
@ -1054,6 +1076,8 @@ private:
// Used by evaluate_media_queries_and_report_changes().
Vector<WeakPtr<CSS::MediaQueryList>> m_media_query_lists;
bool m_needs_layout { false };
bool m_needs_full_style_update { false };
bool m_needs_full_layout_tree_update { false };

View file

@ -1405,7 +1405,7 @@ void Node::set_needs_layout_tree_update(bool value)
break;
ancestor->m_child_needs_layout_tree_update = true;
}
set_needs_layout_update(SetNeedsLayoutReason::LayoutTreeUpdate);
document().set_needs_layout(SetNeedsLayoutReason::LayoutTreeUpdate);
}
}
@ -1425,27 +1425,6 @@ void Node::set_needs_style_update(bool value)
}
}
void Node::set_needs_layout_update(SetNeedsLayoutReason reason)
{
if (m_needs_layout_update)
return;
if constexpr (UPDATE_LAYOUT_DEBUG) {
// NOTE: We check some conditions here to avoid debug spam in documents that don't do layout.
auto navigable = this->navigable();
if (navigable && navigable->active_document() == this)
dbgln_if(UPDATE_LAYOUT_DEBUG, "NEED LAYOUT {}", to_string(reason));
}
m_needs_layout_update = true;
for (auto* ancestor = parent_or_shadow_host(); ancestor; ancestor = ancestor->parent_or_shadow_host()) {
if (ancestor->m_needs_layout_update)
break;
ancestor->m_needs_layout_update = true;
}
}
void Node::post_connection()
{
}

View file

@ -91,26 +91,6 @@ enum class StyleInvalidationReason {
#undef __ENUMERATE_STYLE_INVALIDATION_REASON
};
#define ENUMERATE_SET_NEEDS_LAYOUT_REASONS(X) \
X(CharacterDataReplaceData) \
X(FinalizeACrossDocumentNavigation) \
X(HTMLImageElementReactToChangesInTheEnvironment) \
X(HTMLImageElementUpdateTheImageData) \
X(HTMLVideoElementSetVideoTrack) \
X(KeyframeEffect) \
X(LayoutTreeUpdate) \
X(NavigableSetViewportSize) \
X(SVGImageElementFetchTheDocument) \
X(StyleChange)
enum class SetNeedsLayoutReason {
#define ENUMERATE_SET_NEEDS_LAYOUT_REASON(e) e,
ENUMERATE_SET_NEEDS_LAYOUT_REASONS(ENUMERATE_SET_NEEDS_LAYOUT_REASON)
#undef ENUMERATE_SET_NEEDS_LAYOUT_REASON
};
[[nodiscard]] StringView to_string(SetNeedsLayoutReason);
class Node : public EventTarget
, public TreeNode<Node> {
WEB_PLATFORM_OBJECT(Node, EventTarget);
@ -311,10 +291,6 @@ public:
void set_needs_style_update(bool);
void set_needs_style_update_internal(bool) { m_needs_style_update = true; }
bool needs_layout_update() const { return m_needs_layout_update; }
void set_needs_layout_update(SetNeedsLayoutReason);
void reset_needs_layout_update() { m_needs_layout_update = false; }
bool child_needs_style_update() const { return m_child_needs_style_update; }
void set_child_needs_style_update(bool b) { m_child_needs_style_update = b; }
@ -552,8 +528,6 @@ protected:
bool m_child_needs_style_update { false };
bool m_entire_subtree_needs_style_update { false };
bool m_needs_layout_update { false };
UniqueNodeID m_unique_id;
// https://dom.spec.whatwg.org/#registered-observer-list

View file

@ -762,7 +762,7 @@ void HTMLImageElement::add_callbacks_to_image_request(GC::Ref<ImageRequest> imag
document().list_of_available_images().add(key, *image_data, true);
set_needs_style_update(true);
set_needs_layout_update(DOM::SetNeedsLayoutReason::HTMLImageElementUpdateTheImageData);
document().set_needs_layout(DOM::SetNeedsLayoutReason::HTMLImageElementUpdateTheImageData);
// 4. If maybe omit events is not set or previousURL is not equal to urlString, then fire an event named load at the img element.
if (!maybe_omit_events || previous_url != url_string.serialize())
@ -902,7 +902,7 @@ void HTMLImageElement::react_to_changes_in_the_environment()
image_request->prepare_for_presentation(*this);
// FIXME: This is ad-hoc, updating the layout here should probably be handled by prepare_for_presentation().
set_needs_style_update(true);
set_needs_layout_update(DOM::SetNeedsLayoutReason::HTMLImageElementReactToChangesInTheEnvironment);
document().set_needs_layout(DOM::SetNeedsLayoutReason::HTMLImageElementReactToChangesInTheEnvironment);
// 7. Fire an event named load at the img element.
dispatch_event(DOM::Event::create(realm(), HTML::EventNames::load));

View file

@ -111,7 +111,7 @@ u32 HTMLVideoElement::video_height() const
void HTMLVideoElement::set_video_track(GC::Ptr<HTML::VideoTrack> video_track)
{
set_needs_style_update(true);
set_needs_layout_update(DOM::SetNeedsLayoutReason::HTMLVideoElementSetVideoTrack);
document().set_needs_layout(DOM::SetNeedsLayoutReason::HTMLVideoElementSetVideoTrack);
if (m_video_track)
m_video_track->pause_video({});

View file

@ -2100,7 +2100,7 @@ void finalize_a_cross_document_navigation(GC::Ref<Navigable> navigable, HistoryH
// AD-HOC: If we're inside a navigable container, let's trigger a relayout in the container document.
// This allows size negotiation between the containing document and SVG documents to happen.
if (auto container = navigable->container()) {
container->set_needs_layout_update(DOM::SetNeedsLayoutReason::FinalizeACrossDocumentNavigation);
container->document().set_needs_layout(DOM::SetNeedsLayoutReason::FinalizeACrossDocumentNavigation);
}
}
@ -2223,7 +2223,7 @@ void Navigable::set_viewport_size(CSSPixelSize size)
if (auto document = active_document()) {
// NOTE: Resizing the viewport changes the reference value for viewport-relative CSS lengths.
document->invalidate_style(DOM::StyleInvalidationReason::NavigableSetViewportSize);
document->set_needs_layout_update(DOM::SetNeedsLayoutReason::NavigableSetViewportSize);
document->set_needs_layout(DOM::SetNeedsLayoutReason::NavigableSetViewportSize);
}
if (auto document = active_document()) {

View file

@ -17,13 +17,6 @@ struct LineBoxFragmentCoordinate {
size_t fragment_index { 0 };
};
struct IntrinsicSizes {
Optional<CSSPixels> min_content_width;
Optional<CSSPixels> max_content_width;
HashMap<CSSPixels, Optional<CSSPixels>> min_content_height;
HashMap<CSSPixels, Optional<CSSPixels>> max_content_height;
};
class Box : public NodeWithStyleAndBoxModelMetrics {
GC_CELL(Box, NodeWithStyleAndBoxModelMetrics);
@ -60,14 +53,6 @@ public:
virtual void visit_edges(Cell::Visitor&) override;
IntrinsicSizes& cached_intrinsic_sizes() const
{
if (!m_cached_intrinsic_sizes)
m_cached_intrinsic_sizes = make<IntrinsicSizes>();
return *m_cached_intrinsic_sizes;
}
void reset_cached_intrinsic_sizes() const { m_cached_intrinsic_sizes.clear(); }
protected:
Box(DOM::Document&, DOM::Node*, GC::Ref<CSS::ComputedProperties>);
Box(DOM::Document&, DOM::Node*, NonnullOwnPtr<CSS::ComputedValues>);
@ -80,8 +65,6 @@ private:
Optional<CSSPixelFraction> m_natural_aspect_ratio;
Vector<GC::Ref<Node>> m_contained_abspos_children;
OwnPtr<IntrinsicSizes> mutable m_cached_intrinsic_sizes;
};
template<>

View file

@ -1442,9 +1442,11 @@ CSSPixels FormattingContext::calculate_min_content_width(Layout::Box const& box)
if (box.has_natural_width())
return *box.natural_width();
auto& cache = box.cached_intrinsic_sizes().min_content_width;
if (cache.has_value())
return cache.value();
auto& root_state = m_state.m_root;
auto& cache = *root_state.intrinsic_sizes.ensure(&box, [] { return adopt_own(*new LayoutState::IntrinsicSizes); });
if (cache.min_content_width.has_value())
return *cache.min_content_width;
LayoutState throwaway_state(&m_state);
@ -1464,9 +1466,8 @@ CSSPixels FormattingContext::calculate_min_content_width(Layout::Box const& box)
context->run(AvailableSpace(available_width, available_height));
auto min_content_width = clamp_to_max_dimension_value(context->automatic_content_width());
cache.emplace(min_content_width);
return min_content_width;
cache.min_content_width = clamp_to_max_dimension_value(context->automatic_content_width());
return *cache.min_content_width;
}
CSSPixels FormattingContext::calculate_max_content_width(Layout::Box const& box) const
@ -1474,9 +1475,11 @@ CSSPixels FormattingContext::calculate_max_content_width(Layout::Box const& box)
if (box.has_natural_width())
return *box.natural_width();
auto& cache = box.cached_intrinsic_sizes().max_content_width;
if (cache.has_value())
return cache.value();
auto& root_state = m_state.m_root;
auto& cache = *root_state.intrinsic_sizes.ensure(&box, [] { return adopt_own(*new LayoutState::IntrinsicSizes); });
if (cache.max_content_width.has_value())
return *cache.max_content_width;
LayoutState throwaway_state(&m_state);
@ -1496,9 +1499,8 @@ CSSPixels FormattingContext::calculate_max_content_width(Layout::Box const& box)
context->run(AvailableSpace(available_width, available_height));
auto max_content_width = clamp_to_max_dimension_value(context->automatic_content_width());
cache.emplace(max_content_width);
return max_content_width;
cache.max_content_width = clamp_to_max_dimension_value(context->automatic_content_width());
return *cache.max_content_width;
}
// https://www.w3.org/TR/css-sizing-3/#min-content-block-size
@ -1511,9 +1513,14 @@ CSSPixels FormattingContext::calculate_min_content_height(Layout::Box const& box
if (box.has_natural_height())
return *box.natural_height();
auto cache_slot = box.cached_intrinsic_sizes().min_content_height.ensure(width);
if (cache_slot.has_value())
return cache_slot.value();
auto get_cache_slot = [&]() -> Optional<CSSPixels>* {
auto& root_state = m_state.m_root;
auto& cache = *root_state.intrinsic_sizes.ensure(&box, [] { return adopt_own(*new LayoutState::IntrinsicSizes); });
return &cache.min_content_height.ensure(width);
};
if (auto* cache_slot = get_cache_slot(); cache_slot && cache_slot->has_value())
return cache_slot->value();
LayoutState throwaway_state(&m_state);
@ -1530,7 +1537,9 @@ CSSPixels FormattingContext::calculate_min_content_height(Layout::Box const& box
context->run(AvailableSpace(AvailableSize::make_definite(width), AvailableSize::make_min_content()));
auto min_content_height = clamp_to_max_dimension_value(context->automatic_content_height());
cache_slot.emplace(min_content_height);
if (auto* cache_slot = get_cache_slot()) {
*cache_slot = min_content_height;
}
return min_content_height;
}
@ -1542,9 +1551,14 @@ CSSPixels FormattingContext::calculate_max_content_height(Layout::Box const& box
if (box.has_natural_height())
return *box.natural_height();
auto& cache_slot = box.cached_intrinsic_sizes().max_content_height.ensure(width);
if (cache_slot.has_value())
return cache_slot.value();
auto get_cache_slot = [&]() -> Optional<CSSPixels>* {
auto& root_state = m_state.m_root;
auto& cache = *root_state.intrinsic_sizes.ensure(&box, [] { return adopt_own(*new LayoutState::IntrinsicSizes); });
return &cache.max_content_height.ensure(width);
};
if (auto* cache_slot = get_cache_slot(); cache_slot && cache_slot->has_value())
return cache_slot->value();
LayoutState throwaway_state(&m_state);
@ -1561,7 +1575,11 @@ CSSPixels FormattingContext::calculate_max_content_height(Layout::Box const& box
context->run(AvailableSpace(AvailableSize::make_definite(width), AvailableSize::make_max_content()));
auto max_content_height = clamp_to_max_dimension_value(context->automatic_content_height());
cache_slot.emplace(max_content_height);
if (auto* cache_slot = get_cache_slot()) {
*cache_slot = max_content_height;
}
return max_content_height;
}

View file

@ -221,6 +221,18 @@ struct LayoutState {
HashMap<GC::Ref<Layout::Node const>, NonnullOwnPtr<UsedValues>> used_values_per_layout_node;
// We cache intrinsic sizes once determined, as they will not change over the course of a full layout.
// This avoids computing them several times while performing flex layout.
struct IntrinsicSizes {
Optional<CSSPixels> min_content_width;
Optional<CSSPixels> max_content_width;
HashMap<CSSPixels, Optional<CSSPixels>> min_content_height;
HashMap<CSSPixels, Optional<CSSPixels>> max_content_height;
};
HashMap<GC::Ptr<NodeWithStyle const>, NonnullOwnPtr<IntrinsicSizes>> mutable intrinsic_sizes;
LayoutState const* m_parent { nullptr };
LayoutState const& m_root;

View file

@ -168,7 +168,7 @@ void SVGImageElement::fetch_the_document(URL::URL const& url)
m_animation_timer->start();
}
set_needs_style_update(true);
set_needs_layout_update(DOM::SetNeedsLayoutReason::SVGImageElementFetchTheDocument);
document().set_needs_layout(DOM::SetNeedsLayoutReason::SVGImageElementFetchTheDocument);
dispatch_event(DOM::Event::create(realm(), HTML::EventNames::load));
},