From 1ac23d298a53f6c764e9d8abf0304eed318deddc Mon Sep 17 00:00:00 2001 From: Christian Hergert Date: Fri, 21 Feb 2020 14:16:38 -0800 Subject: [PATCH] clutter: remove string copies from paint node names When creating paint nodes, various names are attached to the node to simplify debugging. These strings are g_strdup()d and g_free()d quite frequently. For example, a minute of GNOME Shell use resulted in nearly .5 MiB of string duplications and frees. All uses within the code-base have access to a static string, so we can just use that instead and take the hit of g_intern_string() for any extensions that would be using the existing clutter_paint_node_set_name() API for backwards compatibility. https://gitlab.gnome.org/GNOME/mutter/merge_requests/1081 --- clutter/clutter/clutter-actor.c | 20 ++++++++--------- clutter/clutter/clutter-canvas.c | 2 +- clutter/clutter/clutter-image.c | 2 +- clutter/clutter/clutter-paint-node-private.h | 2 +- clutter/clutter/clutter-paint-node.c | 23 +++++++++++++++----- clutter/clutter/clutter-paint-node.h | 3 +++ src/compositor/meta-shaped-texture.c | 6 ++--- 7 files changed, 37 insertions(+), 21 deletions(-) diff --git a/clutter/clutter/clutter-actor.c b/clutter/clutter/clutter-actor.c index 728fd5cca..4979c9dcd 100644 --- a/clutter/clutter/clutter-actor.c +++ b/clutter/clutter/clutter-actor.c @@ -3474,8 +3474,8 @@ _clutter_actor_draw_paint_volume_full (ClutterActor *self, cogl_pipeline_set_color (outline, &cogl_color); pipeline_node = clutter_pipeline_node_new (outline); - clutter_paint_node_set_name (pipeline_node, - "ClutterActor (paint volume outline)"); + clutter_paint_node_set_static_name (pipeline_node, + "ClutterActor (paint volume outline)"); clutter_paint_node_add_primitive (pipeline_node, prim); clutter_paint_node_add_child (node, pipeline_node); cogl_object_unref (prim); @@ -3489,8 +3489,8 @@ _clutter_actor_draw_paint_volume_full (ClutterActor *self, pango_layout_set_text (layout, label, -1); text_node = clutter_text_node_new (layout, color); - clutter_paint_node_set_name (text_node, - "ClutterActor (paint volume label)"); + clutter_paint_node_set_static_name (text_node, + "ClutterActor (paint volume label)"); clutter_paint_node_add_rectangle (text_node, &(ClutterActorBox) { .x1 = pv->vertices[0].x, @@ -3586,8 +3586,8 @@ _clutter_actor_paint_cull_result (ClutterActor *self, pango_layout_set_text (layout, label, -1); text_node = clutter_text_node_new (layout, &color); - clutter_paint_node_set_name (text_node, - "ClutterActor (paint volume text)"); + clutter_paint_node_set_static_name (text_node, + "ClutterActor (paint volume text)"); clutter_paint_node_add_rectangle (text_node, &(ClutterActorBox) { .x1 = 0.f, @@ -3868,7 +3868,7 @@ clutter_actor_paint_node (ClutterActor *actor, clear_flags = COGL_BUFFER_BIT_DEPTH; node = clutter_root_node_new (fb, &bg_color, clear_flags); - clutter_paint_node_set_name (node, "stageClear"); + clutter_paint_node_set_static_name (node, "stageClear"); clutter_paint_node_add_rectangle (node, &box); clutter_paint_node_add_child (root, node); clutter_paint_node_unref (node); @@ -3883,7 +3883,7 @@ clutter_actor_paint_node (ClutterActor *actor, / 255; node = clutter_color_node_new (&bg_color); - clutter_paint_node_set_name (node, "backgroundColor"); + clutter_paint_node_set_static_name (node, "backgroundColor"); clutter_paint_node_add_rectangle (node, &box); clutter_paint_node_add_child (root, node); clutter_paint_node_unref (node); @@ -4167,7 +4167,7 @@ clutter_actor_continue_paint (ClutterActor *self, */ framebuffer = clutter_paint_context_get_base_framebuffer (paint_context); dummy = _clutter_dummy_node_new (self, framebuffer); - clutter_paint_node_set_name (dummy, "Root"); + clutter_paint_node_set_static_name (dummy, "Root"); /* XXX - for 1.12, we use the return value of paint_node() to * decide whether we should emit the ::paint signal. @@ -21171,7 +21171,7 @@ clutter_actor_create_texture_paint_node (ClutterActor *self, color.alpha = clutter_actor_get_paint_opacity_internal (self); node = clutter_texture_node_new (texture, &color, priv->min_filter, priv->mag_filter); - clutter_paint_node_set_name (node, "Texture"); + clutter_paint_node_set_static_name (node, "Texture"); if (priv->content_repeat == CLUTTER_REPEAT_NONE) clutter_paint_node_add_rectangle (node, &box); diff --git a/clutter/clutter/clutter-canvas.c b/clutter/clutter/clutter-canvas.c index 2502586a3..43af0b98f 100644 --- a/clutter/clutter/clutter-canvas.c +++ b/clutter/clutter/clutter-canvas.c @@ -352,7 +352,7 @@ clutter_canvas_paint_content (ClutterContent *content, return; node = clutter_actor_create_texture_paint_node (actor, priv->texture); - clutter_paint_node_set_name (node, "Canvas Content"); + clutter_paint_node_set_static_name (node, "Canvas Content"); clutter_paint_node_add_child (root, node); clutter_paint_node_unref (node); diff --git a/clutter/clutter/clutter-image.c b/clutter/clutter/clutter-image.c index c5a1c51ac..eb6c2f50a 100644 --- a/clutter/clutter/clutter-image.c +++ b/clutter/clutter/clutter-image.c @@ -130,7 +130,7 @@ clutter_image_paint_content (ClutterContent *content, return; node = clutter_actor_create_texture_paint_node (actor, priv->texture); - clutter_paint_node_set_name (node, "Image Content"); + clutter_paint_node_set_static_name (node, "Image Content"); clutter_paint_node_add_child (root, node); clutter_paint_node_unref (node); } diff --git a/clutter/clutter/clutter-paint-node-private.h b/clutter/clutter/clutter-paint-node-private.h index 248ed1549..f912a6a10 100644 --- a/clutter/clutter/clutter-paint-node-private.h +++ b/clutter/clutter/clutter-paint-node-private.h @@ -51,7 +51,7 @@ struct _ClutterPaintNode GArray *operations; - gchar *name; + const gchar *name; guint n_children; diff --git a/clutter/clutter/clutter-paint-node.c b/clutter/clutter/clutter-paint-node.c index 877ec1784..708ebf195 100644 --- a/clutter/clutter/clutter-paint-node.c +++ b/clutter/clutter/clutter-paint-node.c @@ -171,8 +171,6 @@ clutter_paint_node_real_finalize (ClutterPaintNode *node) { ClutterPaintNode *iter; - g_free (node->name); - if (node->operations != NULL) { guint i; @@ -297,7 +295,8 @@ clutter_paint_node_get_type (void) * * The @name will be used for debugging purposes. * - * The @node will copy the passed string. + * The @node will intern @name using g_intern_string(). If you have access to a + * static string, use clutter_paint_node_set_static_name() instead. * * Since: 1.10 */ @@ -307,8 +306,22 @@ clutter_paint_node_set_name (ClutterPaintNode *node, { g_return_if_fail (CLUTTER_IS_PAINT_NODE (node)); - g_free (node->name); - node->name = g_strdup (name); + node->name = g_intern_string (name); +} + +/** + * clutter_paint_node_set_static_name: (skip) + * + * Like clutter_paint_node_set_name() but uses a static or interned string + * containing the name. + */ +void +clutter_paint_node_set_static_name (ClutterPaintNode *node, + const char *name) +{ + g_return_if_fail (CLUTTER_IS_PAINT_NODE (node)); + + node->name = name; } /** diff --git a/clutter/clutter/clutter-paint-node.h b/clutter/clutter/clutter-paint-node.h index 49722edd9..9623d8bca 100644 --- a/clutter/clutter/clutter-paint-node.h +++ b/clutter/clutter/clutter-paint-node.h @@ -56,6 +56,9 @@ void clutter_paint_node_paint (Clutter CLUTTER_EXPORT void clutter_paint_node_set_name (ClutterPaintNode *node, const char *name); +CLUTTER_EXPORT +void clutter_paint_node_set_static_name (ClutterPaintNode *node, + const char *name); CLUTTER_EXPORT CoglFramebuffer * clutter_paint_node_get_framebuffer (ClutterPaintNode *node); diff --git a/src/compositor/meta-shaped-texture.c b/src/compositor/meta-shaped-texture.c index 85a5143f9..6f30325cd 100644 --- a/src/compositor/meta-shaped-texture.c +++ b/src/compositor/meta-shaped-texture.c @@ -443,7 +443,7 @@ paint_clipped_rectangle_node (MetaShapedTexture *stex, coords[7] = coords[3]; node = clutter_pipeline_node_new (pipeline); - clutter_paint_node_set_name (node, "MetaShapedTexture (clipped)"); + clutter_paint_node_set_static_name (node, "MetaShapedTexture (clipped)"); clutter_paint_node_add_child (root_node, node); clutter_paint_node_add_multitexture_rectangle (node, @@ -664,7 +664,7 @@ do_paint_content (MetaShapedTexture *stex, g_autoptr (ClutterPaintNode) node = NULL; node = clutter_pipeline_node_new (blended_pipeline); - clutter_paint_node_set_name (node, "MetaShapedTexture (unclipped)"); + clutter_paint_node_set_static_name (node, "MetaShapedTexture (unclipped)"); clutter_paint_node_add_child (root_node, node); /* 3) blended_tex_region is NULL. Do a full paint. */ @@ -1241,7 +1241,7 @@ get_image_via_offscreen (MetaShapedTexture *stex, clear_color = (ClutterColor) { 0, 0, 0, 0 }; root_node = clutter_root_node_new (fb, &clear_color, COGL_BUFFER_BIT_COLOR); - clutter_paint_node_set_name (root_node, "MetaShapedTexture.offscreen"); + clutter_paint_node_set_static_name (root_node, "MetaShapedTexture.offscreen"); paint_context = clutter_paint_context_new_for_framebuffer (fb);