RESOLVED FIXED 76265
Cache RenderStyle pointer as a method to avoid performance regression for region styling 
https://bugs.webkit.org/show_bug.cgi?id=76265
Summary Cache RenderStyle pointer as a method to avoid performance regression for reg...
Mihnea Ovidenie
Reported 2012-01-13 04:32:25 PST
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.
Attachments
Patch (71.88 KB, patch)
2012-01-13 10:23 PST, Mihnea Ovidenie 		no flags
DetailsFormatted DiffDiff
Patch for landing (73.72 KB, patch)
2012-01-18 23:48 PST, Mihnea Ovidenie 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Mihnea Ovidenie
Comment 1 2012-01-13 10:23:57 PST
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
Comment 2 2012-01-13 10:26:39 PST
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 3 2012-01-18 11:22:04 PST
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
Comment 4 2012-01-18 23:48:30 PST
Created attachment 123082 [details] Patch for landing
WebKit Review Bot
Comment 5 2012-01-19 01:23:16 PST
Comment on attachment 123082 [details] Patch for landing Clearing flags on attachment: 123082 Committed r105394: <http://trac.webkit.org/changeset/105394>
WebKit Review Bot
Comment 6 2012-01-19 01:23:21 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.