Summary: | [css-grid] Cache definite height detection | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | zsun | ||||
Component: | CSS | Assignee: | Nobody <webkit-unassigned> | ||||
Status: | RESOLVED LATER | ||||||
Severity: | Normal | CC: | changseok, esprehn+autocc, ews-watchlist, glenn, jfernandez, kondapallykalyan, pdr, rego, simon.fraser, svillar, webkit-bug-importer, zalan | ||||
Priority: | P2 | Keywords: | InRadar | ||||
Version: | WebKit Nightly Build | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
zsun
2021-02-03 09:28:58 PST
Correcting above comment: RenderGrid::RenderBlock() -> RenderGrid::layoutBlock() Created attachment 419167 [details]
Patch
Comment on attachment 419167 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419167&action=review The change looks good in general, but since it only makes sense to improve performance, I'd rather justify it with some data from the perf bots. I think we've recently added some changes in the height definiteness logic that could have impacted negatively, so we could monitor the perf tests we have since that change and evaluate the benefit of this patch. > Source/WebCore/rendering/GridTrackSizingAlgorithm.cpp:1119 > + const bool indefiniteHeight = m_direction == ForRows && !m_renderGrid->cachedHasDefiniteLogicalHeight(); Theoretically, it's possible to call the GridTrackSizingAlgorithm method of RenderGrid instance during the execution of its LayoutBlock method (eg, computing the grid's intrinsic size). If I understood correctly this patch, we are initializing the cache during the Layout, so it'd be possible that we call the cachedHasDefiniteLogicalHeight() when the cache Optional value is uninitialized. Shouldn't we add some defensive code to prevent this scenario ? |