Re: Performance ideas (resend) ...

[Matthias Clasen <matthias clasen gmail com>]

On Thu, Mar 5, 2009 at 2:25 PM, Matthias Clasen
<matthias clasen gmail com> wrote:

> Hey, thanks for saving me from reimplementing that. It is pretty much
> exactly what I had in mind.

Except for the fact that Ryans patch doesn't really work... The
problem is that due to the way preloading works on a per-directory
basis, we also need to keep track of the cache status per-directory.
The attached patch seems to work fine in my (cursory) testing. I've
also added a cache-based implementation of gconf_client_all_entries().

It would be interesting to see how this patch affects startup performance.


Matthias

--- GConf-2.25.2/gconf/gconf-client.c	2009-02-16 19:15:18.000000000 -0500
+++ GConf-2.25.2.neg-cache/gconf/gconf-client.c	2009-03-05 22:02:37.602918136 -0500
@@ -234,6 +234,7 @@
   client->error_mode = GCONF_CLIENT_HANDLE_UNRETURNED;
   client->dir_hash = g_hash_table_new (g_str_hash, g_str_equal);
   client->cache_hash = g_hash_table_new (g_str_hash, g_str_equal);
+  client->cache_dirs = g_hash_table_new (g_str_hash, g_str_equal);
   /* We create the listeners only if they're actually used */
   client->listeners = NULL;
   client->notify_list = NULL;
@@ -657,6 +658,19 @@
     return FALSE;
 }
 
+static gboolean
+clear_cache_dirs_foreach (char *key, gpointer value, char *dir)
+{
+  if (strcmp (dir, key) == 0 ||
+      gconf_key_is_below (dir, key))
+    {
+      trace ("'%s' no longer fully cached\n", dir);
+      return TRUE;
+    }
+
+  return FALSE;
+}
+
 static void
 gconf_client_real_remove_dir    (GConfClient* client,
                                  Dir* d,
@@ -683,7 +697,9 @@
   g_hash_table_foreach_remove (client->cache_hash,
                                (GHRFunc)clear_dir_cache_foreach,
                                d->name);
-
+  g_hash_table_foreach_remove (client->cache_dirs,
+                               (GHRFunc)clear_cache_dirs_foreach,
+                               d->name);
   dir_destroy(d);
 
   ad.client = client;
@@ -713,7 +729,7 @@
       found->add_count -= 1;
 
       if (found->add_count == 0) 
-        gconf_client_real_remove_dir(client, found, err);
+        gconf_client_real_remove_dir (client, found, err);
     }
 #ifndef G_DISABLE_CHECKS
   else
@@ -809,8 +825,8 @@
   
   g_hash_table_foreach_remove (client->cache_hash, (GHRFunc)clear_cache_foreach,
                                client);
-
-  g_assert (g_hash_table_size(client->cache_hash) == 0);
+  
+  g_hash_table_remove_all (client->cache_dirs);
 }
 
 static void
@@ -917,6 +933,8 @@
     }
 
   cache_entry_list_destructively (client, pairs);
+  trace ("Mark '%s' as fully cached\n", dir);
+  g_hash_table_insert (client->cache_dirs, g_strdup (dir), GINT_TO_POINTER (1));
 }
 
 void
@@ -1064,16 +1082,31 @@
 {
   GError *error = NULL;
   GSList *retval;
-  
+  int dirlen;
+
   trace ("Getting all values in '%s'\n", dir);
 
-  /* We could just use the cache to get all the entries,
-   * iff we have previously done an all_entries and the
-   * cache hasn't since been tossed out, and if we are monitoring
-   * this directory.
-   * FIXME
-   */
-  
+  if (g_hash_table_lookup (client->cache_dirs, dir))
+    {
+      GHashTableIter iter;
+      gchar *key;
+      GConfEntry *entry;
+
+      trace ("Using cached values\n");
+
+      dirlen = strlen (dir);
+      retval = NULL;
+      g_hash_table_iter_init (&iter, client->cache_hash);
+      while (g_hash_table_iter_next (&iter, &key, &entry))
+        {
+          if (g_str_has_prefix (key, dir) &&
+              key + dirlen == strrchr (key, '/'))
+            retval = g_slist_prepend (retval, gconf_entry_copy (entry));
+        }
+
+      return retval;
+    }
+
   PUSH_USE_ENGINE (client);
   retval = gconf_engine_all_entries (client->engine, dir, &error);
   POP_USE_ENGINE (client);
@@ -1084,8 +1117,12 @@
     return NULL;
 
   if (key_being_monitored (client, dir))
-    cache_entry_list_destructively (client, copy_entry_list (retval));
-  
+    {
+      cache_entry_list_destructively (client, copy_entry_list (retval));
+      trace ("Mark '%s' as fully cached\n", dir);
+      g_hash_table_insert (client->cache_dirs, g_strdup (dir), GINT_TO_POINTER (1));
+    }
+
   return retval;
 }
 
@@ -1224,7 +1261,8 @@
     {
       trace ("%s was in the client-side cache\n", key);
       
-      g_assert (entry != NULL);
+      if (entry == NULL)
+        return NULL; 
       
       if (gconf_entry_get_is_default (entry) && !use_default)
         return NULL;
@@ -1971,7 +2009,26 @@
   entry = g_hash_table_lookup (client->cache_hash, key);
 
   *entryp = entry;
-      
+ 
+  if (!entry)
+  {
+    char *dir, *last_slash;
+
+    dir = g_strdup (key);
+    last_slash = strrchr (dir, '/');
+    g_assert (last_slash != NULL);
+    *last_slash = 0;
+
+    if (g_hash_table_lookup (client->cache_dirs, dir))
+    {
+      g_free (dir);
+      trace ("Negative cache hit on %s\n", key);
+      return TRUE;
+    }
+
+    g_free (dir);
+  }
+
   return entry != NULL;
 }
 
--- GConf-2.25.2/gconf/gconf-client.h	2009-02-16 19:15:18.000000000 -0500
+++ GConf-2.25.2.neg-cache/gconf/gconf-client.h	2009-03-05 22:00:04.779629244 -0500
@@ -100,8 +100,8 @@
   GSList *notify_list;
   guint notify_handler;
   int pending_notify_count;
-  gpointer pad1;
-  int pad2;  
+  GHashTable *cache_dirs;
+  int pad2;
 };
 
 struct _GConfClientClass