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.
Created attachment 122448 [details] Patch I decided to add just one patch instead of multiple patches since this is mainly a mechanical replacement.
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.
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.
Created attachment 123082 [details] Patch for landing
Comment on attachment 123082 [details] Patch for landing Clearing flags on attachment: 123082 Committed r105394: <http://trac.webkit.org/changeset/105394>
All reviewed patches have been landed. Closing bug.