[Cogl] [PATCH] Adds internal CoglMemoryStack utility API

[Robert Bragg]

On Fri, Apr 20, 2012 at 1:42 PM, Neil Roberts <neil at linux.intel.com> wrote:
> Robert Bragg <robert at sixbynine.org> writes:
>
>> + * We currently use this in our tesselator which has to allocate
>> + * lots of small vertex, edge and face structures but once tesselation
>> + * has been finished we just want to free the whole lot in one go.
>
> It isn't used in the tesselator yet, right?

nope, I've updated this comment, thanks.

>
>> +  guint8 *data;
>
> guint8 is banned!

right, the patch was written before guint8 was banned :-)

>
>> +typedef struct _CoglMemoryStack
>> +{
>> +  GQueue *sub_stacks;
>
> Maybe it doesn't matter much, but it might be nicer to use a COGL_TAILQ
> here instead of a GQueue. The CoglMemorySubStacks will always be in a
> list so it seems to make sense to use an embedded list node. Or at the
> very least this might as well use an inline GQueue instead of a pointer
> to a separately allocated GQueue.

ok, makes sense, I've updated the patch to use the TAILQ macros.

>
> If we allocate the data for the sub stack directly in the same
> allocation as the CoglMemorySubStack struct (ie, over-allocate the
> struct) then we could avoid using glib's slice allocator altogether.
> That might be nice if we eventually want to use this as a basis for a
> replacement for using GSlice.

For now I haven't done this since I didn't want to make the code more
fiddly than it needed to be a.t.m. If necessary this can be done later
as a separate patch.

>
>> +  _cogl_memory_stack_add_sub_stack (stack, MAX (sub_stack->bytes,
>>     bytes) * 2);
>
> It might be nice to ensure that the sub stack size is always a power of
> two. Not sure if that really matters though.

I don't know what that would help with exactly except for avoiding
fragmentation? I think the approach of doubling the sub-stack sizes
each time should also minimize fragmentation overall.

This did make me think though that maybe we should round up the
sub-stack sizes to multiples of a 4k page since e.g. glibc documents
that it will allocate larger sizes via mmap and I think it's safe to
assume the OS is going to be dealing with pages. Now though I'm
starting to think that I shouldn't have done that and maybe I should
go back to how it was. I don't think we should be guessing what any
particular malloc() is optimized for and it's possible for some page
aligned sizes that it's the exact worst size to ask for (e.g. if
malloc wants to over allocate for book keeping.).

The one thing we can know for sure if we round to a power of two or to
multiples of a page is that we will consume more memory and end up
with more page faults.

>
> It took me a while to understand in what situations the rest of the sub
> stacks will be searched. Maybe it could do with a comment above the for
> loop to say this will only be searched after a rewind and in that case
> all of the subsequent stacks will be empty.

right, I've added some comments to that code now.

kind regards,
- Robert

>
> Regards,
> - Neil