Code review comment for lp:~vanvugt/compiz-core/fix-940139

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

On Mon, 27 Feb 2012, Daniel van Vugt wrote:

> You mean +1 to remove the #if's or should we keep them?
>

Remove.

> The performance implication is huge. Compiz was spending most of its time in malloc/free, which was mostly due to "new PrivateRegion" and "delete PrivateRegion". So PrivateRegion had to go. Although after this fix most of compiz' CPU usage is still due to excessive malloc/free/new/deletes, at least it's no longer mostly due to CompRegion.
>

Ok, I can buy the malloc/free argument. The semantics behind CompRegionRef
still feel strange to me though (can't have more than one reference, this
is a thing, or this is a pointer to a thing etc). Have you looked into
what performance benefits could be gained with boost memory pools? It
should be possible to have a static memory pool private to CompRegion
something like this:
http://www.niksula.hut.fi/~hkankaan/Homepages/fastallocation.html

>
> --
> https://code.launchpad.net/~vanvugt/compiz-core/fix-940139/+merge/94715
> Your team Compiz Maintainers is requested to review the proposed merge of lp:~vanvugt/compiz-core/fix-940139 into lp:compiz-core.
>

« Back to merge proposal