[Xfce4-commits] <xfconf:master> Don't use cache singletons.

[Nick Schermer]

Updating branch refs/heads/master
         to e175990a4ea33b92eac71a8819b3a3d121dc879c (commit)
       from dd39de1f20e168ebc4b101b5010ec2f944d86d3e (commit)

commit e175990a4ea33b92eac71a8819b3a3d121dc879c
Author: Nick Schermer <nick at xfce.org>
Date:   Tue Feb 9 19:50:02 2010 +0100

    Don't use cache singletons.
    
    None of the Xfce apps opens more then 2 new channels with
    the same name, so that means we never return an existing
    cache object and use it twice. It also leaked the hash table
    for the cache singletons, so optimize for the 'common case'.
    
    2 of the same caches works fine (but doubles the dbus traffic)
    so to really fix this an app should use the channel singletons.
    Mentioned this in the API docs.

 xfconf/xfconf-cache.c   |   48 +++-------------------------------------------
 xfconf/xfconf-cache.h   |    2 +-
 xfconf/xfconf-channel.c |    6 ++++-
 3 files changed, 10 insertions(+), 46 deletions(-)

diff --git a/xfconf/xfconf-cache.c b/xfconf/xfconf-cache.c
index 5af66a2..a65e2ce 100644
--- a/xfconf/xfconf-cache.c
+++ b/xfconf/xfconf-cache.c
@@ -241,8 +241,6 @@ static void xfconf_cache_property_removed(DBusGProxy *proxy,
 
 
 static guint signals[N_SIGS] = { 0, };
-G_LOCK_DEFINE_STATIC(singletons);
-static GHashTable *singletons = NULL;
 
 
 G_DEFINE_TYPE(XfconfCache, xfconf_cache, G_TYPE_OBJECT)
@@ -416,11 +414,6 @@ xfconf_cache_finalize(GObject *obj)
 
     g_static_mutex_free(&cache->cache_lock);
 
-    G_LOCK(singletons);
-    if(singletons)
-        g_hash_table_remove(singletons, cache);
-    G_UNLOCK(singletons);
-
     G_OBJECT_CLASS(xfconf_cache_parent_class)->finalize(obj);
 }
 
@@ -577,47 +570,14 @@ out:
 }
 #endif
 
-static void
-xfconf_cache_destroyed(gpointer data,
-                       GObject *where_the_object_was)
-{
-    gchar *channel_name = data;
-
-    G_LOCK(singletons);
-    g_hash_table_remove(singletons, channel_name);
-    G_UNLOCK(singletons);
-}
-
 
 
 XfconfCache *
-xfconf_cache_get(const gchar *channel_name)
+xfconf_cache_new(const gchar *channel_name)
 {
-    XfconfCache *cache;
-
-    G_LOCK(singletons);
-
-    if(!singletons) {
-        singletons = g_hash_table_new_full(g_str_hash, g_str_equal,
-                                           (GDestroyNotify)g_free, NULL);
-    }
-
-    cache = g_hash_table_lookup(singletons, channel_name);
-    if(cache)
-        g_object_ref(G_OBJECT(cache));
-    else {
-        gchar *tmp = g_strdup(channel_name);
-
-        cache = g_object_new(XFCONF_TYPE_CACHE,
-                             "channel-name", channel_name,
-                             NULL);
-        g_hash_table_insert(singletons, tmp, cache);
-        g_object_weak_ref(G_OBJECT(cache), xfconf_cache_destroyed, tmp);
-    }
-
-    G_UNLOCK(singletons);
-
-    return cache;
+    return g_object_new(XFCONF_TYPE_CACHE,
+                        "channel-name", channel_name,
+                        NULL);
 }
 
 static void
diff --git a/xfconf/xfconf-cache.h b/xfconf/xfconf-cache.h
index b7d3985..4895953 100644
--- a/xfconf/xfconf-cache.h
+++ b/xfconf/xfconf-cache.h
@@ -37,7 +37,7 @@ G_GNUC_INTERNAL
 GType xfconf_cache_get_type(void) G_GNUC_CONST;
 
 G_GNUC_INTERNAL
-XfconfCache *xfconf_cache_get(const gchar *channel_name);
+XfconfCache *xfconf_cache_new(const gchar *channel_name) G_GNUC_MALLOC;
 
 G_GNUC_INTERNAL
 gboolean xfconf_cache_prefetch(XfconfCache *cache,
diff --git a/xfconf/xfconf-channel.c b/xfconf/xfconf-channel.c
index b9d887f..9ac2d94 100644
--- a/xfconf/xfconf-channel.c
+++ b/xfconf/xfconf-channel.c
@@ -257,7 +257,7 @@ xfconf_channel_constructor(GType type,
     }
 
     if(!channel->cache) {
-        channel->cache = xfconf_cache_get(channel_name);
+        channel->cache = xfconf_cache_new(channel_name);
         xfconf_cache_prefetch(channel->cache, channel->property_base, NULL);
         g_signal_connect(channel->cache, "property-changed",
                          G_CALLBACK(xfconf_channel_property_changed), channel);
@@ -574,6 +574,10 @@ xfconf_channel_get(const gchar *channel_name)
  * lifetime (and thus the lifetime of connected signals and bound
  * #GObject properties) to the lifetime of another object.
  *
+ * Also note that each channel has its own cache, so if you create
+ * 2 new channels with the same name, it will double the dbus traffic,
+ * so in this cases it is highly recommended to use xfconf_channel_get().
+ *
  * Returns: A new #XfconfChannel.  Release with g_object_unref()
  *          when no longer needed.
  **/