[Mesa-dev] [PATCH] mesa: Add new fast mtx_t mutex type for basic use cases

[Emil Velikov]

On 29/01/15 17:14, Kristian Høgsberg wrote:
> On Thu, Jan 29, 2015 at 6:36 AM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> On 28/01/15 05:08, Kristian Høgsberg wrote:
>>> While modern pthread mutexes are very fast, they still incur a call to an
>>> external DSO and overhead of the generality and features of pthread mutexes.
>>> Most mutexes in mesa only needs lock/unlock, and the idea here is that we can
>>> inline the atomic operation and make the fast case just two intructions.
>>> Mutexes are subtle and finicky to implement, so we carefully copy the
>>> implementation from Ulrich Dreppers well-written and well-reviewed paper:
>>>
>>>   "Futexes Are Tricky"
>>>   http://www.akkadia.org/drepper/futex.pdf
>>>
>>> We implement "mutex3", which gives us a mutex that has no syscalls on
>>> uncontended lock or unlock.  Further, the uncontended case boils down to a
>>> cmpxchg and an untaken branch and the uncontended unlock is just a locked decr
>>> and an untaken branch.  We use __builtin_expect() to indicate that contention
>>> is unlikely so that gcc will put the contention code out of the main code
>>> flow.
>>>
>>> A fast mutex only supports lock/unlock, can't be recursive or used with
>>> condition variables.  We keep the pthread mutex implementation around as
>>> full_mtx_t for the few places where we use condition variables or recursive
>>> locking.  For platforms or compilers where futex and atomics aren't available,
>>> mtx_t falls back to the pthread mutex.
>>>
>>> The pthread mutex lock/unlock overhead shows up on benchmarks for CPU bound
>>> applications.  Most CPU bound cases are helped and some of our internal
>>> bind_buffer_object heavy benchmarks gain up to 10%.
>>>
>> Hi Kristian,
>>
>> Can I humbly ask that you split this into two patches - one that
>> introduces the new functions/struct and another one that uses them ?
>> This way it'll be easier if/when things go crazy.
>>
>> Also the patch seems to wonder between posix and win32
>> + typedef full_mtx_t mtx_t;
>>
>> and
>> + typedef mtx_t fast_mtx_t;
>>
>> Looks like a left over from the "should I rename XX variables to fast*
>> or just one to full*" moment :)
> 
> Yeah, that's how it progressed :)  At first I called it fast_mtx_t and
> planned on replacing simple uses of mtx_t one by one. Jordan suggested
> that it'd be easier to make the regular mutex fast and then rename the
> couple of places that use more feature than we provide.  I just
> botched the windows side when I did that.  However, given that the
> patch now reimplements mtx_t, there's really no good way to split up
> introducing the fast mutex and making use of it in multiple patches.
> I guess we can add full_mtx_t in a separate patch, use it in a second
> patch and then finally reimplement mtx_t as a fast mutex.  How does
> that sound?
> 
That sounds great. Thank you.

-Emil