On Thu, Jan 29, 2015 at 6:36 AM, Emil Velikov <emil.l.veli...@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? Kristian _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev