[Pixman] [RFC] Performance statistics analyzer

Maarten Bosmans mkbosmans at gmail.com
Wed Sep 14 09:16:25 PDT 2011


2011/9/14 Andrea Canciani <ranma42 at gmail.com>:
> On Wed, Sep 7, 2011 at 6:46 AM, Maarten Bosmans <mkbosmans at gmail.com> wrote:
>> I took this RFC to also be a request for patches.
>> Attached is a patch that strips out the pthread mutex usage and
>> implements some macros in pixman-compiler.h, not unlike the TLS
>> macros. I also added a win32 implementation and tested it using mingw.
>
> Is there any reason to use win32 Mutexes instead of CriticalRegions?
> AFAIK Mutexes are for inter-process synchronization, while each
> CriticalRegion can only be used by a single process to synchronize
> multiple threads (and has a better performance, because most of the
> times it does not require calling into the kernel).

Not really, other than this was the method already used for TLS in
pixman-compiler.h.
But then again, it doesn't look like that code was really used
anywhere, because for mingw on GCC you would have
TOOLCHAIN_SUPPORTS__THREAD defined and MSVC has it's own TLS
implementation.

Anyway, sure, a critical section can be used. If we go that route, I
would like to change the semantics of PIXMAN_MUTEX_INIT a bit from
this patch. It would be better to avoid the whole
InterlockedCompareExchangePointer thing and have a simple
#define PIXMAN_MUTEX_INIT(name) InitializeCriticalSection (name ## _mutex)
So instead of a static variable declaration, PIXMAN_MUTEX_INIT would
become a function.

>> Some further improvements could be: check whether the win32
>> implementation works with MSVC and use it there too. And also a way of
>> opting out if you're on a non-win32, non-pthread platform and don't
>> want multithreading.
>
> It does not work with MSVC because it obvously does not provide the
> __MINGW32__ definition. Replacing it with _WIN32 fixes that build

I didn't want to enable it for MSVC too because I couldn't test that.
But sure, _WIN32 is better.

> error but still dies in pixman-fast-path.c because "window.h" defines
> IN and OUT. I have a patch for that in my simpleops branch, I'll try
> cherry-picking it.

That can be prevented by #define WIN32_LEAN_AND_MEAN before including windows.h.

Maarten


More information about the Pixman mailing list