GNOME.org
 

[gnome-system-tools] Improve performance when filling tables in users-admin

commit ca44ce48bcbef7e87ac172037251dd2167f1d909
Author: Milan Bouchet-Valat <nalimilan club fr>
Date:   Thu Dec 16 11:26:33 2010 +0100

    Improve performance when filling tables in users-admin
    
    It's absurd to get the model from the tree view again and again
    when inserting users or groups: doing this for about 150 objects
    has a real performance hit on startup. So store models as a static
    variable in each file (OOP of the poor). Making it a GtkListStore
    allows saving a few casts to GtkTreeModel in the insert functions
    that are intensively used.
    
    Also stop adding users that aren't shown to the table: they are most likely
    useless. If settings change and we are asked to show system users, we can
    reload the whole UI anyway. Same for privileges table, where a
    GtkTreeModelFilter was absolutely useless - remove them.
    
    Finally, use gtk_list_store_insert_with_values() to avoid running the
    filter/sort functions on empty rows.

 src/users/groups-table.c     |   69 ++++++++-----------------
 src/users/privileges-table.c |  115 ++++++++++-------------------------------
 src/users/users-table.c      |   65 +++++++-----------------
 src/users/users-tool.c       |   16 ++----
 4 files changed, 75 insertions(+), 190 deletions(-)
---
diff --git a/src/users/groups-table.c b/src/users/groups-table.c
index d48979b..3775c54 100644
--- a/src/users/groups-table.c
+++ b/src/users/groups-table.c
@@ -31,6 +31,8 @@
 
 extern GstTool *tool;
 
+static GtkListStore *groups_model = NULL;
+
 static void
 add_group_columns (GtkTreeView *treeview)
 {
@@ -51,38 +53,27 @@ add_group_columns (GtkTreeView *treeview)
 	gtk_tree_view_insert_column (treeview, column, -1);
 }
 
-static GtkTreeModel*
-create_groups_model (void)
-{
-	GtkTreeModel *sort_model;
-	GtkListStore *store;
-
-	store = gtk_list_store_new (COL_GROUP_LAST,
-	                            G_TYPE_STRING,
-	                            G_TYPE_INT,
-				    G_TYPE_OBJECT);
-
-	/* Sort model */
-	sort_model = gtk_tree_model_sort_new_with_model (GTK_TREE_MODEL (store));
-	gtk_tree_sortable_set_sort_column_id (GTK_TREE_SORTABLE (sort_model),
-	                                      COL_GROUP_NAME, GTK_SORT_ASCENDING);
-
-	return sort_model;
-}
-
 void
 create_groups_table (void)
 {
 	GtkWidget *groups_table;
-	GtkTreeModel *model;
+	GtkTreeModel *sort_model;
 	GtkTreeSelection *selection;
 	GtkWidget *popup;
 	
 	groups_table = gst_dialog_get_widget (tool->main_dialog, "groups_table");
 
-	model = create_groups_model ();
-	gtk_tree_view_set_model (GTK_TREE_VIEW (groups_table), model);
-	g_object_unref (model);
+	groups_model = gtk_list_store_new (COL_GROUP_LAST,
+	                                   G_TYPE_STRING,
+	                                   G_TYPE_INT,
+				           G_TYPE_OBJECT);
+
+	/* Sort model */
+	sort_model = gtk_tree_model_sort_new_with_model (GTK_TREE_MODEL (groups_model));
+	gtk_tree_sortable_set_sort_column_id (GTK_TREE_SORTABLE (sort_model),
+	                                      COL_GROUP_NAME, GTK_SORT_ASCENDING);
+	gtk_tree_view_set_model (GTK_TREE_VIEW (groups_table), GTK_TREE_MODEL (groups_model));
+	g_object_unref (groups_model);
 
 	add_group_columns (GTK_TREE_VIEW (groups_table));
 
@@ -107,23 +98,13 @@ create_groups_table (void)
 GtkTreeModel *
 groups_table_get_model ()
 {
-	GtkWidget *groups_table;
-	GtkTreeModel *sort_model;
-
-	groups_table = gst_dialog_get_widget (tool->main_dialog, "groups_table");
-	sort_model = gtk_tree_view_get_model (GTK_TREE_VIEW (groups_table));
-
-	return gtk_tree_model_sort_get_model (GTK_TREE_MODEL_SORT (sort_model));
+	return GTK_TREE_MODEL (groups_model);
 }
 
 void
 groups_table_set_group (OobsGroup *group, GtkTreeIter *iter)
 {
-	GtkTreeModel *model;
-
-	model = groups_table_get_model ();
-
-	gtk_list_store_set (GTK_LIST_STORE (model), iter,
+	gtk_list_store_set (groups_model, iter,
 			    COL_GROUP_NAME, oobs_group_get_name (group),
 			    COL_GROUP_ID, oobs_group_get_gid (group),
 			    COL_GROUP_OBJECT, group,
@@ -133,23 +114,17 @@ groups_table_set_group (OobsGroup *group, GtkTreeIter *iter)
 void
 groups_table_add_group (OobsGroup *group)
 {
-	GtkTreeModel *model;
-	GtkTreeIter iter;
-
-	model = groups_table_get_model ();
-
-	gtk_list_store_append (GTK_LIST_STORE (model), &iter);
-	groups_table_set_group (group, &iter);
+	gtk_list_store_insert_with_values (groups_model, NULL, G_MAXINT,
+	                                   COL_GROUP_NAME, oobs_group_get_name (group),
+	                                   COL_GROUP_ID, oobs_group_get_gid (group),
+	                                   COL_GROUP_OBJECT, group,
+	                                   -1);
 }
 
 void
 groups_table_clear (void)
 {
-	GtkTreeModel *model;
-
-	model = groups_table_get_model ();
-
-	gtk_list_store_clear (GTK_LIST_STORE (model));
+	gtk_list_store_clear (groups_model);
 }
 
 /*
diff --git a/src/users/privileges-table.c b/src/users/privileges-table.c
index b16405f..056c13d 100644
--- a/src/users/privileges-table.c
+++ b/src/users/privileges-table.c
@@ -35,6 +35,8 @@
 
 extern GstTool *tool;
 
+static GtkListStore *privileges_model = NULL;
+
 enum {
 	COL_MEMBER,
 	COL_DESCRIPTION,
@@ -100,20 +102,13 @@ privilege_search (const gchar *group)
 static void
 on_user_privilege_toggled (GtkCellRendererToggle *cell, gchar *path_str, gpointer data)
 {
-	GtkTreeModel *model, *child_model;
 	GtkTreePath  *path  = gtk_tree_path_new_from_string (path_str);
-	GtkTreeIter   iter, child_iter;
+	GtkTreeIter   iter;
 	OobsGroup    *group;
 	gboolean      value;
 
-	model = (GtkTreeModel*) data;
-
-	if (gtk_tree_model_get_iter (model, &iter, path)) {
-		child_model = gtk_tree_model_filter_get_model (GTK_TREE_MODEL_FILTER (model));
-		gtk_tree_model_filter_convert_iter_to_child_iter (GTK_TREE_MODEL_FILTER (model),
-								  &child_iter, &iter);
-
-		gtk_tree_model_get (child_model, &child_iter,
+	if (gtk_tree_model_get_iter (GTK_TREE_MODEL (privileges_model), &iter, path)) {
+		gtk_tree_model_get (GTK_TREE_MODEL (privileges_model), &iter,
 		                    COL_MEMBER, &value,
 		                    COL_GROUP, &group, -1);
 
@@ -121,7 +116,7 @@ on_user_privilege_toggled (GtkCellRendererToggle *cell, gchar *path_str, gpointe
 		 * possibly showing a warning/error dialog */
 		if (!value || strcmp (oobs_group_get_name (group), ADMIN_GROUP) != 0
 		           || user_settings_check_revoke_admin_rights ())
-			gtk_list_store_set (GTK_LIST_STORE (child_model), &child_iter, COL_MEMBER, !value, -1);
+			gtk_list_store_set (privileges_model, &iter, COL_MEMBER, !value, -1);
 
 		g_object_unref (group);
 	}
@@ -129,43 +124,20 @@ on_user_privilege_toggled (GtkCellRendererToggle *cell, gchar *path_str, gpointe
 	gtk_tree_path_free (path);
 }
 
-static gboolean
-privileges_table_visible_func (GtkTreeModel *model,
-			       GtkTreeIter  *iter,
-			       gpointer      data)
-{
-	gboolean visible;
-	gchar *str;
-
-	gtk_tree_model_get (model, iter,
-			    COL_DESCRIPTION, &str,
-			    -1);
-	visible = (str != NULL);
-	g_free (str);
-
-	return visible;
-}
-
 void
 create_user_privileges_table (void)
 {
 	GtkWidget *list;
-	GtkTreeModel *model, *filter_model;
 	GtkCellRenderer *renderer;
 	GtkTreeViewColumn *column;
 
 	list = gst_dialog_get_widget (tool->main_dialog, "user_privileges");
 
-	model = GTK_TREE_MODEL (gtk_list_store_new (3, G_TYPE_BOOLEAN, G_TYPE_STRING, G_TYPE_OBJECT));
-	filter_model = gtk_tree_model_filter_new (model, NULL);
-	gtk_tree_view_set_model (GTK_TREE_VIEW (list), filter_model);
-	g_object_unref (filter_model);
-	g_object_unref (model);
+	privileges_model = gtk_list_store_new (3, G_TYPE_BOOLEAN, G_TYPE_STRING, G_TYPE_OBJECT);
+	gtk_tree_view_set_model (GTK_TREE_VIEW (list), GTK_TREE_MODEL (privileges_model));
+	g_object_unref (privileges_model);
 
-	gtk_tree_sortable_set_sort_column_id (GTK_TREE_SORTABLE (model), COL_DESCRIPTION, GTK_SORT_ASCENDING);
-	gtk_tree_model_filter_set_visible_func (GTK_TREE_MODEL_FILTER (filter_model),
-						privileges_table_visible_func,
-						NULL, NULL);
+	gtk_tree_sortable_set_sort_column_id (GTK_TREE_SORTABLE (privileges_model), COL_DESCRIPTION, GTK_SORT_ASCENDING);
 
 	column = gtk_tree_view_column_new ();
 
@@ -176,7 +148,7 @@ create_user_privileges_table (void)
 					     "active", COL_MEMBER,
 					     NULL);
 	g_signal_connect (G_OBJECT (renderer), "toggled",
-			  G_CALLBACK (on_user_privilege_toggled), filter_model);
+			  G_CALLBACK (on_user_privilege_toggled), NULL);
 
 	renderer = gtk_cell_renderer_text_new ();
 	gtk_tree_view_column_pack_end (column, renderer, TRUE);
@@ -193,54 +165,40 @@ void
 privileges_table_add_group (OobsGroup *group)
 {
 	const PrivilegeDescription *p;
-	GtkWidget *table;
-	GtkTreeModel *model;
-	GtkListStore *store;
-	GtkTreeIter iter;
 
 	p = privilege_search (oobs_group_get_name (group));
 	if (p == NULL) /* Ignore groups that don't match a privilege */
 		return;
 
-	table = gst_dialog_get_widget (tool->main_dialog, "user_privileges");
-	model = gtk_tree_view_get_model (GTK_TREE_VIEW (table));
-	store = GTK_LIST_STORE (gtk_tree_model_filter_get_model (GTK_TREE_MODEL_FILTER (model)));
-
-	gtk_list_store_append (store, &iter);
-	gtk_list_store_set (store, &iter,
-			    COL_MEMBER, FALSE,
-			    COL_DESCRIPTION, (p) ? _(p->privilege) : NULL,
-			    COL_GROUP, group,
-			    -1);
+	gtk_list_store_insert_with_values (privileges_model, NULL, G_MAXINT,
+	                                   COL_MEMBER, FALSE,
+	                                   COL_DESCRIPTION, (p) ? _(p->privilege) : NULL,
+	                                   COL_GROUP, group,
+	                                   -1);
 }
 
 void
 privileges_table_set_from_user (OobsUser *user)
 {
-	GtkWidget *table;
-	GtkTreeModel *model, *child_model;
 	GtkTreeIter iter;
 	gboolean valid;
 	OobsGroup *group;
 	GList *users;
 
-	table = gst_dialog_get_widget (tool->main_dialog, "user_privileges");
-	model = gtk_tree_view_get_model (GTK_TREE_VIEW (table));
-	child_model = gtk_tree_model_filter_get_model (GTK_TREE_MODEL_FILTER (model));
-	valid = gtk_tree_model_get_iter_first (child_model, &iter);
+	valid = gtk_tree_model_get_iter_first (GTK_TREE_MODEL (privileges_model), &iter);
 
 	while (valid) {
-		gtk_tree_model_get (child_model, &iter,
+		gtk_tree_model_get (GTK_TREE_MODEL (privileges_model), &iter,
 				    COL_GROUP, &group,
 				    -1);
 
 		users = oobs_group_get_users (group);
-		gtk_list_store_set (GTK_LIST_STORE (child_model), &iter,
+		gtk_list_store_set (privileges_model, &iter,
 				    COL_MEMBER, (g_list_find (users, user) != NULL),
 				    -1);
 		g_list_free (users);
 		g_object_unref (group);
-		valid = gtk_tree_model_iter_next (child_model, &iter);
+		valid = gtk_tree_model_iter_next (GTK_TREE_MODEL (privileges_model), &iter);
 	}
 }
 
@@ -270,47 +228,37 @@ find_group_in_profile (OobsGroup      *group,
 void
 privileges_table_set_from_profile (GstUserProfile *profile)
 {
-	GtkWidget *table;
-	GtkTreeModel *model, *child_model;
 	GtkTreeIter iter;
 	gboolean valid;
 	OobsGroup *group;
 
-	table = gst_dialog_get_widget (tool->main_dialog, "user_privileges");
-	model = gtk_tree_view_get_model (GTK_TREE_VIEW (table));
-	child_model = gtk_tree_model_filter_get_model (GTK_TREE_MODEL_FILTER (model));
-	valid = gtk_tree_model_get_iter_first (child_model, &iter);
+	valid = gtk_tree_model_get_iter_first (GTK_TREE_MODEL (privileges_model), &iter);
 
 	while (valid) {
-		gtk_tree_model_get (child_model, &iter,
+		gtk_tree_model_get (GTK_TREE_MODEL (privileges_model), &iter,
 				    COL_GROUP, &group,
 				    -1);
 
-		gtk_list_store_set (GTK_LIST_STORE (child_model), &iter,
+		gtk_list_store_set (privileges_model, &iter,
 				    COL_MEMBER, find_group_in_profile (group, profile),
 				    -1);
 
 		g_object_unref (group);
-		valid = gtk_tree_model_iter_next (child_model, &iter);
+		valid = gtk_tree_model_iter_next (GTK_TREE_MODEL (privileges_model), &iter);
 	}
 }
 
 void
 privileges_table_save (OobsUser *user)
 {
-	GtkWidget *table;
-	GtkTreeModel *model, *child_model;
 	GtkTreeIter iter;
 	OobsGroup *group;
 	gboolean valid, member;
 
-	table = gst_dialog_get_widget (tool->main_dialog, "user_privileges");
-	model = gtk_tree_view_get_model (GTK_TREE_VIEW (table));
-	child_model = gtk_tree_model_filter_get_model (GTK_TREE_MODEL_FILTER (model));
-	valid = gtk_tree_model_get_iter_first (child_model, &iter);
+	valid = gtk_tree_model_get_iter_first (GTK_TREE_MODEL (privileges_model), &iter);
 
 	while (valid) {
-		gtk_tree_model_get (child_model, &iter,
+		gtk_tree_model_get (GTK_TREE_MODEL (privileges_model), &iter,
 				    COL_GROUP, &group,
 				    COL_MEMBER, &member,
 				    -1);
@@ -319,19 +267,12 @@ privileges_table_save (OobsUser *user)
 		else
 			oobs_group_remove_user (group, user);
 
-		valid = gtk_tree_model_iter_next (child_model, &iter);
+		valid = gtk_tree_model_iter_next (GTK_TREE_MODEL (privileges_model), &iter);
 	}
 }
 
 void
 privileges_table_clear (void)
 {
-	GtkWidget *table;
-	GtkTreeModel *model, *child_model;
-
-	table = gst_dialog_get_widget (tool->main_dialog, "user_privileges");
-	model = gtk_tree_view_get_model (GTK_TREE_VIEW (table));
-	child_model = gtk_tree_model_filter_get_model (GTK_TREE_MODEL_FILTER (model));
-
-	gtk_list_store_clear (GTK_LIST_STORE (child_model));
+	gtk_list_store_clear (privileges_model);
 }
diff --git a/src/users/users-table.c b/src/users/users-table.c
index 0d73d0b..f23d965 100644
--- a/src/users/users-table.c
+++ b/src/users/users-table.c
@@ -32,6 +32,8 @@
 
 extern GstTool *tool;
 
+static GtkListStore *users_model = NULL;
+
 static void
 add_user_columns (GtkTreeView *treeview)
 {
@@ -90,14 +92,18 @@ users_model_filter (GtkTreeModel *model, GtkTreeIter *iter, gpointer data)
 	return show;
 }
 
-static GtkTreeModel*
-create_users_model (GstUsersTool *tool)
+void
+create_users_table (GstUsersTool *tool)
 {
-	GtkListStore *store;
+	GtkWidget *users_table;
+	GtkTreeSelection *selection;
 	GtkTreeModel *filter_model;
 	GtkTreeModel *sort_model;
-	
-	store = gtk_list_store_new (COL_USER_LAST,
+	GtkWidget *popup;
+
+	users_table = gst_dialog_get_widget (GST_TOOL (tool)->main_dialog, "users_table");
+
+	users_model = gtk_list_store_new (COL_USER_LAST,
 				    GDK_TYPE_PIXBUF,
 	                            G_TYPE_STRING,
 	                            G_TYPE_STRING,
@@ -109,7 +115,7 @@ create_users_model (GstUsersTool *tool)
 				    OOBS_TYPE_LIST_ITER);
 
 	/* Filter model */
-	filter_model = gtk_tree_model_filter_new (GTK_TREE_MODEL (store), NULL);
+	filter_model = gtk_tree_model_filter_new (GTK_TREE_MODEL (users_model), NULL);
 	gtk_tree_model_filter_set_visible_func (GTK_TREE_MODEL_FILTER (filter_model),
 						users_model_filter, tool, NULL);
 
@@ -118,23 +124,9 @@ create_users_model (GstUsersTool *tool)
 	gtk_tree_sortable_set_sort_column_id (GTK_TREE_SORTABLE (sort_model),
 	                                      COL_USER_LOGIN, GTK_SORT_ASCENDING);
 
-	return sort_model;
-}
+	gtk_tree_view_set_model (GTK_TREE_VIEW (users_table), sort_model);
+	g_object_unref (users_model);
 
-void
-create_users_table (GstUsersTool *tool)
-{
-	GtkWidget *users_table;
-	GtkTreeSelection *selection;
-	GtkTreeModel *model;
-	GtkWidget *popup;
-	
-	users_table = gst_dialog_get_widget (GST_TOOL (tool)->main_dialog, "users_table");
-
-	model = create_users_model (tool);
-	gtk_tree_view_set_model (GTK_TREE_VIEW (users_table), model);
-	g_object_unref (model);
-	
 	add_user_columns (GTK_TREE_VIEW (users_table));
 	
 	selection = gtk_tree_view_get_selection (GTK_TREE_VIEW (users_table));
@@ -160,29 +152,17 @@ create_users_table (GstUsersTool *tool)
 GtkTreeModel *
 users_table_get_model ()
 {
-	GtkWidget *users_table;
-	GtkTreeModel *sort_model;
-	GtkTreeModel *filter_model;
-
-	users_table = gst_dialog_get_widget (GST_TOOL (tool)->main_dialog, "users_table");
-
-	sort_model = gtk_tree_view_get_model (GTK_TREE_VIEW (users_table));
-	filter_model = gtk_tree_model_sort_get_model (GTK_TREE_MODEL_SORT (sort_model));
-
-	return gtk_tree_model_filter_get_model (GTK_TREE_MODEL_FILTER (filter_model));
+	return GTK_TREE_MODEL (users_model);
 }
 
 void
 users_table_set_user (OobsUser *user, GtkTreeIter *iter)
 {
-	GtkTreeModel *model;
 	GdkPixbuf *face;
 	const char *name;
 	const char *login;
 	char *label;
 
-	model = users_table_get_model ();
-
 	face = user_settings_get_user_face (user, 48);
 	name = oobs_user_get_full_name_fallback (user);
 	login = oobs_user_get_login_name (user);
@@ -190,7 +170,7 @@ users_table_set_user (OobsUser *user, GtkTreeIter *iter)
 	label = g_markup_printf_escaped ("<big><b>%s</b>\n<span color=\'dark grey\'><i>%s</i></span></big>",
 	                                 name, login);
 
-	gtk_list_store_set (GTK_LIST_STORE (model), iter,
+	gtk_list_store_set (users_model, iter,
 			    COL_USER_FACE, face,
 			    COL_USER_NAME, name,
 			    COL_USER_LOGIN, login,
@@ -213,25 +193,18 @@ users_table_set_user (OobsUser *user, GtkTreeIter *iter)
 GtkTreePath *
 users_table_add_user (OobsUser *user)
 {
-	GtkTreeModel *model;
 	GtkTreeIter iter;
 
-	model = users_table_get_model ();
-
-	gtk_list_store_append (GTK_LIST_STORE (model), &iter);
+	gtk_list_store_append (users_model, &iter);
 	users_table_set_user (user, &iter);
 
-	return gtk_tree_model_get_path (model, &iter);
+	return gtk_tree_model_get_path (GTK_TREE_MODEL (users_model), &iter);
 }
 
 void
 users_table_clear (void)
 {
-        GtkTreeModel *model;
-
-	model = users_table_get_model ();
-
-        gtk_list_store_clear (GTK_LIST_STORE (model));
+        gtk_list_store_clear (users_model);
 }
 
 /*
diff --git a/src/users/users-tool.c b/src/users/users-tool.c
index 14a01ba..c3786b5 100644
--- a/src/users/users-tool.c
+++ b/src/users/users-tool.c
@@ -59,16 +59,12 @@ on_option_changed (GSettings  *settings,
                    gpointer    user_data)
 {
 	GstTool *tool = GST_TOOL (user_data);
-	GtkWidget *widget;
-	GtkTreeModel *sort_model, *filter_model;
 
 	GST_USERS_TOOL (tool)->showall = g_settings_get_boolean (settings, "showall");
 	GST_USERS_TOOL (tool)->showroot = g_settings_get_boolean (settings, "showroot");
 
-	widget = gst_dialog_get_widget (tool->main_dialog, "users_table");
-	sort_model = gtk_tree_view_get_model (GTK_TREE_VIEW (widget));
-	filter_model = gtk_tree_model_sort_get_model (GTK_TREE_MODEL_SORT (sort_model));
-	gtk_tree_model_filter_refilter (GTK_TREE_MODEL_FILTER (filter_model));
+	/* We need to reload the users table since unshown users aren't added */
+	gst_tool_update_gui (tool);
 }
 
 static void
@@ -137,7 +133,7 @@ update_users (GstUsersTool *tool)
 {
 	OobsList *list;
 	OobsListIter iter;
-	GObject *user;
+	OobsUser *user;
 	OobsUser *self;
 	GtkTreePath *path;
 	gboolean valid;
@@ -149,11 +145,11 @@ update_users (GstUsersTool *tool)
 	valid = oobs_list_get_iter_first (list, &iter);
 
 	while (valid) {
-		user = oobs_list_get (list, &iter);
-		path = users_table_add_user (OOBS_USER (user));
+		user = OOBS_USER (oobs_list_get (list, &iter));
+		path = users_table_add_user (user);
 		gst_tool_add_configuration_object (GST_TOOL (tool), OOBS_OBJECT (user), FALSE);
 
-		if (self == (OobsUser *) user)
+		if (self == user)
 			users_table_select_path (path);
 
 		g_object_unref (user);
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]