GNOME Bugzilla – Bug 731986
GLib: implement GMutex natively on Linux
Last modified: 2015-06-19 14:25:22 UTC
If we have futex(2) then we can implement GMutex natively and gain a substantial performance increase (vs. using pthreads). This also avoids the need to allocate an extra structure in memory when using GMutex or GCond: we can use the structure directly. The main reason for the increase in performance is that our implementation can be made more simple: we don't need to support the array of options on pthread_mutex_t (which includes the possibility, for example, of being recursive). The result is a ~30% improvement in uncontended cases and a much larger increase (3 to 4 times) in contended cases for a simple testcase.
Created attachment 278866 [details] [review] GLib: implement GMutex natively on Linux
Created attachment 278867 [details] [review] GLib: implement GMutex natively on Linux (just putting this here in case it comes in handy some day) See as well: https://bugs.debian.org/731509
It would be nice if this could at least optionally do error checking, to avoid stuff like bug 678758. "Optionally" meaning configurable at runtime. (In reply to comment #2) > See as well: https://bugs.debian.org/731509 url typo?
Re bug 678758, we could easily add a check for "unlock the unlocked" without degrading the performance of the fast path: we do a replace and ensure that the old value was 1 for the fast path before dispatching the slow path. All we'd have to do is add a check on the slow path for it having been 0, and throw an abort. As for the url, I believe it to be correct. Why do you think this is a typo? This is the only report I could find about getenv() not working inside of ifuncs...
ah, I didn't look in detail at the obsoleted patch, so I was assuming you meant to link to some debian bug that had something to do with futexes
Created attachment 279801 [details] [review] GMutex (linux): detect and report some errors Detect the following two errors: - attempting to unlock a mutex that is not locked - attempting to clear a mutex that was not initialised or was initialised but is still locked Both of these are fatal errors. We avoid using g_error() here because doing so would involve calls back into the GMutex code, and if things are going off the rails then we want to avoid that.
Created attachment 279981 [details] [review] GCond (linux): fix g_cond_wait_until() return value on timeout It should return FALSE on timeout (and only on timeout), and TRUE otherwise. Includes addition to the unit tests.
This is great stuff. It improves performance of GStreamer's main data processing code paths (gst_pad_push()) by just over 20% according to my measurements. The code looks good to me, didn't spot anything wrong, with the exception of the g_cond_wait_until() return value (see patch above). Two minor niggles: the error fprintf() lines should have a trailing newline, and there's a code style inconsistency, sometimes you use foo->i and sometimes &foo->i[0]. I prefer &foo->i[0] since it makes it clearer that only one int is used here. I have run these patches through the 'make check' test suites of all the main GStreamer modules (GStreamer core, gst-plugins-base, gst-plugins-good, gst-plugins-ugly, gst-plugins-bad, gst-libav, gst-rtsp-server), and 'make check-torture' (= run 'make check' 10 times in a loop) now passes successfully on all of them after I fixed a few bugs in GStreamer elements or tests (double unlocks). Would be great if we could get this into the next release.
Created attachment 280004 [details] [review] GLib: implement GMutex natively on Linux If we have futex(2) then we can implement GMutex natively and gain a substantial performance increase (vs. using pthreads). This also avoids the need to allocate an extra structure in memory when using GMutex or GCond: we can use the structure directly. The main reason for the increase in performance is that our implementation can be made more simple: we don't need to support the array of options on pthread_mutex_t (which includes the possibility, for example, of being recursive). The result is a ~30% improvement in uncontended cases and a much larger increase (3 to 4 times) in contended cases for a simple testcase.
Created attachment 280005 [details] [review] GMutex (linux): detect and report some errors Detect the following two errors: - attempting to unlock a mutex that is not locked - attempting to clear a mutex that was not initialised or was initialised but is still locked Both of these are fatal errors. We avoid using g_error() here because doing so would involve calls back into the GMutex code, and if things are going off the rails then we want to avoid that.
Created attachment 280006 [details] [review] GCond (linux): fix g_cond_wait_until() return value on timeout It should return FALSE on timeout (and only on timeout), and TRUE otherwise.
Review of attachment 280004 [details] [review]: ok.
Review of attachment 280005 [details] [review]: ok
Review of attachment 280006 [details] [review]: ok
Attachment 280004 [details] pushed as 49b59e5 - GLib: implement GMutex natively on Linux Attachment 280005 [details] pushed as ecf1359 - GMutex (linux): detect and report some errors Attachment 280006 [details] pushed as 636cd00 - GCond (linux): fix g_cond_wait_until() return value on timeout
Does this mean that GRWLock is now potentially less efficient than GMutex on Linux? Can similar gains be made in GRWLock?