[glib/wip/gmaincontext: 1/12] gmain: Simplify source id tracking

[Ryan Lortie <desrt src gnome org>]

commit 344be664b2fc0c4a421c132e8f55f02ccc7a9e66
Author: Ryan Lortie <desrt desrt ca>
Date:   Sat Feb 8 12:17:10 2014 +0000

    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.

 glib/gmain.c |   77 +++++++++++++--------------------------------------------
 1 files changed, 18 insertions(+), 59 deletions(-)
---
diff --git a/glib/gmain.c b/glib/gmain.c
index 6bb726e..2f166fb 100644
--- a/glib/gmain.c
+++ b/glib/gmain.c
@@ -236,11 +236,12 @@ struct _GMainContext
 
   gint ref_count;
 
+  GHashTable *sources;              /* guint -> GSource */
+
   GPtrArray *pending_dispatches;
   gint timeout;                        /* Timeout for current iteration */
 
   guint next_id;
-  GHashTable *overflow_used_source_ids; /* set<guint> */
   GList *source_lists;
   gint in_check_or_prepare;
 
@@ -542,8 +543,7 @@ g_main_context_unref (GMainContext *context)
     }
   g_list_free (context->source_lists);
 
-  if (context->overflow_used_source_ids)
-    g_hash_table_destroy (context->overflow_used_source_ids);
+  g_hash_table_destroy (context->sources);
 
   g_mutex_clear (&context->mutex);
 
@@ -598,6 +598,7 @@ g_main_context_new (void)
   g_mutex_init (&context->mutex);
   g_cond_init (&context->cond);
 
+  context->sources = g_hash_table_new (NULL, NULL);
   context->owner = NULL;
   context->waiters = NULL;
 
@@ -1055,61 +1056,6 @@ source_remove_from_context (GSource      *source,
       context->source_lists = g_list_remove (context->source_lists, source_list);
       g_slice_free (GSourceList, source_list);
     }
-
-  if (context->overflow_used_source_ids)
-    g_hash_table_remove (context->overflow_used_source_ids,
-                         GUINT_TO_POINTER (source->source_id));
-  
-}
-
-static void
-assign_source_id_unlocked (GMainContext   *context,
-                           GSource        *source)
-{
-  guint id;
-
-  /* Are we about to overflow back to 0? 
-   * See https://bugzilla.gnome.org/show_bug.cgi?id=687098
-   */
-  if (G_UNLIKELY (context->next_id == G_MAXUINT &&
-                  context->overflow_used_source_ids == NULL))
-    {
-      GSourceIter iter;
-      GSource *source;
-
-      context->overflow_used_source_ids = g_hash_table_new (NULL, NULL);
-  
-      g_source_iter_init (&iter, context, FALSE);
-      while (g_source_iter_next (&iter, &source))
-        {
-          g_hash_table_add (context->overflow_used_source_ids,
-                            GUINT_TO_POINTER (source->source_id));
-        }
-      id = G_MAXUINT;
-      g_hash_table_add (context->overflow_used_source_ids, GUINT_TO_POINTER (id));
-    }
-  else if (context->overflow_used_source_ids == NULL)
-    {
-      id = context->next_id++;
-    }
-  else
-    {
-      /*
-       * If we overran G_MAXUINT, we fall back to randomly probing the
-       * source ids for the current context.  This will be slower the more
-       * sources there are, but we're mainly concerned right now about
-       * correctness and code size.  There's time for a more clever solution
-       * later.
-       */
-      do
-        id = g_random_int ();
-      while (id == 0 ||
-             g_hash_table_contains (context->overflow_used_source_ids,
-                                    GUINT_TO_POINTER (id)));
-      g_hash_table_add (context->overflow_used_source_ids, GUINT_TO_POINTER (id));
-    }
-
-  source->source_id = id;
 }
 
 static guint
@@ -1118,10 +1064,21 @@ g_source_attach_unlocked (GSource      *source,
                           gboolean      do_wakeup)
 {
   GSList *tmp_list;
+  guint id;
+
+  /* The counter may have wrapped, so we must ensure that we do not
+   * reuse the source id of an existing source.
+   */
+  do
+    id = context->next_id++;
+  while (id == 0 || g_hash_table_contains (context->sources, GUINT_TO_POINTER (id)));
 
   source->context = context;
-  assign_source_id_unlocked (context, source);
+  source->source_id = id;
   source->ref_count++;
+
+  g_hash_table_insert (context->sources, GUINT_TO_POINTER (id), source);
+
   source_add_to_context (source, context);
 
   if (!SOURCE_BLOCKED (source))
@@ -1997,6 +1954,8 @@ g_source_unref_internal (GSource      *source,
          if (!SOURCE_DESTROYED (source))
            g_warning (G_STRLOC ": ref_count == 0, but source was still attached to a context!");
          source_remove_from_context (source, context);
+
+          g_hash_table_remove (context->sources, GUINT_TO_POINTER (source->source_id));
        }
 
       if (source->source_funcs->finalize)