Bug 535608 – do not string-copy accel paths in the menu code

Don't store the accel path as a string in gtkmenu/gtkmenuitem.
The accel path will be interned anyway, so keeping a string copy around
is just a waste of memory.
Improve the documentation to mention this.

svn path=/trunk/; revision=20331

7 files changed, 52 insertions, 36 deletions

diff --git a/ChangeLog b/ChangeLog
index fd381d3e4b..0689322e26 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2008-06-08  Christian Persch <chpe@gnome.org>
+
+	Bug 535608 – do not string-copy accel paths in the menu code
+
+        * gtk/gtkaccelgroup.c:
+        * gtk/gtkaccelmap.c:
+        * gtk/gtkaction.c:
+        * gtk/gtkmenu.c:
+        * gtk/gtkmenuitem.c:
+        * gtk/gtkwidget.c:  Don't store the accel path as a string in
+        gtkmenu/gtkmenuitem. The accel path will be interned anyway, so
+        keeping a string copy around is just a waste of memory.
+        Improve the documentation to mention this.
+
 2008-06-07  Tor Lillqvist  <tml@novell.com>
 
 	Bug 536990 - updateiconcache.c: 'close ()' is redundant
diff --git a/gtk/gtkaccelgroup.c b/gtk/gtkaccelgroup.c
index cc7df18d02..30b2aa6051 100644
--- a/gtk/gtkaccelgroup.c
+++ b/gtk/gtkaccelgroup.c
@@ -525,6 +525,10 @@ gtk_accel_group_connect (GtkAccelGroup	*accel_group,
  * for the path.
  *
  * The signature used for the @closure is that of #GtkAccelGroupActivate.
+ * 
+ * Note that @accel_path string will be stored in a #GQuark. Therefore, if you
+ * pass a static string, you can save some memory by interning it first with 
+ * g_intern_static_string().
  */
 void
 gtk_accel_group_connect_by_path (GtkAccelGroup	*accel_group,
diff --git a/gtk/gtkaccelmap.c b/gtk/gtkaccelmap.c
index 4756035ae2..c193baaaf8 100644
--- a/gtk/gtkaccelmap.c
+++ b/gtk/gtkaccelmap.c
@@ -149,6 +149,10 @@ _gtk_accel_path_is_valid (const gchar *accel_path)
  * e.g. "File/Save As", "Image/View/Zoom" or "Edit/Select All".
  * So a full valid accelerator path may look like:
  * "&lt;Gimp-Toolbox&gt;/File/Dialogs/Tool Options...".
+ * 
+ * Note that @accel_path string will be stored in a #GQuark. Therefore, if you
+ * pass a static string, you can save some memory by interning it first with 
+ * g_intern_static_string().
  */
 void
 gtk_accel_map_add_entry (const gchar    *accel_path,
@@ -429,6 +433,10 @@ internal_change_entry (const gchar    *accel_path,
  * conflicts. A change will only occur if all conflicts could be resolved (which
  * might not be the case if conflicting accelerators are locked). Successful
  * changes are indicated by a %TRUE return value.
+ * 
+ * Note that @accel_path string will be stored in a #GQuark. Therefore, if you
+ * pass a static string, you can save some memory by interning it first with 
+ * g_intern_static_string().
  */
 gboolean
 gtk_accel_map_change_entry (const gchar    *accel_path,
diff --git a/gtk/gtkaction.c b/gtk/gtkaction.c
index 22fb10812c..f7d3a7ad34 100644
--- a/gtk/gtkaction.c
+++ b/gtk/gtkaction.c
@@ -119,9 +119,7 @@ G_DEFINE_TYPE_WITH_CODE (GtkAction, gtk_action, G_TYPE_OBJECT,
 						gtk_action_buildable_init))
 
 
-static GQuark      accel_path_id  = 0;
 static GQuark      quark_gtk_action_proxy  = 0;
-static const gchar accel_path_key[] = "GtkAction::accel_path";
 static const gchar gtk_action_proxy_key[] = "gtk-action";
 
 static void gtk_action_finalize     (GObject *object);
@@ -176,7 +174,6 @@ gtk_action_class_init (GtkActionClass *klass)
 {
   GObjectClass *gobject_class;
 
-  accel_path_id = g_quark_from_static_string (accel_path_key);
   quark_gtk_action_proxy = g_quark_from_static_string (gtk_action_proxy_key);
 
   gobject_class = G_OBJECT_CLASS (klass);
@@ -1662,6 +1659,10 @@ gtk_action_set_action_group (GtkAction	    *action,
  * with the action will have this accel path, so that their
  * accelerators are consistent.
  *
+ * Note that @accel_path string will be stored in a #GQuark. Therefore, if you
+ * pass a static string, you can save some memory by interning it first with 
+ * g_intern_static_string().
+ *
  * Since: 2.4
  */
 void
diff --git a/gtk/gtkmenu.c b/gtk/gtkmenu.c
index 027e312d93..75fad41251 100644
--- a/gtk/gtkmenu.c
+++ b/gtk/gtkmenu.c
@@ -152,7 +152,6 @@ static void     gtk_menu_get_child_property(GtkContainer     *container,
                                             GValue           *value,
                                             GParamSpec       *pspec);
 static void     gtk_menu_destroy           (GtkObject        *object);
-static void     gtk_menu_finalize          (GObject          *object);
 static void     gtk_menu_realize           (GtkWidget        *widget);
 static void     gtk_menu_unrealize         (GtkWidget        *widget);
 static void     gtk_menu_size_request      (GtkWidget        *widget,
@@ -438,7 +437,6 @@ gtk_menu_class_init (GtkMenuClass *class)
   GtkMenuShellClass *menu_shell_class = GTK_MENU_SHELL_CLASS (class);
   GtkBindingSet *binding_set;
   
-  gobject_class->finalize = gtk_menu_finalize;
   gobject_class->set_property = gtk_menu_set_property;
   gobject_class->get_property = gtk_menu_get_property;
 
@@ -964,16 +962,6 @@ gtk_menu_destroy (GtkObject *object)
 }
 
 static void
-gtk_menu_finalize (GObject *object)
-{
-  GtkMenu *menu = GTK_MENU (object);
-
-  g_free (menu->accel_path);
-  
-  G_OBJECT_CLASS (gtk_menu_parent_class)->finalize (object);
-}
-
-static void
 menu_change_screen (GtkMenu   *menu,
 		    GdkScreen *new_screen)
 {
@@ -1646,6 +1634,10 @@ gtk_menu_real_can_activate_accel (GtkWidget *widget,
  * Assigning accel paths to menu items then enables the user to change
  * their accelerators at runtime. More details about accelerator paths
  * and their default setups can be found at gtk_accel_map_add_entry().
+ * 
+ * Note that @accel_path string will be stored in a #GQuark. Therefore, if you
+ * pass a static string, you can save some memory by interning it first with 
+ * g_intern_static_string().
  */
 void
 gtk_menu_set_accel_path (GtkMenu     *menu,
@@ -1657,9 +1649,7 @@ gtk_menu_set_accel_path (GtkMenu     *menu,
   if (accel_path)
     g_return_if_fail (accel_path[0] == '<' && strchr (accel_path, '/')); /* simplistic check */
 
-  old_accel_path = menu->accel_path;
-  menu->accel_path = g_strdup (accel_path);
-  g_free (old_accel_path);
+  menu->accel_path = g_intern_string (accel_path);
   if (menu->accel_path)
     _gtk_menu_refresh_accel_paths (menu, FALSE);
 }
diff --git a/gtk/gtkmenuitem.c b/gtk/gtkmenuitem.c
index 050ff93253..73f1f17a98 100644
--- a/gtk/gtkmenuitem.c
+++ b/gtk/gtkmenuitem.c
@@ -64,7 +64,6 @@ static void gtk_menu_item_get_property   (GObject          *object,
 					  guint             prop_id,
 					  GValue           *value,
 					  GParamSpec       *pspec);
-static void gtk_menu_item_finalize       (GObject          *object);
 static void gtk_menu_item_destroy        (GtkObject        *object);
 static void gtk_menu_item_size_request   (GtkWidget        *widget,
 					  GtkRequisition   *requisition);
@@ -123,7 +122,6 @@ gtk_menu_item_class_init (GtkMenuItemClass *klass)
 
   gobject_class->set_property = gtk_menu_item_set_property;
   gobject_class->get_property = gtk_menu_item_get_property;
-  gobject_class->finalize = gtk_menu_item_finalize;
 
   object_class->destroy = gtk_menu_item_destroy;
 
@@ -362,16 +360,6 @@ gtk_menu_item_get_property (GObject    *object,
 }
 
 static void
-gtk_menu_item_finalize (GObject *object)
-{
-  GtkMenuItem *menu_item = GTK_MENU_ITEM (object);
-
-  g_free (menu_item->accel_path);
-
-  G_OBJECT_CLASS (gtk_menu_item_parent_class)->finalize (object);
-}
-
-static void
 gtk_menu_item_destroy (GtkObject *object)
 {
   GtkMenuItem *menu_item = GTK_MENU_ITEM (object);
@@ -1567,14 +1555,19 @@ _gtk_menu_item_refresh_accel_path (GtkMenuItem   *menu_item,
       path = menu_item->accel_path;
       if (!path && prefix)
 	{
-	  gchar *postfix = NULL;
+	  const gchar *postfix = NULL;
+          gchar *new_path;
 
 	  /* try to construct one from label text */
 	  gtk_container_foreach (GTK_CONTAINER (menu_item),
 				 gtk_menu_item_accel_name_foreach,
 				 &postfix);
-	  menu_item->accel_path = postfix ? g_strconcat (prefix, "/", postfix, NULL) : NULL;
-	  path = menu_item->accel_path;
+          if (postfix)
+            {
+              new_path = g_strconcat (prefix, "/", postfix, NULL);
+              path = menu_item->accel_path = g_intern_string (new_path);
+              g_free (new_path);
+            }
 	}
       if (path)
 	gtk_widget_set_accel_path (widget, path, accel_group);
@@ -1603,6 +1596,10 @@ _gtk_menu_item_refresh_accel_path (GtkMenuItem   *menu_item,
  *
  * Note that you do need to set an accelerator on the parent menu with
  * gtk_menu_set_accel_group() for this to work.
+ *
+ * Note that @accel_path string will be stored in a #GQuark. Therefore, if you
+ * pass a static string, you can save some memory by interning it first with 
+ * g_intern_static_string().
  */
 void
 gtk_menu_item_set_accel_path (GtkMenuItem *menu_item,
@@ -1618,9 +1615,7 @@ gtk_menu_item_set_accel_path (GtkMenuItem *menu_item,
   widget = GTK_WIDGET (menu_item);
 
   /* store new path */
-  old_accel_path = menu_item->accel_path;
-  menu_item->accel_path = g_strdup (accel_path);
-  g_free (old_accel_path);
+  menu_item->accel_path = g_intern_string (accel_path);
 
   /* forget accelerators associated with old path */
   gtk_widget_set_accel_path (widget, NULL, NULL);
diff --git a/gtk/gtkwidget.c b/gtk/gtkwidget.c
index 2d10cd854f..aa5402d7c2 100644
--- a/gtk/gtkwidget.c
+++ b/gtk/gtkwidget.c
@@ -4344,6 +4344,10 @@ destroy_accel_path (gpointer data)
  * Even when you you aren't using #GtkUIManager, if you only want to
  * set up accelerators on menu items gtk_menu_item_set_accel_path()
  * provides a somewhat more convenient interface.
+ * 
+ * Note that @accel_path string will be stored in a #GQuark. Therefore, if you
+ * pass a static string, you can save some memory by interning it first with 
+ * g_intern_static_string().
  **/
 void
 gtk_widget_set_accel_path (GtkWidget     *widget,