From 2ff91cf4e10b509a7237dfd0894d31ec52bfbf0f Mon Sep 17 00:00:00 2001 From: Robert Fewell <14uBobIT@gmail.com> Date: Wed, 30 Oct 2019 13:39:06 +0000 Subject: [PATCH] Bug 797453 - Chart of Accounts is slow to update / redraw The chart of Accounts tree view gets redrawn often and gets the data from the model via gnc_tree_model_account_get_value(). As such the numeric values are constantly being recalculated on every draw which is not ideal. This change caches all the string values to a GHashTable for retrieval which should be quicker than before. Changes to an account will clear the cache for that account and its parents so values can be updated. Changes to the preference negative colour clears the whole cache. --- gnucash/gnome-utils/gnc-tree-model-account.c | 160 +++++++++++++++++++ gnucash/gnome-utils/gnc-tree-model-account.h | 5 + gnucash/gnome-utils/gnc-tree-view-account.c | 11 ++ gnucash/gnome-utils/gnc-tree-view-account.h | 7 + gnucash/gnome/dialog-price-edit-db.c | 2 + gnucash/gnome/gnc-plugin-page-account-tree.c | 4 + 6 files changed, 189 insertions(+) diff --git a/gnucash/gnome-utils/gnc-tree-model-account.c b/gnucash/gnome-utils/gnc-tree-model-account.c index 21ea030b1f..e5a3d42f74 100644 --- a/gnucash/gnome-utils/gnc-tree-model-account.c +++ b/gnucash/gnome-utils/gnc-tree-model-account.c @@ -97,6 +97,9 @@ typedef struct GncTreeModelAccountPrivate Account *root; gint event_handler_id; const gchar *negative_color; + + GHashTable *account_values_hash; + } GncTreeModelAccountPrivate; #define GNC_TREE_MODEL_ACCOUNT_GET_PRIVATE(o) \ @@ -134,9 +137,16 @@ gnc_tree_model_account_update_color (gpointer gsettings, gchar *key, gpointer us g_return_if_fail(GNC_IS_TREE_MODEL_ACCOUNT(user_data)); model = user_data; priv = GNC_TREE_MODEL_ACCOUNT_GET_PRIVATE(model); + + // destroy/recreate the cached acount value hash to force update + g_hash_table_destroy (priv->account_values_hash); + priv->account_values_hash = g_hash_table_new_full (g_str_hash, g_str_equal, + g_free, g_free); + use_red = gnc_prefs_get_bool (GNC_PREFS_GROUP_GENERAL, GNC_PREF_NEGATIVE_IN_RED); priv->negative_color = use_red ? get_negative_color () : NULL; } + /************************************************************/ /* g_object required functions */ /************************************************************/ @@ -182,6 +192,10 @@ gnc_tree_model_account_init (GncTreeModelAccount *model) priv->root = NULL; priv->negative_color = red ? get_negative_color () : NULL; + // create the account values cache hash + priv->account_values_hash = g_hash_table_new_full (g_str_hash, g_str_equal, + g_free, g_free); + gnc_prefs_register_cb(GNC_PREFS_GROUP_GENERAL, GNC_PREF_NEGATIVE_IN_RED, gnc_tree_model_account_update_color, model); @@ -230,6 +244,9 @@ gnc_tree_model_account_dispose (GObject *object) priv->event_handler_id = 0; } + // destroy the cached acount values + g_hash_table_destroy (priv->account_values_hash); + gnc_prefs_remove_cb_by_func(GNC_PREFS_GROUP_GENERAL, GNC_PREF_NEGATIVE_IN_RED, gnc_tree_model_account_update_color, model); @@ -561,6 +578,131 @@ gnc_tree_model_account_compute_period_balance(GncTreeModelAccount *model, return g_strdup(xaccPrintAmount(b3, gnc_account_print_info(acct, TRUE))); } +static gboolean +row_changed_foreach_func (GtkTreeModel *model, GtkTreePath *path, + GtkTreeIter *iter, gpointer user_data) +{ + gtk_tree_model_row_changed (model, path, iter); + return FALSE; +} + +void +gnc_tree_model_account_clear_cache (GncTreeModelAccount *model) +{ + if (model) + { + GncTreeModelAccountPrivate *priv = GNC_TREE_MODEL_ACCOUNT_GET_PRIVATE(model); + + // destroy the cached acount values and recreate + g_hash_table_destroy (priv->account_values_hash); + priv->account_values_hash = g_hash_table_new_full (g_str_hash, g_str_equal, + g_free, g_free); + + gtk_tree_model_foreach (GTK_TREE_MODEL(model), row_changed_foreach_func, NULL); + } +} + +static void +clear_account_cached_values (GncTreeModelAccount *model, GHashTable *hash, Account *account) +{ + GtkTreeIter iter; + gchar acct_guid_str[GUID_ENCODING_LENGTH + 1]; + + if (!account) + return; + + // make sure tree view sees the change + if (gnc_tree_model_account_get_iter_from_account (model, account, &iter)) + { + GtkTreePath *path = gtk_tree_model_get_path (GTK_TREE_MODEL(model), &iter); + + gtk_tree_model_row_changed (GTK_TREE_MODEL(model), path, &iter); + gtk_tree_path_free (path); + } + + guid_to_string_buff (xaccAccountGetGUID (account), acct_guid_str); + + // loop over the columns and remove any found + for (gint col = 0; col <= GNC_TREE_MODEL_ACCOUNT_NUM_COLUMNS; col++) + { + gchar *key = g_strdup_printf ("%s,%d", acct_guid_str, col); + + g_hash_table_remove (hash, key); + g_free (key); + } +} + +static void +gnc_tree_model_account_clear_cached_values (GncTreeModelAccount *model, Account *account) +{ + GncTreeModelAccountPrivate *priv = GNC_TREE_MODEL_ACCOUNT_GET_PRIVATE(model); + Account *parent; + + // no hash table or account, return + if ((!priv->account_values_hash) || (!account)) + return; + + clear_account_cached_values (model, priv->account_values_hash, account); + parent = gnc_account_get_parent (account); + + // clear also all parent accounts, this will update any balances/totals + while (parent) + { + clear_account_cached_values (model, priv->account_values_hash, parent); + parent = gnc_account_get_parent (parent); + } +} + +static gboolean +gnc_tree_model_account_get_cached_value (GncTreeModelAccount *model, Account *account, + gint column, gchar **cached_string) +{ + GncTreeModelAccountPrivate *priv = GNC_TREE_MODEL_ACCOUNT_GET_PRIVATE(model); + gchar acct_guid_str[GUID_ENCODING_LENGTH + 1]; + gchar *key = NULL; + gpointer value; + gboolean found; + + if ((!priv->account_values_hash) || (!account)) + return FALSE; + + guid_to_string_buff (xaccAccountGetGUID (account), acct_guid_str); + key = g_strdup_printf ("%s,%d", acct_guid_str, column); + + found = g_hash_table_lookup_extended (priv->account_values_hash, key, + NULL, &value); + + if (found) + *cached_string = g_strdup (value); + + g_free (key); + + return found; +} + +static void +gnc_tree_model_account_set_cached_value (GncTreeModelAccount *model, Account *account, + gint column, GValue *value) +{ + GncTreeModelAccountPrivate *priv = GNC_TREE_MODEL_ACCOUNT_GET_PRIVATE(model); + + if ((!priv->account_values_hash) || (!account)) + return; + + // only interested in string values + if (G_VALUE_HOLDS_STRING(value)) + { + gchar acct_guid_str[GUID_ENCODING_LENGTH + 1]; + const gchar *str = g_value_get_string (value); + gchar *key = NULL; + + guid_to_string_buff (xaccAccountGetGUID (account), acct_guid_str); + key = g_strdup_printf ("%s,%d", acct_guid_str, column); + + g_hash_table_insert (priv->account_values_hash, key, g_strdup (str)); + } +} + static void gnc_tree_model_account_get_value (GtkTreeModel *tree_model, GtkTreeIter *iter, @@ -572,6 +714,8 @@ gnc_tree_model_account_get_value (GtkTreeModel *tree_model, Account *account; gboolean negative; /* used to set "deficit style" also known as red numbers */ gchar *string; + gchar *cached_string = NULL; + time64 last_date; g_return_if_fail (GNC_IS_TREE_MODEL_ACCOUNT (model)); @@ -585,6 +729,14 @@ gnc_tree_model_account_get_value (GtkTreeModel *tree_model, account = (Account *) iter->user_data; priv = GNC_TREE_MODEL_ACCOUNT_GET_PRIVATE(model); + // lets see if the value is in the cache + if (gnc_tree_model_account_get_cached_value (model, account, column, &cached_string)) + { + g_value_init (value, G_TYPE_STRING); + g_value_take_string (value, cached_string); + return; + } + switch (column) { case GNC_TREE_MODEL_ACCOUNT_COL_NAME: @@ -797,6 +949,10 @@ gnc_tree_model_account_get_value (GtkTreeModel *tree_model, g_assert_not_reached (); break; } + + // save the value to the account values cache + gnc_tree_model_account_set_cached_value (model, account, column, value); + LEAVE(" "); } @@ -1272,6 +1428,10 @@ gnc_tree_model_account_event_handler (QofInstance *entity, priv = GNC_TREE_MODEL_ACCOUNT_GET_PRIVATE(model); account = GNC_ACCOUNT(entity); + + /* clear the cached model values for account */ + gnc_tree_model_account_clear_cached_values (model, account); + if (gnc_account_get_book(account) != priv->book) { LEAVE("not in this book"); diff --git a/gnucash/gnome-utils/gnc-tree-model-account.h b/gnucash/gnome-utils/gnc-tree-model-account.h index 9561ad04ab..0c193dd5b6 100644 --- a/gnucash/gnome-utils/gnc-tree-model-account.h +++ b/gnucash/gnome-utils/gnc-tree-model-account.h @@ -122,6 +122,11 @@ typedef struct */ GType gnc_tree_model_account_get_type (void); +/** Clear the tree model account cached values. + * + * @param model A pointer to the account tree model. + */ +void gnc_tree_model_account_clear_cache (GncTreeModelAccount *model); /** @name Account Tree Model Constructors @{ */ diff --git a/gnucash/gnome-utils/gnc-tree-view-account.c b/gnucash/gnome-utils/gnc-tree-view-account.c index 44f9dac18a..5efa3d20db 100644 --- a/gnucash/gnome-utils/gnc-tree-view-account.c +++ b/gnucash/gnome-utils/gnc-tree-view-account.c @@ -1076,6 +1076,17 @@ gnc_tree_view_account_count_children (GncTreeViewAccount *view, return num_children; } +void +gnc_tree_view_account_clear_model_cache (GncTreeViewAccount *view) +{ + GtkTreeModel *model, *f_model, *s_model; + + s_model = gtk_tree_view_get_model (GTK_TREE_VIEW(view)); + f_model = gtk_tree_model_sort_get_model (GTK_TREE_MODEL_SORT(s_model)); + model = gtk_tree_model_filter_get_model (GTK_TREE_MODEL_FILTER(f_model)); + + gnc_tree_model_account_clear_cache (GNC_TREE_MODEL_ACCOUNT(model)); +} /************************************************************/ /* Account Tree View Filter Functions */ diff --git a/gnucash/gnome-utils/gnc-tree-view-account.h b/gnucash/gnome-utils/gnc-tree-view-account.h index 9da2769cd1..db9279a988 100644 --- a/gnucash/gnome-utils/gnc-tree-view-account.h +++ b/gnucash/gnome-utils/gnc-tree-view-account.h @@ -342,6 +342,13 @@ void gnc_tree_view_account_refilter (GncTreeViewAccount *view); gint gnc_tree_view_account_count_children (GncTreeViewAccount *view, Account *account); +/** This function clears the tree model account cache so the values will + * be updated/refreshed. + * + * @param view A pointer to an account tree view. + * + */ +void gnc_tree_view_account_clear_model_cache (GncTreeViewAccount *view); /** This function returns the account associated with the specified diff --git a/gnucash/gnome/dialog-price-edit-db.c b/gnucash/gnome/dialog-price-edit-db.c index ef89a72e61..d6ce1d1b82 100644 --- a/gnucash/gnome/dialog-price-edit-db.c +++ b/gnucash/gnome/dialog-price-edit-db.c @@ -205,6 +205,7 @@ gnc_prices_dialog_remove_clicked (GtkWidget *widget, gpointer data) g_list_foreach(price_list, (GFunc)remove_helper, pdb_dialog->price_db); } g_list_free(price_list); + gnc_gui_refresh_all (); LEAVE(" "); } @@ -504,6 +505,7 @@ gnc_prices_dialog_remove_old_clicked (GtkWidget *widget, gpointer data) } g_list_free (comm_list); } + gnc_gui_refresh_all (); gtk_widget_destroy (pdb_dialog->remove_dialog); LEAVE(" "); } diff --git a/gnucash/gnome/gnc-plugin-page-account-tree.c b/gnucash/gnome/gnc-plugin-page-account-tree.c index 76b571b657..ecd5a2369b 100644 --- a/gnucash/gnome/gnc-plugin-page-account-tree.c +++ b/gnucash/gnome/gnc-plugin-page-account-tree.c @@ -602,6 +602,8 @@ gnc_plugin_page_account_refresh_cb (GHashTable *changes, gpointer user_data) return; priv = GNC_PLUGIN_PAGE_ACCOUNT_TREE_GET_PRIVATE(page); + + gnc_tree_view_account_clear_model_cache (GNC_TREE_VIEW_ACCOUNT(priv->tree_view)); gtk_widget_queue_draw(priv->widget); } @@ -1671,6 +1673,8 @@ gnc_plugin_page_account_tree_cmd_refresh (GtkAction *action, g_return_if_fail(GNC_IS_PLUGIN_PAGE_ACCOUNT_TREE(page)); priv = GNC_PLUGIN_PAGE_ACCOUNT_TREE_GET_PRIVATE(page); + + gnc_tree_view_account_clear_model_cache (GNC_TREE_VIEW_ACCOUNT(priv->tree_view)); gtk_widget_queue_draw (priv->widget); }