[webkit-dev] Arena is crufty?

[Maciej Stachowiak]

On Sep 2, 2010, at 8:51 AM, Chris Marrin wrote:

> 
> On Sep 1, 2010, at 7:20 PM, Kenneth Russell wrote:
> 
>> I would be happy to not add another Arena client, but the primary
>> reason I need an arena is not just for performance but to avoid having
>> to keep track of all of the objects I need to delete.
>> 
>> Is there any consensus yet on how to proceed with
>> https://bugs.webkit.org/show_bug.cgi?id=45059 ? I'm concerned about
>> taking on large-scale restructuring with potential performance impact
>> as a prerequisite for my landing any initial code. I could revert my
>> PODArena class to use its own memory allocation rather than that in
>> Arena.h.
> 
> I just posted that it seems like your RB tree could be replaced by std::multimap. And, given comments from others, it seems like the right thing to do with Arena is to put PODArena into the gpu directory like you were originally going to do, but to not use Arena.h (suck it's functionality into PODArena). Alternately, you could try Jeremy's idea and ref count your objects. If you use std::multimap, elements can be of type RefPtr<something>, so you can avoid all memory management issues.

We usually don't use STL containers because they can raise exceptions and it's WebKit policy to not use exceptions in code, and to support building with exceptions disabled.

Long ago, WebCore had a red-black tree class written by me but we managed to replace all uses with hashtables. I am assuming in this case a balanced tree is genuinely needed. If that is the case, I'd much prefer to see one that supports more than just POD types; a limitation to POD types is a significant loss of generality. It is not hard to make a balanced tree class that supports arbitrary C++ types, in fact it would be the natural way to code it.

If the arena exists to avoid memory management bookkeeping and not for performance reasons, then I'm not sure that is a very good reason to have it. If it is required for performance, then it is reasonable to make it an implementation detail of the RB tree.

Regards,
Maciej