Bug 100893 – RenderArena has a memory leak and poor efficiency

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Chris Evans]

1) Leak!
RenderArena has lifetime tied to a parent Document, but if it grows without bound during that lifetime, it is still a leak -- and one that won't ever get spotted by a memory tool.
The intent is that objects within a RenderArena get re-used after a free, but free sizes were only tracked up to 400 bytes. This was historically adequate but now we have (sizes on 64-bit Linux Chromium):
RenderNamedFlowThread: 720 bytes
RenderSVGText: 480 bytes
Possibly more. So these objects leak by definition. Ugh.

2) Incorrectly sized recycler array on 64-bit.
The granularity of alloc / free size on 64-bit is 8 bytes: sizeof(void*). However, free list buckets are maintained with a granulatity of 4 bytes across all platforms. Seems like the move to 64-bit computing passed RenderArena by :-P

3) Wasteful interaction with underlying memory allocator backing the arena chunks
Whilst the RenderArena constructor has a default size of 4096 bytes, which sounds nice and page-sized for the underlying malloc() allocator, Arena adds a few more bytes to the malloc() size for metadata.
On my system, this leads to malloc(4131).
tcmalloc (as used by Chromium and also Mac for fastMalloc, as is the case here) has size buckets in this vicinity of 4096 and 4608. So malloc(4131) wastes at least 10% of memory -- and that's not counting the non-perfect tesselation of 4608 into page-sized multiples either.

4) Excessive interaction with the underlying memory alloctor
Even a tiny and very simple render (small text file) uses 16k of RenderArena size, so it's wasteful to go into malloc() 4 times. After all, RenderArena is trying to avoid going into malloc() as a goal.
No need to go to town here but the default allocation size of 4096 could use a little upping.

[Chris Evans]

Created attachment 171735 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 171735 [details]
Patch

LGTM.

[Eric Seidel (no email)]

CCing Apple folks who might like to see this go by.

[Eric Seidel (no email)]

It seems horrible to me that each RenderSVGText is nearly a half k. :(

[WebKit Review Bot]

Comment on attachment 171735 [details]
Patch

Clearing flags on attachment: 171735

Committed r133119: <http://trac.webkit.org/changeset/133119>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

(In reply to comment #4)
> It seems horrible to me that each RenderSVGText is nearly a half k. :(

No kidding!

[Chris Evans]

Just for kicks, here's a rough breakdown of RenderSVGText's size:

- Inherits from WebCore::RenderSVGBlock (size: 176 === sizeof(RenderBlock))
- Has member of type WebCore::SVGTextLayoutAttributesBuilder (size: 232)
- Has member of type WebCore::AffineTransform (size: 48)

Interesting that sizeof(WebCore::RenderBlock) isn't exactly small.