From 41a8f63ce28e228611d72156abb39d496691d8be Mon Sep 17 00:00:00 2001 From: Alan Fitton Date: Wed, 11 Jan 2012 19:52:57 +0000 Subject: fix memory leaks and parse a files tree in a thread if there's over 600 files, to avoid UI freezes on massive torrents. --- src/trg-file-parser.c | 2 +- src/trg-files-model.c | 136 +++++++++++++++++++++++++++++++++++-------- src/trg-files-model.h | 2 + src/trg-files-tree.c | 3 + src/trg-main-window.c | 19 +++--- src/trg-torrent-add-dialog.c | 2 +- 6 files changed, 131 insertions(+), 33 deletions(-) diff --git a/src/trg-file-parser.c b/src/trg-file-parser.c index 60dfe1f..7cf9b89 100644 --- a/src/trg-file-parser.c +++ b/src/trg-file-parser.c @@ -81,7 +81,6 @@ static trg_files_tree_node } if (isFile) { - target_node->index = index; target_node->length = (gint64) file_length_node->val.i; while (lastIter) @@ -91,6 +90,7 @@ static trg_files_tree_node } } + target_node->index = isFile ? index : -1; lastIter = target_node; } diff --git a/src/trg-files-model.c b/src/trg-files-model.c index ab9eec5..b3688f1 100644 --- a/src/trg-files-model.c +++ b/src/trg-files-model.c @@ -45,6 +45,12 @@ struct _TrgFilesModelPrivate { gboolean accept; }; +/* Push a given increment to a treemodel node and its parents. + * Used for updating bytescompleted. + * This is only used for user interaction, the initial population is done + * in a simple tree before for performance. + * */ + static void trg_files_update_parent_progress(GtkTreeModel * model, GtkTreeIter * iter, gint64 increment) @@ -74,6 +80,12 @@ static void trg_files_update_parent_progress(GtkTreeModel * model, } } +/* Update the bytesCompleted and size for a nodes parents, and also figure out + * if a priority/enabled change requires updating the parents (needs to iterate + * over the other nodes at its level). + * + * It's faster doing it in here than when it's in the model. + */ static void trg_files_tree_update_ancestors(trg_files_tree_node * node) { trg_files_tree_node *back_iter = node; @@ -141,6 +153,9 @@ static trg_files_tree_node GList *li; int i; + /* Build up a list of pointers to each parent trg_files_tree_node + * reversing the order as it iterates over its parent. + */ if (lastIter) while ((lastIter = lastIter->parent)) parentList = g_list_prepend(parentList, lastIter); @@ -155,6 +170,10 @@ static trg_files_tree_node gboolean isFile = !path[i + 1]; trg_files_tree_node *target_node = NULL; + /* No point checking for files. If there is a last parents iterator + * check it for a shortcut. I'm assuming that these come in order of + * directory at least to give performance a boost. + */ if (li && !isFile) { trg_files_tree_node *lastPathNode = (trg_files_tree_node *) li->data; @@ -163,24 +182,33 @@ static trg_files_tree_node target_node = lastPathNode; li = g_list_next(li); } else { + /* No need to check any further. */ li = NULL; } } + /* Node needs creating */ + if (!target_node) { target_node = g_new0(trg_files_tree_node, 1); target_node->name = g_strdup(path[i]); target_node->parent = lastIter; + /* Under the parent of the last iteration. */ if (lastIter) lastIter->children = g_list_append(lastIter->children, target_node); + /* None set, so under the top node. */ else top->children = g_list_append(top->children, target_node); } lastIter = target_node; + /* Files have more properties set here than for files. + * Directories are updated from here too, by trg_files_tree_update_ancestors + * working up the parents. + */ if (isFile) { target_node->length = file_get_length(file); target_node->bytesCompleted = file_get_bytes_completed(file); @@ -197,6 +225,7 @@ static trg_files_tree_node } g_list_free(parentList); + g_strfreev(path); return lastIter; } @@ -286,46 +315,107 @@ gboolean trg_files_model_update_foreach(GtkListStore * model, return FALSE; } +struct FirstUpdateThreadData +{ + TrgFilesModel * model; + JsonArray *files; + gint n_items; + trg_files_tree_node *top_node; + gint64 torrent_id; + GList *filesList; + gboolean idle_add; +}; + +static gboolean trg_files_model_applytree_idlefunc(gpointer data) +{ + struct FirstUpdateThreadData* args = (struct FirstUpdateThreadData*)data; + TrgFilesModelPrivate *priv = TRG_FILES_MODEL_GET_PRIVATE(args->model); + + if (args->torrent_id == priv->torrentId) { + store_add_node(GTK_TREE_STORE(args->model), NULL, args->top_node); + priv->n_items = args->n_items; + priv->accept = TRUE; + } + + trg_files_tree_node_free(args->top_node); + g_free(data); + + return FALSE; +} + +static gpointer trg_files_model_buildtree_threadfunc(gpointer data) +{ + struct FirstUpdateThreadData* args = (struct FirstUpdateThreadData*)data; + TrgFilesModelPrivate *priv = TRG_FILES_MODEL_GET_PRIVATE(args->model); + trg_files_tree_node *lastNode = NULL; + GList *li; + + args->top_node = g_new0(trg_files_tree_node, 1); + + for (li = args->filesList; li; li = g_list_next(li)) { + JsonObject *file = json_node_get_object((JsonNode *) li->data); + + lastNode = + trg_file_parser_node_insert(args->top_node, lastNode, + file, args->n_items++, priv->wanted, + priv->priorities); + } + + g_list_free(args->filesList); + json_array_unref(args->files); + + if (args->idle_add) + g_idle_add(trg_files_model_applytree_idlefunc, data); + + return NULL; +} + void trg_files_model_update(TrgFilesModel * model, gint64 updateSerial, JsonObject * t, gint mode) { TrgFilesModelPrivate *priv = TRG_FILES_MODEL_GET_PRIVATE(model); - GList *filesList, *li; - JsonObject *file; - gint j = 0; + JsonArray *files = torrent_get_files(t); + GList *filesList = json_array_get_elements(files); + guint filesListLength = g_list_length(filesList); priv->torrentId = torrent_get_id(t); priv->priorities = torrent_get_priorities(t); priv->wanted = torrent_get_wanted(t); - filesList = json_array_get_elements(torrent_get_files(t)); + /* It's quicker to build this up with simple data structures before + * putting it into GTK models. + */ + if (mode == TORRENT_GET_MODE_FIRST || priv->n_items != filesListLength) { + struct FirstUpdateThreadData *futd = g_new0(struct FirstUpdateThreadData, 1); - if (mode == TORRENT_GET_MODE_FIRST || priv->n_items != g_list_length(filesList)) { - trg_files_tree_node *top_node = g_new0(trg_files_tree_node, 1); - trg_files_tree_node *lastNode = NULL; gtk_tree_store_clear(GTK_TREE_STORE(model)); - priv->accept = TRUE; - - for (li = filesList; li; li = g_list_next(li)) { - file = json_node_get_object((JsonNode *) li->data); - - lastNode = - trg_file_parser_node_insert(top_node, lastNode, - file, j++, priv->wanted, - priv->priorities); + json_array_ref(files); + + futd->files = files; + futd->filesList = filesList; + futd->torrent_id = priv->torrentId; + futd->model = model; + futd->idle_add = filesListLength > TRG_FILES_MODEL_CREATE_THREAD_IF_GT; + + /* If this update has more than a given number of files, build up the + * simple tree in a thread, then g_idle_add a function whichs + * add the contents of this prebuilt tree. + * + * If less than or equal to, I don't think it's worth spawning threads + * for. Just do it in the main loop. + */ + if (futd->idle_add) { + g_thread_create(trg_files_model_buildtree_threadfunc, futd, FALSE, NULL); + } else { + trg_files_model_buildtree_threadfunc(futd); + trg_files_model_applytree_idlefunc(futd); } - - priv->n_items = j; - - store_add_node(GTK_TREE_STORE(model), NULL, top_node); - trg_files_tree_node_free(top_node); } else { gtk_tree_model_foreach(GTK_TREE_MODEL(model), (GtkTreeModelForeachFunc) trg_files_model_update_foreach, filesList); + g_list_free(filesList); } - - g_list_free(filesList); } gint64 trg_files_model_get_torrent_id(TrgFilesModel * model) diff --git a/src/trg-files-model.h b/src/trg-files-model.h index cc6ebfe..7a53a55 100644 --- a/src/trg-files-model.h +++ b/src/trg-files-model.h @@ -60,6 +60,8 @@ G_END_DECLS enum { FILESCOL_COLUMNS }; +#define TRG_FILES_MODEL_CREATE_THREAD_IF_GT 600 + void trg_files_model_update(TrgFilesModel * model, gint64 updateSerial, JsonObject * t, gint mode); diff --git a/src/trg-files-tree.c b/src/trg-files-tree.c index 4da802c..cadef12 100644 --- a/src/trg-files-tree.c +++ b/src/trg-files-tree.c @@ -17,6 +17,9 @@ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. */ +/* This is the stuff common between both files trees, built up before + * populating the model. + */ #include #include diff --git a/src/trg-main-window.c b/src/trg-main-window.c index aa074f2..e4bea05 100644 --- a/src/trg-main-window.c +++ b/src/trg-main-window.c @@ -887,6 +887,13 @@ gboolean on_session_set(gpointer data) { return FALSE; } +static gboolean hasEnabledChanged(JsonObject *a, JsonObject *b, + const gchar *key) +{ + return json_object_get_boolean_member(a, key) != + json_object_get_boolean_member(b, key); +} + static gboolean on_session_get(gpointer data) { trg_response *response = (trg_response *) data; TrgMainWindow *win = TRG_MAIN_WINDOW(response->cb_data); @@ -935,14 +942,10 @@ static gboolean on_session_get(gpointer data) { && g_strcmp0(session_get_download_dir(lastSession), session_get_download_dir(newSession)); gboolean refreshSpeed = lastSession - && ((session_get_alt_speed_enabled(lastSession) - != session_get_alt_speed_enabled(newSession)) - || (session_get_speed_limit_down_enabled(lastSession) - != session_get_speed_limit_down_enabled( - newSession)) - || (session_get_speed_limit_up_enabled(lastSession) - != session_get_speed_limit_up_enabled( - newSession))); + && + (hasEnabledChanged(lastSession, newSession, SGET_ALT_SPEED_ENABLED) + || hasEnabledChanged(lastSession, newSession, SGET_SPEED_LIMIT_DOWN_ENABLED) + || hasEnabledChanged(lastSession, newSession, SGET_SPEED_LIMIT_UP_ENABLED)); trg_client_set_session(client, newSession); diff --git a/src/trg-torrent-add-dialog.c b/src/trg-torrent-add-dialog.c index b78c30a..3345a80 100644 --- a/src/trg-torrent-add-dialog.c +++ b/src/trg-torrent-add-dialog.c @@ -471,7 +471,7 @@ static void store_add_node(GtkTreeStore * store, GtkTreeIter * parent, if (node->name) { gtk_tree_store_append(store, &child, parent); gtk_tree_store_set(store, &child, FC_LABEL, node->name, FC_ENABLED, - 1, FC_INDEX, node->children ? -1 : node->index, + 1, FC_INDEX, node->index, FC_PRIORITY, TR_PRI_NORMAL, FC_SIZE, node->length, -1); } -- cgit v1.2.3