Optimize FloatIntervalSearchAdapter::collectIfNeeded

Optimize FloatIntervalSearchAdapter::collectIfNeeded by reducing calls to logical(Left/Right/Bottom)ForFloat.

Brings average page load time of bookish.com (from tough_layout_cases) from ~4100 ms to ~2800ms on my z620.

BUG=260903
R=eseidel@chromium.org

Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=155958

[eseidel, 2013-08-09 23:15:08]

It looks like you and I are going to conflict:
https://codereview.chromium.org/22493015/

I'm happy to take your change instead of mine. :)

I'm considering making it possible for RenderBlocks which only have overflow
content to not hold a FloatedObjects list.  That's a rather involved change, but
I may attempt it next week.  If so, it should get rid of the O(N^2) behavior on
oilevent at least.

[eseidel, 2013-08-09 23:18:09]

lgtm

[ojan, 2013-08-10 17:26:44]

Change description nit: I believe this patch improves performance, but I doubt
that it's from reducing calls to m_renderer. I expect it's primarily from
removing the duplicate calls to logicalLeftForFloat and logicalRightForFloat,
and possibly from removing the m_heightRemaining member from
FloatIntervalSearchAdapter (do we allocate a lot of those?).

[shatch, 2013-08-12 16:14:35]

On 2013/08/10 17:26:44, ojan wrote:
> Change description nit: I believe this patch improves performance, but I doubt
> that it's from reducing calls to m_renderer. I expect it's primarily from
> removing the duplicate calls to logicalLeftForFloat and logicalRightForFloat,
> and possibly from removing the m_heightRemaining member from
> FloatIntervalSearchAdapter (do we allocate a lot of those?).

Hi ojan! Yeah I suppose my description wasn't totally clear, I'll edit the
description.

[shatch, 2013-08-12 19:51:30]

Rebased and merged with latest, ptal

[eseidel, 2013-08-12 22:13:29]

lgtm.  Lets let Bem lgtm too.  We're hacking on floats today.

https://codereview.chromium.org/22463002/diff/11001/Source/core/rendering/Ren...
File Source/core/rendering/RenderBlock.cpp (right):

https://codereview.chromium.org/22463002/diff/11001/Source/core/rendering/Ren...
Source/core/rendering/RenderBlock.cpp:4283: if (floatIsNewExtreme) {
I believe webkit style drops these {}

[shatch, 2013-08-12 22:20:33]

Cool, waiting for Bem's review!

https://codereview.chromium.org/22463002/diff/11001/Source/core/rendering/Ren...
File Source/core/rendering/RenderBlock.cpp (right):

https://codereview.chromium.org/22463002/diff/11001/Source/core/rendering/Ren...
Source/core/rendering/RenderBlock.cpp:4283: if (floatIsNewExtreme) {
On 2013/08/12 22:13:29, eseidel wrote:
> I believe webkit style drops these {}

Done.

[Bem Jones-Bey (adobe), 2013-08-12 22:28:33]

lgtm. I think that we only need one of m_last and m_floatForHeight, but I'll
work on that.

[commit-bot: I haz the power, 2013-08-12 22:32:30]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/simonhatch@chromium.org/22463002/21001

[eseidel, 2013-08-12 22:43:21]

Committed patchset #4 manually as r155958 (presubmit successful).

[shatch, 2013-08-13 16:30:48]

On 2013/08/12 22:43:21, eseidel wrote:
> Committed patchset #4 manually as r155958 (presubmit successful).

~20% improvement on Layout_floats_50_100 in blink_perf and overall ~8-10 on page
load time for tough_layout_cases! Yay!

https://chromeperf.appspot.com/report?rev=216854&masters=ChromiumPerf&bots=ch...

https://chromeperf.appspot.com/report?masters=ChromiumPerf&bots=android-nexus...