File evolution-contact-list-performance.patch of Package evolution

Overview Repositories Revisions Requests Users Attributes Meta

File evolution-contact-list-performance.patch of Package evolution (Revision 927b5607f19ed2dcbedb7c203cc67ff9)

Currently displaying revision 927b5607f19ed2dcbedb7c203cc67ff9 , Show latest

From cffd6b51eca9fecd39a760c07da526d8ea4b98c4 Mon Sep 17 00:00:00 2001
From: Mike Gorse <mgorse@suse.com>
Date: Wed, 11 Jan 2017 19:56:48 +0100
Subject: [PATCH] Bug 774494 - ENameSelectorDialog slow with large address
 books

Wait until contacts are fetched before attaching a GtkTreeModelSort.
Similarly, when changing the view during a search, detach the
GtkTreeModelSort while updating the tree view, and use a hash table
to temporarily store contacts, as this speeds up searching.

Also, in ETreeModelStore, add a cache to optimize
generated_offset_to_child_offset to avoid needing to iterate through the
whole list to count rows when a value is needed.
---
 src/e-util/e-contact-store.c        | 81 ++++++++++++++++++++++++++++++++----
 src/e-util/e-contact-store.h        |  4 ++
 src/e-util/e-name-selector-dialog.c | 83 ++++++++++++++++++++++++++++---------
 src/e-util/e-tree-model-generator.c | 75 +++++++++++++++++++++++++++------
 4 files changed, 202 insertions(+), 41 deletions(-)

diff --git a/src/e-util/e-contact-store.c b/src/e-util/e-contact-store.c
index f81ffca..56c2b51 100644
--- a/src/e-util/e-contact-store.c
+++ b/src/e-util/e-contact-store.c
@@ -51,6 +51,8 @@ struct _EContactStorePrivate {
 enum {
 	START_CLIENT_VIEW,
 	STOP_CLIENT_VIEW,
+	START_UPDATE,
+	STOP_UPDATE,
 	LAST_SIGNAL
 };
 
@@ -182,6 +184,26 @@ e_contact_store_class_init (EContactStoreClass *class)
 		g_cclosure_marshal_VOID__OBJECT,
 		G_TYPE_NONE, 1,
 		E_TYPE_BOOK_CLIENT_VIEW);
+
+	signals[START_UPDATE] = g_signal_new (
+		"start-update",
+		G_OBJECT_CLASS_TYPE (object_class),
+		G_SIGNAL_RUN_LAST,
+		G_STRUCT_OFFSET (EContactStoreClass, start_update),
+		NULL, NULL,
+		g_cclosure_marshal_VOID__OBJECT,
+		G_TYPE_NONE, 1,
+		E_TYPE_BOOK_CLIENT_VIEW);
+
+	signals[STOP_UPDATE] = g_signal_new (
+		"stop-update",
+		G_OBJECT_CLASS_TYPE (object_class),
+		G_SIGNAL_RUN_LAST,
+		G_STRUCT_OFFSET (EContactStoreClass, stop_update),
+		NULL, NULL,
+		g_cclosure_marshal_VOID__OBJECT,
+		G_TYPE_NONE, 1,
+		E_TYPE_BOOK_CLIENT_VIEW);
 }
 
 static void
@@ -435,6 +457,42 @@ find_contact_by_view_and_uid (EContactStore *contact_store,
 	return -1;
 }
 
+static GHashTable *
+get_contact_hash (EContactStore *contact_store,
+                  EBookClientView *find_view)
+{
+	GArray *array;
+	ContactSource *source;
+	GPtrArray *contacts;
+	gint source_index;
+	gint ii;
+	GHashTable *hash;
+
+	source_index = find_contact_source_by_view (contact_store, find_view);
+	if (source_index < 0)
+		return NULL;
+
+	array = contact_store->priv->contact_sources;
+	source = &g_array_index (array, ContactSource, source_index);
+
+	if (find_view == source->client_view)
+		contacts = source->contacts;          /* Current view */
+	else
+		contacts = source->contacts_pending;  /* Pending view */
+
+	hash = g_hash_table_new (g_str_hash, g_str_equal);
+
+	for (ii = 0; ii < contacts->len; ii++) {
+		EContact *contact = g_ptr_array_index (contacts, ii);
+		const gchar *uid = e_contact_get_const (contact, E_CONTACT_UID);
+
+		if (uid)
+			g_hash_table_insert (hash, (gpointer) uid, GINT_TO_POINTER (ii));
+	}
+
+	return hash;
+}
+
 static gint
 find_contact_by_uid (EContactStore *contact_store,
                      const gchar *find_uid)
@@ -647,6 +705,7 @@ view_complete (EContactStore *contact_store,
 	ContactSource *source;
 	gint           offset;
 	gint           i;
+	GHashTable *hash;
 
 	if (!find_contact_source_details_by_view (contact_store, client_view, &source, &offset)) {
 		g_warning ("EContactStore got 'complete' signal from unknown EBookClientView!");
@@ -662,18 +721,17 @@ view_complete (EContactStore *contact_store,
 	g_return_if_fail (client_view == source->client_view_pending);
 
 	/* However, if it was a pending view, calculate and emit the differences between that
-	 * and the current view, and move the pending view up to current.
-	 *
-	 * This is O(m * n), and can be sped up with a temporary hash table if needed. */
+	 * and the current view, and move the pending view up to current. */
+
+	g_signal_emit (contact_store, signals[START_UPDATE], 0, client_view);
 
 	/* Deletions */
+	hash = get_contact_hash (contact_store, source->client_view_pending);
 	for (i = 0; i < source->contacts->len; i++) {
 		EContact    *old_contact = g_ptr_array_index (source->contacts, i);
 		const gchar *old_uid = e_contact_get_const (old_contact, E_CONTACT_UID);
-		gint         result;
 
-		result = find_contact_by_view_and_uid (contact_store, source->client_view_pending, old_uid);
-		if (result < 0) {
+		if (!g_hash_table_contains (hash, old_uid)) {
 			/* Contact is not in new view; removed */
 			g_object_unref (old_contact);
 			g_ptr_array_remove_index (source->contacts, i);
@@ -681,15 +739,15 @@ view_complete (EContactStore *contact_store,
 			i--;  /* Stay in place */
 		}
 	}
+	g_hash_table_unref (hash);
 
 	/* Insertions */
+	hash = get_contact_hash (contact_store, source->client_view);
 	for (i = 0; i < source->contacts_pending->len; i++) {
 		EContact    *new_contact = g_ptr_array_index (source->contacts_pending, i);
 		const gchar *new_uid = e_contact_get_const (new_contact, E_CONTACT_UID);
-		gint         result;
 
-		result = find_contact_by_view_and_uid (contact_store, source->client_view, new_uid);
-		if (result < 0) {
+		if (!g_hash_table_contains (hash, new_uid)) {
 			/* Contact is not in old view; inserted */
 			g_ptr_array_add (source->contacts, new_contact);
 			row_inserted (contact_store, offset + source->contacts->len - 1);
@@ -698,6 +756,9 @@ view_complete (EContactStore *contact_store,
 			g_object_unref (new_contact);
 		}
 	}
+	g_hash_table_unref (hash);
+
+	g_signal_emit (contact_store, signals[STOP_UPDATE], 0, client_view);
 
 	/* Move pending view up to current */
 	stop_view (contact_store, source->client_view);
@@ -805,6 +866,7 @@ clear_contact_source (EContactStore *contact_store,
 		GtkTreePath *path = gtk_tree_path_new ();
 		gint         i;
 
+		g_signal_emit (contact_store, signals[START_UPDATE], 0, source->client_view);
 		gtk_tree_path_append_index (path, source->contacts->len);
 
 		for (i = source->contacts->len - 1; i >= 0; i--) {
@@ -818,6 +880,7 @@ clear_contact_source (EContactStore *contact_store,
 		}
 
 		gtk_tree_path_free (path);
+		g_signal_emit (contact_store, signals[STOP_UPDATE], 0, source->client_view);
 	}
 
 	/* Free main and pending views, clear cached contacts */
diff --git a/src/e-util/e-contact-store.h b/src/e-util/e-contact-store.h
index f6a5ead..4d25f7a 100644
--- a/src/e-util/e-contact-store.h
+++ b/src/e-util/e-contact-store.h
@@ -67,6 +67,10 @@ struct _EContactStoreClass {
 						 EBookClientView *client_view);
 	void		(*stop_client_view)	(EContactStore *contact_store,
 						 EBookClientView *client_view);
+	void		(*start_update)		(EContactStore *contact_store,
+						 EBookClientView *client_view);
+	void		(*stop_update)		(EContactStore *contact_store,
+						 EBookClientView *client_view);
 };
 
 GType		e_contact_store_get_type	(void) G_GNUC_CONST;
diff --git a/src/e-util/e-name-selector-dialog.c b/src/e-util/e-name-selector-dialog.c
index f2db4d7..30e39fa 100644
--- a/src/e-util/e-name-selector-dialog.c
+++ b/src/e-util/e-name-selector-dialog.c
@@ -746,6 +746,44 @@ add_destination (ENameSelectorModel *name_selector_model,
 }
 
 static void
+disable_sort (ENameSelectorDialog *dialog)
+{
+	if (dialog->priv->contact_sort) {
+		g_object_unref (dialog->priv->contact_sort);
+		dialog->priv->contact_sort = NULL;
+	}
+
+	gtk_tree_view_set_model (
+		dialog->priv->contact_view,
+		NULL);
+}
+
+static void
+enable_sort (ENameSelectorDialog *dialog)
+{
+	ETreeModelGenerator *contact_filter;
+
+	/* Get contact store and its filter wrapper */
+	contact_filter = e_name_selector_model_peek_contact_filter (
+		dialog->priv->name_selector_model);
+
+	/* Create sorting model on top of filter, assign it to view */
+	if (!dialog->priv->contact_sort) {
+		dialog->priv->contact_sort = GTK_TREE_MODEL_SORT (
+			gtk_tree_model_sort_new_with_model (GTK_TREE_MODEL (contact_filter)));
+
+		/* sort on full name as we display full name in name selector dialog */
+		gtk_tree_sortable_set_sort_column_id (
+			GTK_TREE_SORTABLE (dialog->priv->contact_sort),
+			E_CONTACT_FULL_NAME, GTK_SORT_ASCENDING);
+	}
+
+	gtk_tree_view_set_model (
+		dialog->priv->contact_view,
+		GTK_TREE_MODEL (dialog->priv->contact_sort));
+}
+
+static void
 remove_books (ENameSelectorDialog *name_selector_dialog)
 {
 	EContactStore *contact_store;
@@ -771,8 +809,11 @@ remove_books (ENameSelectorDialog *name_selector_dialog)
 		g_object_unref (name_selector_dialog->priv->cancellable);
 		name_selector_dialog->priv->cancellable = NULL;
 	}
+
+	disable_sort (name_selector_dialog);
 }
 
+
 /* ------------------ *
  * Section management *
  * ------------------ */
@@ -1134,6 +1175,8 @@ view_complete (EBookClientView *view,
                ENameSelectorDialog *dialog)
 {
 	view_progress (view, -1, NULL, dialog);
+
+	enable_sort (dialog);
 }
 
 static void
@@ -1160,6 +1203,22 @@ stop_client_view_cb (EContactStore *store,
 }
 
 static void
+start_update_cb (EContactStore *store,
+                 EBookClientView *client_view,
+                 ENameSelectorDialog *name_selector_dialog)
+{
+	disable_sort (name_selector_dialog);
+}
+
+static void
+stop_update_cb (EContactStore *store,
+                EBookClientView *client_view,
+                ENameSelectorDialog *name_selector_dialog)
+{
+	enable_sort (name_selector_dialog);
+}
+
+static void
 name_selector_dialog_get_client_cb (GObject *source_object,
                                     GAsyncResult *result,
                                     gpointer user_data)
@@ -1589,7 +1648,6 @@ transfer_button_clicked (ENameSelectorDialog *name_selector_dialog,
 static void
 setup_name_selector_model (ENameSelectorDialog *name_selector_dialog)
 {
-	ETreeModelGenerator *contact_filter;
 	EContactStore       *contact_store;
 	GList               *new_sections;
 	GList               *l;
@@ -1625,29 +1683,12 @@ setup_name_selector_model (ENameSelectorDialog *name_selector_dialog)
 		name_selector_dialog->priv->name_selector_model, "section-removed",
 		G_CALLBACK (model_section_removed), name_selector_dialog);
 
-	/* Get contact store and its filter wrapper */
-
-	contact_filter = e_name_selector_model_peek_contact_filter (
-		name_selector_dialog->priv->name_selector_model);
-
-	/* Create sorting model on top of filter, assign it to view */
-
-	name_selector_dialog->priv->contact_sort = GTK_TREE_MODEL_SORT (
-		gtk_tree_model_sort_new_with_model (GTK_TREE_MODEL (contact_filter)));
-
-	/* sort on full name as we display full name in name selector dialog */
-	gtk_tree_sortable_set_sort_column_id (
-		GTK_TREE_SORTABLE (name_selector_dialog->priv->contact_sort),
-		E_CONTACT_FULL_NAME, GTK_SORT_ASCENDING);
-
-	gtk_tree_view_set_model (
-		name_selector_dialog->priv->contact_view,
-		GTK_TREE_MODEL (name_selector_dialog->priv->contact_sort));
-
 	contact_store = e_name_selector_model_peek_contact_store (name_selector_dialog->priv->name_selector_model);
 	if (contact_store) {
 		g_signal_connect (contact_store, "start-client-view", G_CALLBACK (start_client_view_cb), name_selector_dialog);
 		g_signal_connect (contact_store, "stop-client-view", G_CALLBACK (stop_client_view_cb), name_selector_dialog);
+		g_signal_connect (contact_store, "start-update", G_CALLBACK (start_update_cb), name_selector_dialog);
+		g_signal_connect (contact_store, "stop-update", G_CALLBACK (stop_update_cb), name_selector_dialog);
 	}
 
 	/* Make sure UI is consistent */
@@ -1684,6 +1725,8 @@ shutdown_name_selector_model (ENameSelectorDialog *name_selector_dialog)
 		if (contact_store) {
 			g_signal_handlers_disconnect_by_func (contact_store, start_client_view_cb, name_selector_dialog);
 			g_signal_handlers_disconnect_by_func (contact_store, stop_client_view_cb, name_selector_dialog);
+			g_signal_handlers_disconnect_by_func (contact_store, start_update_cb, name_selector_dialog);
+			g_signal_handlers_disconnect_by_func (contact_store, stop_update_cb, name_selector_dialog);
 		}
 
 		g_signal_handlers_disconnect_matched (
diff --git a/src/e-util/e-tree-model-generator.c b/src/e-util/e-tree-model-generator.c
index b8fe4a3..ec49712 100644
--- a/src/e-util/e-tree-model-generator.c
+++ b/src/e-util/e-tree-model-generator.c
@@ -56,8 +56,14 @@ struct _ETreeModelGeneratorPrivate {
 
 	ETreeModelGeneratorModifyFunc modify_func;
 	gpointer modify_func_data;
+	GSList *offset_cache;
 };
 
+typedef struct {
+	gint offset;
+	gint index;
+} CacheItem;
+
 static void e_tree_model_generator_tree_model_init (GtkTreeModelIface *iface);
 
 G_DEFINE_TYPE_WITH_CODE (
@@ -192,6 +198,8 @@ tree_model_generator_finalize (GObject *object)
 	if (tree_model_generator->priv->root_nodes)
 		release_node_map (tree_model_generator->priv->root_nodes);
 
+	g_slist_free_full (tree_model_generator->priv->offset_cache, g_free);
+
 	/* Chain up to parent's finalize() method. */
 	G_OBJECT_CLASS (e_tree_model_generator_parent_class)->finalize (object);
 }
@@ -300,15 +308,44 @@ row_changed (ETreeModelGenerator *tree_model_generator,
 static gint
 generated_offset_to_child_offset (GArray *group,
                                   gint offset,
-                                  gint *internal_offset)
+                                  gint *internal_offset,
+                                  GSList **cache_p)
 {
 	gboolean success = FALSE;
 	gint     accum_offset = 0;
 	gint     i;
+	GSList *cache, *cache_last;
+	gint last_cached_offset;
+
+	i = 0;
+	cache = *cache_p;
+	cache_last = NULL;
+	last_cached_offset = 0;
+	for (; cache; cache = cache->next) {
+		CacheItem *item = cache->data;
+		cache_last = cache;
+		last_cached_offset = item->offset;
+		if (item->offset <= offset) {
+			i = item->index;
+			accum_offset = item->offset;
+		} else
+			break;
+	}
 
-	for (i = 0; i < group->len; i++) {
+	for (; i < group->len; i++) {
 		Node *node = &g_array_index (group, Node, i);
 
+		if (accum_offset - last_cached_offset > 500) {
+			CacheItem *item = g_malloc (sizeof (CacheItem));
+			item->offset = accum_offset;
+			item->index = i;
+			last_cached_offset = accum_offset;
+			if (cache_last)
+				cache_last = g_slist_last (g_slist_append (cache_last, item));
+			else
+				*cache_p = cache_last = g_slist_append (NULL, item);
+		}
+
 		accum_offset += node->n_generated;
 		if (accum_offset > offset) {
 			accum_offset -= node->n_generated;
@@ -395,6 +432,9 @@ build_node_map (ETreeModelGenerator *tree_model_generator,
 	GtkTreeIter  iter;
 	gboolean     result;
 
+	g_slist_free_full (tree_model_generator->priv->offset_cache, g_free);
+	tree_model_generator->priv->offset_cache = NULL;
+
 	if (parent_iter)
 		result = gtk_tree_model_iter_children (tree_model_generator->priv->child_model, &iter, parent_iter);
 	else
@@ -519,6 +559,9 @@ create_node_at_child_path (ETreeModelGenerator *tree_model_generator,
 
 	append_node (group);
 
+	g_slist_free_full (tree_model_generator->priv->offset_cache, g_free);
+	tree_model_generator->priv->offset_cache = NULL;
+
 	if (group->len - 1 - index > 0) {
 		gint i;
 
@@ -587,6 +630,9 @@ delete_node_at_child_path (ETreeModelGenerator *tree_model_generator,
 	Node        *node;
 	gint         i;
 
+	g_slist_free_full (tree_model_generator->priv->offset_cache, g_free);
+	tree_model_generator->priv->offset_cache = NULL;
+
 	parent_path = gtk_tree_path_copy (path);
 	gtk_tree_path_up (parent_path);
 	node = get_node_by_child_path (tree_model_generator, parent_path, &parent_group);
@@ -658,6 +704,11 @@ child_row_changed (ETreeModelGenerator *tree_model_generator,
 		gtk_tree_path_next (generated_path);
 	}
 
+	if (n_generated != node->n_generated) {
+		g_slist_free_full (tree_model_generator->priv->offset_cache, g_free);
+		tree_model_generator->priv->offset_cache = NULL;
+	}
+
 	for (; i < node->n_generated; ) {
 		node->n_generated--;
 		row_deleted (tree_model_generator, generated_path);
@@ -946,7 +997,7 @@ e_tree_model_generator_convert_path_to_child_path (ETreeModelGenerator *tree_mod
 		}
 
 		index = gtk_tree_path_get_indices (generator_path)[depth];
-		child_index = generated_offset_to_child_offset (group, index, NULL);
+		child_index = generated_offset_to_child_offset (group, index, NULL, &tree_model_generator->priv->offset_cache);
 		node = &g_array_index (group, Node, child_index);
 		group = node->child_nodes;
 
@@ -985,7 +1036,7 @@ e_tree_model_generator_convert_iter_to_child_iter (ETreeModelGenerator *tree_mod
 	path = gtk_tree_path_new ();
 	ITER_GET (generator_iter, &group, &index);
 
-	index = generated_offset_to_child_offset (group, index, &internal_offset);
+	index = generated_offset_to_child_offset (group, index, &internal_offset, &tree_model_generator->priv->offset_cache);
 	gtk_tree_path_prepend_index (path, index);
 
 	while (group) {
@@ -1066,7 +1117,7 @@ e_tree_model_generator_get_iter (GtkTreeModel *tree_model,
 		gint  child_index;
 
 		index = gtk_tree_path_get_indices (path)[depth];
-		child_index = generated_offset_to_child_offset (group, index, NULL);
+		child_index = generated_offset_to_child_offset (group, index, NULL, &tree_model_generator->priv->offset_cache);
 		if (child_index < 0)
 			return FALSE;
 
@@ -1103,7 +1154,7 @@ e_tree_model_generator_get_path (GtkTreeModel *tree_model,
 	 * lists, not sure about trees. */
 
 	gtk_tree_path_prepend_index (path, index);
-	index = generated_offset_to_child_offset (group, index, NULL);
+	index = generated_offset_to_child_offset (group, index, NULL, &tree_model_generator->priv->offset_cache);
 
 	while (group) {
 		Node *node = &g_array_index (group, Node, index);
@@ -1135,7 +1186,7 @@ e_tree_model_generator_iter_next (GtkTreeModel *tree_model,
 	g_return_val_if_fail (ITER_IS_VALID (tree_model_generator, iter), FALSE);
 
 	ITER_GET (iter, &group, &index);
-	child_index = generated_offset_to_child_offset (group, index, &internal_offset);
+	child_index = generated_offset_to_child_offset (group, index, &internal_offset, &tree_model_generator->priv->offset_cache);
 	node = &g_array_index (group, Node, child_index);
 
 	if (internal_offset + 1 < node->n_generated ||
@@ -1169,7 +1220,7 @@ e_tree_model_generator_iter_children (GtkTreeModel *tree_model,
 	}
 
 	ITER_GET (parent, &group, &index);
-	index = generated_offset_to_child_offset (group, index, NULL);
+	index = generated_offset_to_child_offset (group, index, NULL, &tree_model_generator->priv->offset_cache);
 	if (index < 0)
 		return FALSE;
 
@@ -1205,7 +1256,7 @@ e_tree_model_generator_iter_has_child (GtkTreeModel *tree_model,
 	}
 
 	ITER_GET (iter, &group, &index);
-	index = generated_offset_to_child_offset (group, index, NULL);
+	index = generated_offset_to_child_offset (group, index, NULL, &tree_model_generator->priv->offset_cache);
 	if (index < 0)
 		return FALSE;
 
@@ -1236,7 +1287,7 @@ e_tree_model_generator_iter_n_children (GtkTreeModel *tree_model,
 			count_generated_nodes (tree_model_generator->priv->root_nodes) : 0;
 
 	ITER_GET (iter, &group, &index);
-	index = generated_offset_to_child_offset (group, index, NULL);
+	index = generated_offset_to_child_offset (group, index, NULL, &tree_model_generator->priv->offset_cache);
 	if (index < 0)
 		return 0;
 
@@ -1273,7 +1324,7 @@ e_tree_model_generator_iter_nth_child (GtkTreeModel *tree_model,
 	}
 
 	ITER_GET (parent, &group, &index);
-	index = generated_offset_to_child_offset (group, index, NULL);
+	index = generated_offset_to_child_offset (group, index, NULL, &tree_model_generator->priv->offset_cache);
 	if (index < 0)
 		return FALSE;
 
@@ -1303,7 +1354,7 @@ e_tree_model_generator_iter_parent (GtkTreeModel *tree_model,
 	g_return_val_if_fail (ITER_IS_VALID (tree_model_generator, iter), FALSE);
 
 	ITER_GET (child, &group, &index);
-	index = generated_offset_to_child_offset (group, index, NULL);
+	index = generated_offset_to_child_offset (group, index, NULL, &tree_model_generator->priv->offset_cache);
 	if (index < 0)
 		return FALSE;
 
-- 
2.6.6

Places

File evolution-contact-list-performance.patch of Package evolution (Revision 927b5607f19ed2dcbedb7c203cc67ff9)

Places