[glib] gresource: avoid allocations in enumerate_children()



commit 5464461e4c0eac948a81a8aae6a3ef6774ebe6bb
Author: Christian Hergert <chergert redhat com>
Date:   Sun Nov 12 23:04:12 2017 -0800

    gresource: avoid allocations in enumerate_children()
    
    In the vast majority of cases, we can avoid temporary
    allocations for paths in g_resources_enumerate_children().
    
    In the case we need to add a suffix "/", we can usually just
    build the path on the stack. In other cases, we can completely
    avoid the strdup, which appears to only have been added for
    readability. If the path is really long, we fallback to doing
    what we did before, and use g_strconcat().
    
    In the case of Builder, this saved 5.3mb of temporary
    allocations in the process of showing the first application
    window.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=790275

 gio/gresource.c       |   40 +++++++++++++++++++++++++++++++++++-----
 gio/tests/resources.c |   26 ++++++++++++++++++++++++++
 2 files changed, 61 insertions(+), 5 deletions(-)
---
diff --git a/gio/gresource.c b/gio/gresource.c
index f38de86..8ead26b 100644
--- a/gio/gresource.c
+++ b/gio/gresource.c
@@ -862,9 +862,17 @@ g_resource_enumerate_children (GResource             *resource,
                                GResourceLookupFlags   lookup_flags,
                                GError               **error)
 {
+  gchar local_str[256];
+  const gchar *path_with_slash;
   gchar **children;
+  gchar *free_path = NULL;
   gsize path_len;
-  char *path_with_slash;
+
+  /*
+   * Size of 256 is arbitrarily chosen based on being large enough
+   * for pretty much everything we come across, but not cumbersome
+   * on the stack. It also matches common cacheline sizes.
+   */
 
   if (*path == 0)
     {
@@ -875,13 +883,35 @@ g_resource_enumerate_children (GResource             *resource,
     }
 
   path_len = strlen (path);
-  if (path[path_len-1] != '/')
-    path_with_slash = g_strconcat (path, "/", NULL);
+
+  if G_UNLIKELY (path[path_len-1] != '/')
+    {
+      if (path_len < sizeof (local_str) - 2)
+        {
+          /*
+           * We got a path that does not have a trailing /. It is not the
+           * ideal use of this API as we require trailing / for our lookup
+           * into gvdb. Some degenerate application configurations can hit
+           * this code path quite a bit, so we try to avoid using the
+           * g_strconcat()/g_free().
+           */
+          memcpy (local_str, path, path_len);
+          local_str[path_len] = '/';
+          local_str[path_len+1] = 0;
+          path_with_slash = local_str;
+        }
+      else
+        {
+          path_with_slash = free_path = g_strconcat (path, "/", NULL);
+        }
+    }
   else
-    path_with_slash = g_strdup (path);
+    {
+      path_with_slash = path;
+    }
 
   children = gvdb_table_list (resource->table, path_with_slash);
-  g_free (path_with_slash);
+  g_free (free_path);
 
   if (children == NULL)
     {
diff --git a/gio/tests/resources.c b/gio/tests/resources.c
index 81922d3..8163aa1 100644
--- a/gio/tests/resources.c
+++ b/gio/tests/resources.c
@@ -134,6 +134,32 @@ test_resource (GResource *resource)
   g_assert_no_error (error);
   g_assert_cmpint (g_strv_length (children), ==, 2);
   g_strfreev (children);
+
+  /* Test the preferred lookup where we have a trailing slash. */
+  children = g_resource_enumerate_children  (resource,
+                                            "/a_prefix/",
+                                            G_RESOURCE_LOOKUP_FLAGS_NONE,
+                                            &error);
+  g_assert (children != NULL);
+  g_assert_no_error (error);
+  g_assert_cmpint (g_strv_length (children), ==, 2);
+  g_strfreev (children);
+
+  /* test with a path > 256 and no trailing slash to test the
+   * slow path of resources where we allocate a modified path.
+   */
+  children = g_resource_enumerate_children  (resource,
+                                            "/not/here/not/here/not/here/not/here/not/here/not/here/not/here"
+                                            "/not/here/not/here/not/here/not/here/not/here/not/here/not/here"
+                                            "/not/here/not/here/not/here/not/here/not/here/not/here/not/here"
+                                            "/not/here/not/here/not/here/not/here/not/here/not/here/not/here"
+                                            "/not/here/not/here/not/here/not/here/not/here/not/here/not/here"
+                                            "/with/no/trailing/slash",
+                                            G_RESOURCE_LOOKUP_FLAGS_NONE,
+                                            &error);
+  g_assert (children == NULL);
+  g_assert_error (error, G_RESOURCE_ERROR, G_RESOURCE_ERROR_NOT_FOUND);
+  g_clear_error (&error);
 }
 
 static void


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]