[Webkit-unassigned] [Bug 73712] PODIntervalTree takes 1.7MB memory on www.nytimes.com

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=73712





--- Comment #12 from Kenneth Russell <kbr at google.com>  2011-12-08 19:22:06 PST ---
(In reply to comment #11)
> (In reply to comment #8)
> > (From update of attachment 117731 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=117731&action=review
> > 
> > First, nice work tracking down this issue and proposing a fix.
> > 
> 
> Thanks for the comments!
> 
> > I'm not qualified to review the renderer code; hyatt or darin should.
> > 
> > However, I am concerned about the overall strategy of adding a free list to the PODArena. As indicated above, the currently proposed changes violate invariants, so they will either have to be fixed or a different approach will need to be taken. Now, it's possible to fix the free list code, but a solution that would be more in line with the design of the arena, red/black and interval trees would be to just clear out the arena wholesale at certain points in time. I don't know whether there's a good point in the layout algorithm to do this, but if there is, I would lean toward a solution of that form. That could introduce the possibility of dangling pointers from existing red/black and interval tree instances, and that might be a worse side-effect than the additional complexity of free list maintenance, so the tradeoffs would have to be carefully considered.
> 
> That was also my concern when first looking at the code.  However, it seems like a PODBlackRedTree only allocates fixed size objects from its PODArena, and PODArena is so far only used by PODBlackRedTree.  So this patch makes the assumption that each object allocated from the same arena has the same size, which simplifies the freelist implementation and we don't need to track the size of each cell from a chunk.   I agree it is not future proof if we are to use PODArena with different sized cells.

Again, that's true for its current usage but not true in the general case. See Source/WebCore/platform/graphics/gpu/LoopBlinnPathProcessor.cpp (which is not in use and which is going away) for an example of more general usage.

> I agree.  How about we make a special cased PODArena for the PODIntervalTree that we know for sure each entry will have the same size, and keep the generalize case as it is?  That way we fix the memory issue without violating the invariants of PODArena.

Yes, you could subclass PODArena into PODFreeListArena<T> which only supports allocations and frees of a particular type. But would you give my suggestion about clearing out the arena some thought?

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.