From cffd6b51eca9fecd39a760c07da526d8ea4b98c4 Mon Sep 17 00:00:00 2001 From: Mike Gorse 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