Bug 117637 – Improve the number of style resolutions for NodeRenderingContext

• Status: RESOLVED
• Resolution: DUPLICATE
• Component: WebCore Misc.

[Mihai Maerean]

Answer Simon Fraser's concerns raised on https://bugs.webkit.org/show_bug.cgi?id=74144 that there could be less calls to styleForRenderer() in NodeRenderingContext.

[Mihai Maerean]

Created attachment 204813 [details]
patch

This patch reduces the number of style resolutions (an expensive operation) by checking for
moveToFlowThreadIfNeeded in 2 steps:
* 1st step: we determine if the node is the child of a region.
* 2nd step: after calling shouldCreateRenderer, we call moveToFlowThreadIfNeeded for all other nodes and, by
that time, the style has already been computed.

The number of style resolutions are as follows:
WHEN OPENING                            STYLE RESOLUTIONS WITH THE _OLD_ CODE   STYLE RESOLUTIONS WITH THE _PATCH_
http://www.whitehouse.gov/              4027                                    3257
http://en.wikipedia.org/wiki/Main_Page  7513                                    5638

The results of running PerformanceTests/Parser/ are as follows:
TEST                            UNIT    NEW CODE    DELTA           OLD CODE
/Parser/textarea-parsing:Runs   runs/s  77.32       5.72% BETTER    73.14
/Parser/tiny-innerHTML:Runs     runs/s  3.65        4.81% BETTER    3.48
/Parser/html-parser:Time        ms      2423.15     0.49% BETTER    2435.15
/Parser/css-parser-yui:Runs     runs/s  280.09      0.44% BETTER    278.87
/Parser/xml-parser:Runs         runs/s  6.08        4.85% WORSE     6.39
/Parser/innerHTML-setter:Runs   runs/s  185.71      4.61% WORSE     194.69

[Simon Fraser (smfr)]

Comment on attachment 204813 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=204813&action=review

I find it very hard to follow this code, because both Element and NodeRenderContext have moveToFlowThread* functions. I think some renaming first would improve understanding of the code.

> Source/WebCore/dom/NodeRenderingContext.cpp:250
> +    bool haveTestedForMoveToFlowThread = false;
> +    for (const Element* parentElement = m_node->parentElement(); parentElement; parentElement = parentElement->parentElement()) {

You're adding a parent-chain walk here, which I assume will kick in for every renderer creation, which is bad too.

[Radu Stavila]

Considering https://bugs.webkit.org/show_bug.cgi?id=118300, is this issue still valid?

[Mihai Maerean]

*** This bug has been marked as a duplicate of bug 118300 ***