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 705728 - GLib weak refs depend on cascade of locks, including global ones, which makes them non-scalable
GLib weak refs depend on cascade of locks, including global ones, which makes...
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: gobject
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2013-08-09 16:38 UTC by Maciej (Matthew) Piechotka
Modified: 2018-05-24 15:36 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
0001-Introduce-a-per-GObject-mutex-for-weak-references.patch (16.25 KB, patch)
2013-08-09 16:38 UTC, Maciej (Matthew) Piechotka
none Details | Review
The benchmark (1.88 KB, text/x-csrc)
2013-08-09 16:40 UTC, Maciej (Matthew) Piechotka
  Details

Description Maciej (Matthew) Piechotka 2013-08-09 16:38:18 UTC
Created attachment 251245 [details] [review]
0001-Introduce-a-per-GObject-mutex-for-weak-references.patch

Current implementation of weak refs use a cascade of locks, including global ones. Proposed patch uses a single Object-specific mutex to handle the weak references.

There is slight change in behaviour - previously all weak references were cleared when count reached 0 even if object was resurrected. Now they are not cleared in such case unless they are check in that moment.

Benchmark results - I could not find any statistical difference in time of make check time or time of threadtests. I have constructed a benchmark which used heavy use of weak references and the results were as follows:

+---------+----------------+---------------+
| Threads |     Original   |      Modified |
+---------+----------------+---------------+
|       1 |  1.535 ± 0.008 | 1.413 ± 0.004 |
|       2 |  3.0   ± 0.17  | 1.475 ± 0.006 |
|       4 | 16     ± 1     | 1.79  ± 0.03  |
|       8 | 45     ± 1     | 3.47  ± 0.09  |
+---------+----------------+---------------+

Testing platform: 4 core i7 ivy bridge.
Flags: -O2 -march=native -Wl,-O1 -Wl,--as-needed -Wl,--add-needed -Wl,--hash-style=both -Wl,--sort-common -Wl,--no-keep-memory
Comment 1 Maciej (Matthew) Piechotka 2013-08-09 16:40:14 UTC
Created attachment 251246 [details]
The benchmark
Comment 2 Allison Karlitskaya (desrt) 2013-08-12 09:13:38 UTC
Did you run into this while profiling real world code?
Comment 3 Maciej (Matthew) Piechotka 2013-08-12 09:22:05 UTC
(In reply to comment #2)
> Did you run into this while profiling real world code?

No. I was thinking more about it in terms of usage in multithread collections and across threads in futures.
Comment 4 Matthias Clasen 2013-11-24 02:43:35 UTC
see bug 682849
Comment 5 Maciej (Matthew) Piechotka 2013-11-24 03:22:08 UTC
(In reply to comment #4)
> see bug 682849

It has been some time since I look at the code so sorry for question about clarification - as far as I understand you bug 682849 provides real live example and patch there only fixes g_object_weak_{,un}ref - not g_weak_ref_*.
Comment 6 Matthias Clasen 2013-11-24 14:42:26 UTC
quite possible - I just wanted to establish the connection to that bug, since it talks about eliminating global locks from weak refs.
Comment 7 Maciej (Matthew) Piechotka 2013-11-24 16:12:51 UTC
(In reply to comment #6)
> quite possible - I just wanted to establish the connection to that bug, since
> it talks about eliminating global locks from weak refs.

AFAIK there are 2 types of weak refs and 3 ways of accessing them:

 - g_object_weak_{,un}ref - those methods simply register a callback to be called when object is destroyed. It is not thread safe as callback might be in another thread then accessing (1. Read pointer. 2. Callback setting pointer to NULL. 3. Destroy of object 4. g_object_ref on dead pointer).
 - g_object_{add,remove}_weak_pointer - those methods wrap around g_object_weak_{,un}ref but are not thread safe. The assume that the pointer will be accessed from the same thread as it will be destroyed as the read and refing is not protected.
 - GWeakRef is a new API which is thread safe.

Now patch from bug #682849 removes locks from the non thread-safe API and is less intrusive. the one here is more intrusive (it changes _init, _unref etc.) but protects the thread safe API as well.
Comment 8 GNOME Infrastructure Team 2018-05-24 15:36:42 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/743.