Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(111)

Issue 14093017: Add FINAL decorators to the InlineBox class hierarchy. (Closed)

Created:
7 years, 8 months ago by Chris Evans
Modified:
7 years, 8 months ago
Reviewers:
cevans, esprehn, infero, eseidel
CC:
blink-reviews, Nico
Visibility:
Public.

Description

Add FINAL decorators to the InlineBox class hierarchy. (FINAL is a macro in wtf/Compiler.h that does the correct thing if the compiler does not support "final"). The approach used is as simple as possible whilst being thorough. So, leaf classes have FINAL applied to the whole class whereas intermediary classes have FINAL applied to relevant methods. FINAL allows a compiler to devirtualize call sites and turn them into direct calls. As you might expect, this is perf positive: (clang on Linux): - line_layout.html goes from 120 runs/s -> 123 runs/2, +2.5% - html5-full-render.html goes from 3176ms -> 3162ms, +0.4% I have confidence that the former result is statistically significant (as the numbers are very very stable) but not the latter. Elliott for review of approach, Abhishek as OWNER of rendering. R=esprehn@chromium.org,infero@chromium.org TEST=wktry worked Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=148628

Patch Set 1 #

Patch Set 2 : Rebased. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+45 lines, -45 lines) Patch
M Source/core/rendering/EllipsisBox.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/InlineBox.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/InlineFlowBox.h View 7 chunks +10 lines, -10 lines 0 comments Download
M Source/core/rendering/InlineTextBox.h View 6 chunks +15 lines, -15 lines 0 comments Download
M Source/core/rendering/RootInlineBox.h View 4 chunks +13 lines, -13 lines 0 comments Download
M Source/core/rendering/TrailingFloatsRootInlineBox.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/SVGInlineFlowBox.h View 1 chunk +1 line, -1 line 0 comments Download
M Source/core/rendering/svg/SVGInlineTextBox.h View 2 chunks +2 lines, -2 lines 0 comments Download
M Source/core/rendering/svg/SVGRootInlineBox.h View 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 6 (0 generated)
cevans
7 years, 8 months ago (2013-04-17 22:26:59 UTC) #1
eseidel
lgtm You're really going for a medal here, eh? This is AWESOME.
7 years, 8 months ago (2013-04-18 07:25:12 UTC) #2
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cevans@chromium.org/14093017/1
7 years, 8 months ago (2013-04-18 07:25:22 UTC) #3
commit-bot: I haz the power
Retried try job too often on linux_layout_rel for step(s) test_shell_tests, webkit_lint, webkit_tests, webkit_unit_tests http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_layout_rel&number=4204
7 years, 8 months ago (2013-04-18 07:55:26 UTC) #4
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/cevans@chromium.org/14093017/3002
7 years, 8 months ago (2013-04-18 07:58:44 UTC) #5
commit-bot: I haz the power
7 years, 8 months ago (2013-04-18 08:32:36 UTC) #6
Message was sent while issue was closed.
Change committed as 148628

Powered by Google App Engine
This is Rietveld 408576698