From 5464461e4c0eac948a81a8aae6a3ef6774ebe6bb Mon Sep 17 00:00:00 2001
From: Christian Hergert <chergert@redhat.com>
Date: Sun, 12 Nov 2017 23:04:12 -0800
Subject: [PATCH] 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 f38de86f5..8ead26bc6 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 81922d3f8..8163aa141 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