Replace RenderArena with PartitionAlloc.

Replace RenderArena with PartitionAlloc.

This is a significant code clean-up.
In terms of performance, it seems roughly a wash.
Memory usage, however, seems improved. The new allocator releases pages back to
the system, and is an improvement for cases where the render tree is large but
then shrinks during the lifetime of the Document.

See the bug for details of tests run and results.

BUG=252057

[Chris Evans, 2013-06-20 01:21:21]

Looks horrific but is mostly mechanical. Most of the files touched only have
characters removed.

[abarth-chromium, 2013-06-20 01:39:50]

LGTM

You've written this CL in a way that's going to be hard to revert if it goes
wrong.  If you're confident, that's fine.  Otherwise you might consider doing it
in more steps

https://codereview.chromium.org/16896019/diff/1/Source/core/css/CSSCursorImag...
File Source/core/css/CSSCursorImageValue.cpp (right):

https://codereview.chromium.org/16896019/diff/1/Source/core/css/CSSCursorImag...
Source/core/css/CSSCursorImageValue.cpp:207: // TODO: report m_image. It has
never been allocated from any of our rendering custom heaps.
TODO -> FIXME :)

https://codereview.chromium.org/16896019/diff/1/Source/core/css/CSSImageValue...
File Source/core/css/CSSImageValue.cpp (right):

https://codereview.chromium.org/16896019/diff/1/Source/core/css/CSSImageValue...
Source/core/css/CSSImageValue.cpp:110: // TODO: report m_image. It has never
been allocated from any of our rendering custom heaps.
ditto

https://codereview.chromium.org/16896019/diff/1/Source/core/rendering/RenderW...
File Source/core/rendering/RenderWidgetProtector.h (left):

https://codereview.chromium.org/16896019/diff/1/Source/core/rendering/RenderW...
Source/core/rendering/RenderWidgetProtector.h:33: class RenderWidgetProtector {
Isn't this just a RefPtr now?

[esprehn, 2013-06-20 09:15:43]

I don't think this is a good change. The render tree needs to be packed for
performance reasons, now when you have lots of documents in the same render
process all their render trees will be intermingled with each other in the
allocator.

https://codereview.chromium.org/16896019/diff/1/Source/core/rendering/RenderO...
File Source/core/rendering/RenderObject.cpp (right):

https://codereview.chromium.org/16896019/diff/1/Source/core/rendering/RenderO...
Source/core/rendering/RenderObject.cpp:122: return
partitionAlloc(Partitions::getRenderingPartition(), sz);
This is bad, now you don't pack the render tree for each Document. None of the
benchmarks you ran are pages with a lot of iframes where having each of the
render trees all packed together is important.

[esprehn, 2013-06-20 09:20:28]

On 2013/06/20 09:15:43, esprehn wrote:
> ...
> 
>
https://codereview.chromium.org/16896019/diff/1/Source/core/rendering/RenderO...
> Source/core/rendering/RenderObject.cpp:122: return
> partitionAlloc(Partitions::getRenderingPartition(), sz);
> This is bad, now you don't pack the render tree for each Document. None of the
> benchmarks you ran are pages with a lot of iframes where having each of the
> render trees all packed together is important.

btw this may kill us on Android, they use the same render process for many tabs.
Ojan and others have expressed the importance of keeping the render tree packed
together for cpu cache reasons since we walk all over it repeatedly during
layout.

[abarth-chromium, 2013-06-20 15:57:48]

On 2013/06/20 09:20:28, esprehn wrote:
> Ojan and others have expressed the importance of keeping the render tree
packed
> together for cpu cache reasons since we walk all over it repeatedly during
> layout.

Can we write a benchmark that illustrates this issue?  If you look in the linked
bug, Chris did some performance measurements using html5-full-render, but I
don't think he tried targeting the multiple document fragmentation issue.  I'm a
bit skeptical that this issue matters, but I'm willing to be convinced by data
one way or another.

[Chris Evans, 2013-06-20 17:48:50]

On 2013/06/20 15:57:48, abarth wrote:
> On 2013/06/20 09:20:28, esprehn wrote:
> > Ojan and others have expressed the importance of keeping the render tree
> packed
> > together for cpu cache reasons since we walk all over it repeatedly during
> > layout.
> 
> Can we write a benchmark that illustrates this issue?  If you look in the
linked
> bug, Chris did some performance measurements using html5-full-render, but I
> don't think he tried targeting the multiple document fragmentation issue.  I'm
a
> bit skeptical that this issue matters, but I'm willing to be convinced by data
> one way or another.

I'm also skeptical. Furthermore, even if we found e.g. a 3% slowdown in some
benchmark,
I'd say the potential memory saving and code cleanliness would still dominate.

The above said, what I will do is land a hacked-up version of the change behind
a #define
and give the define a land/revert cycle on the real bots which includes page
cyclers on ARM.

[ojan, 2013-07-16 00:56:24]

This landed in a different patch, right? Can we close this issue?

[esprehn, 2013-07-16 01:01:13]

On 2013/07/16 00:56:24, ojan wrote:
> This landed in a different patch, right? Can we close this issue?

We never landed this entire patch, we landed a big chunk of this though I think?
I thought cevans was going to see what his patch did for the perf tests, maybe
we should just land the final bits and see what the graphs do!

[cevans, 2013-07-17 09:23:16]

On 2013/07/16 01:01:13, esprehn wrote:
> On 2013/07/16 00:56:24, ojan wrote:
> > This landed in a different patch, right? Can we close this issue?
> 
> We never landed this entire patch, we landed a big chunk of this though I
think?
> I thought cevans was going to see what his patch did for the perf tests, maybe
> we should just land the final bits and see what the graphs do!

That's right, we landed some of the prep work for this patch but nothing that
changes behaviour yet. The actual switch is pending a couple more performance
measurements that Elliot and I talked about; they're documented in the bug. I'll
finish off the work here when I'm back from vacation.