GNOME Bugzilla – Bug 652514
CoglPipeline could embed the list nodes for the children directly in the CoglPipeline struct
Last modified: 2011-07-01 17:04:30 UTC
Currently each CoglPipeline has a GList for the children. This isn't ideal because it means that when a pipeline has more than one child it ends up needing a separate slice allocation for the GList node. We also have no way of getting to the GList node given a pointer to the pipeline so when a pipeline is unparented we have to walk the entire list of children to find the node. One way to fix this would be to use the linked list code from the FreeBSD kernel. The code is neatly self-contained in a single header so it is easy to borrow. All of the code is implemented in macros. The list implementation is more versatile and faster than using GLists when it makes sense to embed the list pointers directly in another struct. The FreeBSD implementation has the following advantages over the Linux implementation: - It's licensed under a BSD license so it's compatible with Cogl's license. - It has singly-linked lists as well as double-linked lists where as Linux only has the latter. - All of the list manipulation macros take the struct field name as a parameter so that they can function in terms of pointers to the outer struct type even if the list node struct isn't the first member of the struct. This also makes it easier to make the struct contain multiple list nodes (ie, so it would belong to multiple lists). - It doesn't appear to use any GCC-isms or C99-isms
Created attachment 189862 [details] [review] Add the code from FreeBSD for their linked-list/queue implementation This directly copies in the header from the FreeBSD kernel for their linked-list implementation. A later patch will modify it but this patch is here so we can have a clear patch to show what the changes are. Using the list implementation from this header is beneficial as opposed to using GList because it's possible to embed the list pointers directly into another struct. This saves a separate allocation and it also makes it possible to remove an item from the list without having to iterate the entire list to find its list node. The header provides four different list types: single and doubley linked lists and each of them can either have a header with pointers to the beginning and end or just to the beginning. Glib effectively only provides single and doubley linked lists with a pointer to the beginning or a doubley-linked list with a pointer to both (GQueue).
Created attachment 189863 [details] [review] cogl-queue: Make suitable for use in Cogl source code This modifies cogl-queue.h so that:- - Everything is in a COGL_* namespace - It assumes there is a typedef for all of the types instead of requiring the code to use struct WhateverType. - It doesn't contain any tabs
Created attachment 189864 [details] [review] cogl-pipeline: Use BSD lists for the list of pipeline children Instead of having a separate GList for the children we can use the linked list code from FreeBSD and store the list node directly in the struct. That way we can avoid having a separate slice allocation for the list node. It also means that we effectively have a pointer to the list node given a pointer to the pipeline node. That means we can unparent a pipeline without having to walk the entire list of children. With this change there is no need to have the optimisation to fast track a pipeline that only has one child which simplifies the code somewhat. With this patch we are removing a pointer and a gboolean from the CoglPipeline struct and adding two pointers. On 32-bit architectures this should end up exactly the same size because a gboolean is the same size as a pointer. On 64-bit architectures it will end up adding 4 bytes.
Moving all bugs assigned to the Cogl component of the Clutter product to be under the general component of the new Cogl product.
Pushed to master as 945a64d..b9b4172 Thinking about it a bit more I think on 64-bit architectures the pipeline node struct is actually 4 bytes smaller not bigger because we are removing two cases of padding for alignment.