Commit 5464461e authored by Christian Hergert's avatar Christian Hergert

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
parent 38ffcd29
......@@ -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)
{
......
......@@ -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
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment