Bug 76265 – Cache RenderStyle pointer as a method to avoid performance regression for region styling

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Mihnea Ovidenie]

During the investigation of regression introduced by region style patch (https://bugs.webkit.org/show_bug.cgi?id=71488), it turned out that an important contribution to the regression was introduced by the layout of RenderObject::style()

inline RenderStyle* RenderObject::style() const
{
    if (!inRenderFlowThread())
        return m_style.get();
    return styleInRegion();
}

During several runs of webkit build in instruments, i was able to identify some areas in which the method of caching RenderStyle pointer and replace style() with the cached pointer seems to fix the performance regression introduced by the modified version of style().

These areas are:
* updateBoxModelInfoFromStyle (RenderBoxModelObject/RenderBox)
* computePreferredLogicalWidths
* styleDidChange/styleWillChange (RenderText/RenderBlock/RenderBoxModelObject/RenderInline/RenderBox)
* RenderBlock layout methods (layoutRunsAndFloats/LineBreaker::nextLineBreak/layoutInlineChildren/layoutBlock)
* RenderBox willBeDestroyed/paintBoxDecorarations/computeRectForRepaint/computeLogicalHeight
* RenderText widthFromCache/width


I intend to create a WebKit bug for each of the above bullets instead of creating a big patch.

[Mihnea Ovidenie]

Created attachment 122448 [details]
Patch

I decided to add just one patch instead of multiple patches since this is mainly a mechanical replacement.

[WebKit Review Bot]

Attachment 122448 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/rendering/RenderBox.cpp:2134:  An else should appear on the same line as the preceding }  [whitespace/newline] [4]
Source/WebCore/rendering/RenderBlock.cpp:105:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/rendering/RenderBlock.cpp:112:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Source/WebCore/rendering/RenderBlock.cpp:114:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:331:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:332:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:333:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:334:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:335:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:336:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:346:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Source/WebCore/rendering/RenderBoxModelObject.cpp:347:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Total errors found: 12 in 7 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Dave Hyatt]

Comment on attachment 122448 [details]
Patch

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

r=me with minor changes suggested.

> Source/WebCore/rendering/RenderBlock.cpp:247
> +    RenderStyle* newStyle = style();

This one isn't buying you anything. Only used once.

> Source/WebCore/rendering/RenderBlock.cpp:2049
> +    RenderStyle* cstyle = child->style();

Call this childStyle please, not cstyle.

> Source/WebCore/rendering/RenderBlock.cpp:2146
> +    if (cstyle->marginAfterCollapse() == MSEPARATE) {

childStyle not cstyle.

[Mihnea Ovidenie]

Created attachment 123082 [details]
Patch for landing

[WebKit Review Bot]

Comment on attachment 123082 [details]
Patch for landing

Clearing flags on attachment: 123082

Committed r105394: <http://trac.webkit.org/changeset/105394>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.