Enhance performance by updating the GtkTreeModel inside the TaskManager

The code to update the model has been moved inside the XtmTaskManager
class and this in order to enhance performance. In fact all the rows of
the model were udpdated everytime (150~ processes × 9 columns calls on
gtk_list_store_set per seconds) which represented a big CPU hog. Now
that the model is being updated within the same class that pulls the
processes information it is possible to run low check routines and
update only the rows that have updated information.

Also big surprise, the new tasks weren't added, well they did but not
the right data. The pointer's location was copied instead of the
pointer's content.

src/main.c

@@ -16,116 +16,12 @@
 #include <gtk/gtk.h>
 
 #include "process-window.h"
-#include "process-tree-view.h"
 #include "task-manager.h"
 
 static GtkWidget *window;
 static XtmTaskManager *task_manager;
 static gboolean timeout = 0;
 
-static void
-update_tree_iter (GtkTreeModel *model, GtkTreeIter *iter, Task *task)
-{
-	gchar vsz[64], rss[64], cpu[16];
-
-	// TODO add precision for values < 1 MB
-	g_snprintf (vsz, 64, _("%lu MB"), task->vsz / 1024 / 1024);
-	g_snprintf (rss, 64, _("%lu MB"), task->rss / 1024 / 1024);
-	// TODO make precision optional
-	g_snprintf (cpu, 16, _("%.2f%%"), task->cpu_user + task->cpu_system);
-
-	gtk_list_store_set (GTK_LIST_STORE (model), iter,
-		XTM_PTV_COLUMN_PPID, task->ppid,
-		XTM_PTV_COLUMN_STATE, task->state,
-		XTM_PTV_COLUMN_VSZ, task->vsz,
-		XTM_PTV_COLUMN_VSZ_STR, vsz,
-		XTM_PTV_COLUMN_RSS, task->rss,
-		XTM_PTV_COLUMN_RSS_STR, rss,
-		XTM_PTV_COLUMN_CPU, task->cpu_user + task->cpu_system,
-		XTM_PTV_COLUMN_CPU_STR, cpu,
-		XTM_PTV_COLUMN_PRIORITY, task->prio,
-		-1);
-}
-
-static void
-add_tree_iter (GtkTreeModel *model, Task *task)
-{
-	GtkTreeIter iter;
-	gtk_list_store_append (GTK_LIST_STORE (model), &iter);
-	gtk_list_store_set (GTK_LIST_STORE (model), &iter,
-		XTM_PTV_COLUMN_COMMAND, task->cmdline,
-		XTM_PTV_COLUMN_PID, task->pid,
-		XTM_PTV_COLUMN_STATE, task->state,
-		XTM_PTV_COLUMN_UID, task->uid_name,
-		-1);
-	update_tree_iter (model, &iter, task);
-}
-
-static void
-update_tree_model (const GArray *task_list)
-{
-	GtkTreeModel *model;
-	GtkTreeIter iter;
-	Task *task;
-	guint i;
-	gboolean valid;
-
-	model = xtm_process_window_get_model (XTM_PROCESS_WINDOW (window));
-
-	// TODO pick a timestamp for started/terminated tasks to keep them momentary (red/green color, italic, ...)
-	/* Remove terminated tasks */
-	valid = gtk_tree_model_get_iter_first (model, &iter);
-	while (valid)
-	{
-		guint pid;
-		gboolean found = FALSE;
-
-		gtk_tree_model_get (model, &iter, XTM_PTV_COLUMN_PID, &pid, -1);
-		for (i = 0; i < task_list->len; i++)
-		{
-			task = &g_array_index (task_list, Task, i);
-			if (pid != task->pid)
-				continue;
-			found = TRUE;
-			break;
-		}
-
-		if (found == FALSE)
-			gtk_list_store_remove (GTK_LIST_STORE (model), &iter);
-
-		valid = gtk_tree_model_iter_next (model, &iter);
-	}
-
-	/* Append started tasks and update existing ones */
-	for (i = 0; i < task_list->len; i++)
-	{
-		guint pid;
-		gboolean found = FALSE;
-
-		task = &g_array_index (task_list, Task, i);
-		valid = gtk_tree_model_get_iter_first (model, &iter);
-		while (valid)
-		{
-			gtk_tree_model_get (model, &iter, XTM_PTV_COLUMN_PID, &pid, -1);
-
-			if (pid == task->pid)
-			{
-				// TODO check if elements have to be updated, updating everything always is a CPU hog
-				update_tree_iter (model, &iter, task);
-				found = TRUE;
-				break;
-			}
-
-			valid = gtk_tree_model_iter_next (model, &iter);
-		}
-
-		if (found == FALSE)
-		{
-			add_tree_iter (model, task);
-		}
-	}
-}
-
 static gboolean
 init_timeout ()
 {
@@ -136,8 +32,7 @@ init_timeout ()
 	xtm_task_manager_get_system_info (task_manager, &num_processes, &cpu, &memory, &swap);
 	xtm_process_window_set_system_info (XTM_PROCESS_WINDOW (window), num_processes, cpu, memory, swap);
 
-	task_list = xtm_task_manager_get_task_list (task_manager);
+	xtm_task_manager_update_model (task_manager);
-	update_tree_model (task_list);
 
 	if (timeout == 0)
 		timeout = g_timeout_add (1000, init_timeout, NULL);
@@ -158,7 +53,7 @@ int main (int argc, char *argv[])
 	window = xtm_process_window_new ();
 	gtk_widget_show (window);
 
-	task_manager = xtm_task_manager_new ();
+	task_manager = xtm_task_manager_new (xtm_process_window_get_model (XTM_PROCESS_WINDOW (window)));
 	g_message ("Running as %s on %s", xtm_task_manager_get_username (task_manager), xtm_task_manager_get_hostname (task_manager));
 
 	init_timeout ();

src/task-manager.c

@@ -20,6 +20,7 @@
 #include <gtk/gtk.h>
 
 #include "task-manager.h"
+#include "process-tree-view.h" /* for the columns of the model */
 
 
 
@@ -32,6 +33,7 @@ struct _XtmTaskManager
 {
 	GObject			parent;
 	/*<private>*/
+	GtkTreeModel *		model;
 	GArray *		tasks;
 	guint			owner_uid;
 	gchar *			owner_uid_name;
@@ -52,6 +54,10 @@ static void	xtm_task_manager_finalize			(GObject *object);
 
 static void	get_owner_uid					(guint *owner_uid, gchar **owner_uid_name);
 static gchar *	get_hostname					();
+static void	model_add_task					(GtkTreeModel *model, Task *task);
+static void	model_update_tree_iter				(GtkTreeModel *model, GtkTreeIter *iter, Task *task);
+static void	model_update_task				(GtkTreeModel *model, Task *task);
+static void	model_remove_task				(GtkTreeModel *model, Task *task);
 
 
 
@@ -80,6 +86,14 @@ xtm_task_manager_finalize (GObject *object)
 	g_free (manager->hostname);
 }
 
+static void
+_xtm_task_manager_set_model (XtmTaskManager *manager, GtkTreeModel *model)
+{
+	g_return_if_fail (G_LIKELY (XTM_IS_TASK_MANAGER (manager)));
+	g_return_if_fail (G_LIKELY (GTK_IS_TREE_MODEL (model)));
+	manager->model = model;
+}
+
 static void
 get_owner_uid (guint *owner_uid, gchar **owner_uid_name)
 {
@@ -108,12 +122,84 @@ get_hostname ()
 	return g_strdup_printf ("%s", hostname);
 }
 
+static void
+model_add_task (GtkTreeModel *model, Task *task)
+{
+	GtkTreeIter iter;
+	gtk_list_store_append (GTK_LIST_STORE (model), &iter);
+	gtk_list_store_set (GTK_LIST_STORE (model), &iter,
+		XTM_PTV_COLUMN_COMMAND, task->cmdline,
+		XTM_PTV_COLUMN_PID, task->pid,
+		XTM_PTV_COLUMN_STATE, task->state,
+		XTM_PTV_COLUMN_UID, task->uid_name,
+		-1);
+	model_update_tree_iter (model, &iter, task);
+}
+
+static void
+model_update_tree_iter (GtkTreeModel *model, GtkTreeIter *iter, Task *task)
+{
+	gchar vsz[64], rss[64], cpu[16];
+
+	// TODO show values < 1 MB in KB or B
+	g_snprintf (vsz, 64, _("%lu MB"), task->vsz / 1024 / 1024);
+	g_snprintf (rss, 64, _("%lu MB"), task->rss / 1024 / 1024);
+	// TODO make precision optional
+	g_snprintf (cpu, 16, _("%.2f%%"), task->cpu_user + task->cpu_system);
+
+	gtk_list_store_set (GTK_LIST_STORE (model), iter,
+		XTM_PTV_COLUMN_PPID, task->ppid,
+		XTM_PTV_COLUMN_STATE, task->state,
+		XTM_PTV_COLUMN_VSZ, task->vsz,
+		XTM_PTV_COLUMN_VSZ_STR, vsz,
+		XTM_PTV_COLUMN_RSS, task->rss,
+		XTM_PTV_COLUMN_RSS_STR, rss,
+		XTM_PTV_COLUMN_CPU, task->cpu_user + task->cpu_system,
+		XTM_PTV_COLUMN_CPU_STR, cpu,
+		XTM_PTV_COLUMN_PRIORITY, task->prio,
+		-1);
+}
+
+static void
+model_find_tree_iter_for_pid (GtkTreeModel *model, guint pid, GtkTreeIter *iter)
+{
+	gboolean valid;
+	guint pid_iter;
+
+	valid = gtk_tree_model_get_iter_first (model, iter);
+	while (valid)
+	{
+		gtk_tree_model_get (model, iter, XTM_PTV_COLUMN_PID, &pid_iter, -1);
+		if (pid == pid_iter)
+			break;
+		valid = gtk_tree_model_iter_next (model, iter);
+	}
+}
+
+static void
+model_update_task (GtkTreeModel *model, Task *task)
+{
+	GtkTreeIter iter;
+	model_find_tree_iter_for_pid (model, task->pid, &iter);
+	model_update_tree_iter (model, &iter, task);
+}
+
+static void
+model_remove_task (GtkTreeModel *model, Task *task)
+{
+	GtkTreeIter iter;
+	model_find_tree_iter_for_pid (model, task->pid, &iter);
+	gtk_list_store_remove (GTK_LIST_STORE (model), &iter);
+}
+
 
 
 XtmTaskManager *
-xtm_task_manager_new ()
+xtm_task_manager_new (GtkTreeModel *model)
 {
-	return g_object_new (XTM_TYPE_TASK_MANAGER, NULL);
+	XtmTaskManager *manager = g_object_new (XTM_TYPE_TASK_MANAGER, NULL);
+	_xtm_task_manager_set_model (manager, model);
+	return manager;
 }
 
 const gchar *
@@ -155,24 +241,32 @@ xtm_task_manager_get_system_info (XtmTaskManager *manager, guint *num_processes,
 
 const GArray *
 xtm_task_manager_get_task_list (XtmTaskManager *manager)
+{
+	xtm_task_manager_update_model (manager);
+	return manager->tasks;
+}
+
+void
+xtm_task_manager_update_model (XtmTaskManager *manager)
 {
 	GArray *array;
+	GtkTreeIter iter;
+	gboolean valid;
 	guint i;
 
+	/* Retrieve initial task list and return */
 	if (manager->tasks->len == 0)
 	{
 		get_task_list (manager->tasks);
-#if 1|DEBUG
+		for (i = 0; i < manager->tasks->len; i++)
 		{
-			gint i;
+			Task *task = &g_array_index (manager->tasks, Task, i);
-			for (i = 0; i < manager->tasks->len; i++)
+			model_add_task (manager->model, task);
-			{
+#if 1|DEBUG
-				Task *task = &g_array_index (manager->tasks, Task, i);
+			g_print ("%5d %5s %15s %.50s\n", task->pid, task->uid_name, task->name, task->cmdline);
-				g_print ("%5d %5s %15s %.50s\n", task->pid, task->uid_name, task->name, task->cmdline);
-			}
-		}
 #endif
-		return manager->tasks;
+		}
+		return;
 	}
 
 	/* Retrieve new task list */
@@ -183,12 +277,13 @@ xtm_task_manager_get_task_list (XtmTaskManager *manager)
 	for (i = 0; i < manager->tasks->len; i++)
 	{
 		guint j;
+		Task *tasktmp;
 		Task *task = &g_array_index (manager->tasks, Task, i);
 		gboolean found = FALSE;
 
 		for (j = 0; j < array->len; j++)
 		{
-			Task *tasktmp = &g_array_index (array, Task, j);
+			tasktmp = &g_array_index (array, Task, j);
 			if (task->pid != tasktmp->pid)
 				continue;
 			found = TRUE;
@@ -196,7 +291,13 @@ xtm_task_manager_get_task_list (XtmTaskManager *manager)
 		}
 
 		if (found == FALSE)
+		{
+#if DEBUG
+			g_debug ("Remove old task %d %s", task->pid, task->name);
+#endif
+			model_remove_task (manager->model, task);
 			g_array_remove_index (manager->tasks, i);
+		}
 	}
 
 	/* Append started tasks and update existing ones */
@@ -214,24 +315,41 @@ xtm_task_manager_get_task_list (XtmTaskManager *manager)
 
 			found = TRUE;
 
-			task->ppid = tasktmp->ppid;
+			/* Update the model (with the rest) only if needed, this keeps the CPU cool */
-			if (g_strcmp0 (task->state, tasktmp->state))
+			if (task->ppid != tasktmp->ppid
+				|| g_strcmp0 (task->state, tasktmp->state)
+				|| task->cpu_user != tasktmp->cpu_user
+				|| task->cpu_system != tasktmp->cpu_system
+				|| task->rss != tasktmp->rss
+				|| task->vsz != tasktmp->vsz
+				|| task->prio != tasktmp->prio)
+			{
+				task->ppid = tasktmp->ppid;
 				g_strlcpy (task->state, tasktmp->state, sizeof (task->state));
-			task->cpu_user = tasktmp->cpu_user;
+				task->cpu_user = tasktmp->cpu_user;
-			task->cpu_system = tasktmp->cpu_system;
+				task->cpu_system = tasktmp->cpu_system;
-			task->rss = tasktmp->rss;
+				task->rss = tasktmp->rss;
-			task->vsz = tasktmp->vsz;
+				task->vsz = tasktmp->vsz;
-			task->prio = tasktmp->prio;
+				task->prio = tasktmp->prio;
+				model_update_task (manager->model, tasktmp);
+			}
+
 			break;
 		}
 
 		if (found == FALSE)
-			g_array_append_val (manager->tasks, tasktmp);
+		{
+#if DEBUG
+			g_debug ("Add new task %d %s", tasktmp->pid, tasktmp->name);
+#endif
+			model_add_task (manager->model, tasktmp);
+			g_array_append_val (manager->tasks, *tasktmp);
+		}
 	}
 
 	g_array_free (array, TRUE);
 
-	return manager->tasks;
+	return;
 }
 
 void

src/task-manager.h

@@ -66,5 +66,6 @@ const gchar *		xtm_task_manager_get_username			(XtmTaskManager *manager);
 const gchar *		xtm_task_manager_get_hostname			(XtmTaskManager *manager);
 void			xtm_task_manager_get_system_info		(XtmTaskManager *manager, guint *num_processes, gfloat *cpu, gfloat *memory, gfloat *swap);
 const GArray *		xtm_task_manager_get_task_list			(XtmTaskManager *manager);
+void			xtm_task_manager_update_model			(XtmTaskManager *manager);
 
 #endif /* !TASK_MANAGER_H */