After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 599954 - Native RWLock implementation
Native RWLock implementation
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gthread
2.22.x
Other Linux
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on: 585375
Blocks: 557100
 
 
Reported: 2009-10-28 21:29 UTC by Edward Hervey
Modified: 2011-09-30 04:45 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gthread: add GRWMutex prototypes (6.08 KB, patch)
2009-10-28 21:30 UTC, Edward Hervey
none Details | Review
symbols: add new symbol too (689 bytes, patch)
2009-10-28 21:30 UTC, Edward Hervey
none Details | Review
gthread: avoid locking in _get_mutex_impl (1.38 KB, patch)
2009-10-28 21:30 UTC, Edward Hervey
committed Details | Review
gthread: rename functions to match static variants (5.53 KB, patch)
2009-10-28 21:30 UTC, Edward Hervey
none Details | Review
uhm, rename again, now it's consistent (8.36 KB, patch)
2009-10-28 21:30 UTC, Edward Hervey
none Details | Review
use native pthread rwlocks (3.76 KB, patch)
2009-10-28 21:30 UTC, Edward Hervey
none Details | Review
add some test apps. (4.90 KB, patch)
2009-10-28 21:31 UTC, Edward Hervey
none Details | Review
Initial implementation of native GStaticRWLock (10.54 KB, patch)
2009-10-28 21:31 UTC, Edward Hervey
none Details | Review
gthread.h: Fix g_static_rw_lock_* macros (1.80 KB, patch)
2009-10-28 21:31 UTC, Edward Hervey
none Details | Review
glib/gthread.h: *actually* use the shortcut for rw_lock access. (1.79 KB, patch)
2009-10-28 21:31 UTC, Edward Hervey
none Details | Review
glib: Re-insert old API functions to remain ABI compatible. (6.51 KB, patch)
2009-10-28 21:31 UTC, Edward Hervey
none Details | Review

Description Edward Hervey 2009-10-28 21:29:11 UTC
+++ This bug was initially created as a clone of Bug #585375 +++

I took up Wim's branch which added native RWLock and extended it to replace GStaticRWLock to use it.

Patches follow
Comment 1 Edward Hervey 2009-10-28 21:30:28 UTC
Created attachment 146453 [details] [review]
gthread: add GRWMutex prototypes
Comment 2 Edward Hervey 2009-10-28 21:30:32 UTC
Created attachment 146454 [details] [review]
symbols: add new symbol too
Comment 3 Edward Hervey 2009-10-28 21:30:37 UTC
Created attachment 146455 [details] [review]
gthread: avoid locking in _get_mutex_impl

When getting the mutex implementation of a static mutex, avoid taking the global
lock every time but only take the lock when there was no mutex and we need to
create one.
Comment 4 Edward Hervey 2009-10-28 21:30:41 UTC
Created attachment 146456 [details] [review]
gthread: rename functions to match static variants
Comment 5 Edward Hervey 2009-10-28 21:30:47 UTC
Created attachment 146457 [details] [review]
uhm, rename again, now it's consistent
Comment 6 Edward Hervey 2009-10-28 21:30:57 UTC
Created attachment 146458 [details] [review]
use native pthread rwlocks
Comment 7 Edward Hervey 2009-10-28 21:31:01 UTC
Created attachment 146459 [details] [review]
add some test apps.
Comment 8 Edward Hervey 2009-10-28 21:31:07 UTC
Created attachment 146460 [details] [review]
Initial implementation of native GStaticRWLock
Comment 9 Edward Hervey 2009-10-28 21:31:11 UTC
Created attachment 146461 [details] [review]
gthread.h: Fix g_static_rw_lock_* macros
Comment 10 Edward Hervey 2009-10-28 21:31:16 UTC
Created attachment 146462 [details] [review]
glib/gthread.h: *actually* use the shortcut for rw_lock access.
Comment 11 Edward Hervey 2009-10-28 21:31:22 UTC
Created attachment 146463 [details] [review]
glib: Re-insert old API functions to remain ABI compatible.

Only code compiled against the new gthread.h will use the new native
rwlock API.

Old code will still be able to use the old symbols.
Comment 12 Edward Hervey 2009-10-28 21:32:46 UTC
The commits are also available here : http://cgit.freedesktop.org/~bilboed/glib/log/?h=rwlock
Comment 13 Lennart Poettering 2009-10-28 21:58:55 UTC
Two small notes:

AFAIK pthread_rwlock is a relatively new addition to Linux and I think POSIX too. Unless I am misreading you patch compat for systems where these calls do not exist/return ENOSYS is not kept. I personally don't care about breaking that compat at all, but maybe some legacy loving folks might complain.

It might make sense to use pthread_rwlockattr_setkind_np() to make the rwlocks follow the same ordering protocol that was used in the old, fake locks.
Comment 14 Edward Hervey 2009-10-28 23:27:40 UTC
I'll rework the patches to make the old fake rwlock to be the default implementation of GRWlock, that should work on all system afaict, including those that don't have pthread_rwlock.

As for changing the ordering, it looks like we need the PTHREAD_RWLOCK_PREFER_WRITER_NP ordering.

There's actually a initializer for that that we could use : PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP

Not certain it's available on all systems though.

Thanks for the comments
Comment 15 Lennart Poettering 2009-10-28 23:40:43 UTC
(In reply to comment #14)

> There's actually a initializer for that that we could use :
> PTHREAD_RWLOCK_WRITER_NONRECURSIVE_INITIALIZER_NP
> 
> Not certain it's available on all systems though.

The "NP" suffix means it is non-portable. So you need to #ifdef that, so that it is used only when it exists, like on Linux.
Comment 16 Matthias Clasen 2010-08-14 02:48:03 UTC
Edward, did you ever rework your patch series ?
Comment 17 Edward Hervey 2010-08-16 06:25:32 UTC
Thanks for reminding me, got led astray by other parts. No, I still haven't reworked it.
Comment 18 Matthias Clasen 2011-06-04 01:43:34 UTC
Review of attachment 146455 [details] [review]:

Committed this one
Comment 19 Matthias Clasen 2011-09-30 03:53:33 UTC
we have a native rwlock implementation now
Comment 20 Wim Taymans 2011-09-30 04:45:30 UTC
> we have a native rwlock implementation now

And some other very nice GThread cleanups, I an see. Thanks, for the effort!