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 724839 - GMainContext: some source ID cleanups
GMainContext: some source ID cleanups
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: mainloop
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2014-02-20 23:06 UTC by Allison Karlitskaya (desrt)
Modified: 2014-02-24 14:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gmain: Simplify source id tracking (5.04 KB, patch)
2014-02-20 23:06 UTC, Allison Karlitskaya (desrt)
committed Details | Review
gmain: simplify g_main_context_find_source_by_id() (1.82 KB, patch)
2014-02-20 23:06 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Allison Karlitskaya (desrt) 2014-02-20 23:06:04 UTC
Simplify the tracking of sources by ID (and also the logic about
reissuing of IDs) by just keeping a hashtable of all sources in the
table.

We're going to need something like this anyway once we start reworking
GMainContext to remove the "pure" (no check/prepare vfunc) sources from
the primary source list that we iterate through each time around.
Comment 1 Allison Karlitskaya (desrt) 2014-02-20 23:06:07 UTC
Created attachment 269845 [details] [review]
gmain: Simplify source id tracking

Simplify our tracking of issued source id integers and fix some bugs.

Previously the source's id was remove from the 'used' table from
source_remove_from_context() which was also called if the source
priority was changed (in which case it would never be added back to the
table).  The source id could be reissued in that case.

In the new approach, we just always keep a hash table of sources, by
source id.  This simplifies the logic and will also allow us to improve
performance of g_main_context_find_source_by_id() which is called in some
fairly common cases, such as g_source_remove().  These improvements will be in
the following commits.
Comment 2 Allison Karlitskaya (desrt) 2014-02-20 23:06:10 UTC
Created attachment 269846 [details] [review]
gmain: simplify g_main_context_find_source_by_id()

Since we now keep a hashtable of sources, we can implement this function
without iteration.
Comment 3 Dan Winship 2014-02-22 15:25:30 UTC
Comment on attachment 269845 [details] [review]
gmain: Simplify source id tracking

makes sense
Comment 4 Dan Winship 2014-02-22 15:29:18 UTC
Comment on attachment 269846 [details] [review]
gmain: simplify g_main_context_find_source_by_id()

Is this significant enough to warrant a README mention?
Comment 5 Allison Karlitskaya (desrt) 2014-02-22 15:32:29 UTC
(In reply to comment #4)
> (From update of attachment 269846 [details] [review])
> Is this significant enough to warrant a README mention?

Why?  For the bugfix?

Other than that, this change should not be visible in any way from the outside (other than improved performance, I guess)...  I doubt that people will really care about one operation on GMainContext suddenly being O(1) when pretty much everything else we do is still O(n)...
Comment 6 Dan Winship 2014-02-22 16:57:23 UTC
For the performance improvement. Some people may have been avoiding the ID-based APIs before because of the O(n)-ness, and now they wouldn't have to. But there's nothing in the docs to indicate that anything changed, so no one will realize that.
Comment 7 Allison Karlitskaya (desrt) 2014-02-24 14:29:47 UTC
Attachment 269845 [details] pushed as 9e81709 - gmain: Simplify source id tracking
Attachment 269846 [details] pushed as 393503b - gmain: simplify g_main_context_find_source_by_id()

I'll hold off the README update until next cycle when I plan to be able to report that almost everything in GMainContext is O(1) instead of O(n). :)