Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(167)

Issue 35333003: Intentionally bloat RenderObject (Closed)

Created:
7 years, 3 months ago by Julien - ping for review
Modified:
7 years, 3 months ago
CC:
blink-reviews, eae+blinkwatch, leviw+renderwatch, jchaffraix+rendering, zoltan1, vclarke, jyasskin (not chrome), jsbell, antonm
Visibility:
Public.

Description

Intentionally bloat RenderObject This has been talked about for years that we should never grow RenderObject or else we would regress performance. We have enough coverage to catch and quantify any performance hit so let's land this and rock 'n' roll. Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=160402

Patch Set 1 #

Total comments: 1

Patch Set 2 : Updated after pdr's review #

Unified diffs Side-by-side diffs Delta from patch set Stats (+7 lines, -0 lines) Patch
M Source/core/rendering/RenderObject.h View 1 1 chunk +5 lines, -0 lines 0 comments Download
M Source/core/rendering/RenderObject.cpp View 1 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
Julien - ping for review
7 years, 3 months ago (2013-10-22 23:13:21 UTC) #1
esprehn
lgtm
7 years, 3 months ago (2013-10-22 23:14:36 UTC) #2
pdr.
LGTM https://codereview.chromium.org/35333003/diff/1/Source/core/rendering/RenderObject.h File Source/core/rendering/RenderObject.h (right): https://codereview.chromium.org/35333003/diff/1/Source/core/rendering/RenderObject.h#newcode1204 Source/core/rendering/RenderObject.h:1204: LayoutRect m_newAbsoluteBoundingRect; Can you add a comment that ...
7 years, 3 months ago (2013-10-22 23:16:01 UTC) #3
eseidel
Please CC the perf sheriffs. :)
7 years, 3 months ago (2013-10-23 00:32:45 UTC) #4
leviw_travelin_and_unemployed
And memory?
7 years, 3 months ago (2013-10-23 00:35:57 UTC) #5
Jeffrey Yasskin
On 2013/10/23 00:35:57, Levi wrote: > And memory? FWIW, the memory bots won't care about ...
7 years, 3 months ago (2013-10-23 00:39:16 UTC) #6
Julien - ping for review
Adding the gardeners as it could impact the roll.
7 years, 3 months ago (2013-10-23 22:26:09 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/jchaffraix@chromium.org/35333003/80001
7 years, 3 months ago (2013-10-24 00:27:21 UTC) #8
commit-bot: I haz the power
Change committed as 160402
7 years, 3 months ago (2013-10-24 01:34:59 UTC) #9
antonm
7 years, 3 months ago (2013-10-24 01:53:56 UTC) #10
+ Matt who kindly took my shift.

On Wed, Oct 23, 2013 at 6:35 PM,  <commit-bot@chromium.org> wrote:
> Change committed as 160402
>
> https://codereview.chromium.org/35333003/

To unsubscribe from this group and stop receiving emails from it, send an email
to blink-reviews+unsubscribe@chromium.org.

Powered by Google App Engine
This is Rietveld 408576698