[mutter/wip/chergert/paint-node-cleanup: 2/2] clutter: remove string copies from paint node names



commit 1ac23d298a53f6c764e9d8abf0304eed318deddc
Author: Christian Hergert <chergert redhat com>
Date:   Fri Feb 21 14:16:38 2020 -0800

    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);
 


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