From 8628ca87150835d786909ecc902f3191ea880ecb Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Wed, 31 May 2023 00:44:05 +0800 Subject: [PATCH 1/2] [Scrub.c] refactor xaccAccountTreeScrubImbalance The scrubbing routines are transaction oriented. Instead of xaccAccountTreeScrubImbalance calling xaccAccountScrubImbalance for each descendant, refactor so that the transaction list is generated only once. The scrubbing is faster and progress bar is more accurate. --- libgnucash/engine/Scrub.c | 106 +++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 53 deletions(-) diff --git a/libgnucash/engine/Scrub.c b/libgnucash/engine/Scrub.c index f1cd86f75c..d603a2b2ae 100644 --- a/libgnucash/engine/Scrub.c +++ b/libgnucash/engine/Scrub.c @@ -43,6 +43,7 @@ #include #include #include +#include #include "Account.h" #include "AccountP.h" @@ -52,6 +53,8 @@ #include "gnc-commodity.h" #include "qofinstance-p.h" #include "gnc-session.h" +#include "qofquery.h" +#include "Query.h" #undef G_LOG_DOMAIN #define G_LOG_DOMAIN "gnc.engine.scrub" @@ -88,6 +91,22 @@ gnc_get_ongoing_scrub (void) /* ================================================================ */ +static GList* +get_all_transactions (Account *account, bool descendants) +{ + GList *accounts = descendants ? gnc_account_get_descendants (account) : NULL; + accounts = g_list_prepend (accounts, account); + QofQuery *q = qof_query_create_for (GNC_ID_TRANS); + QofBook *book = qof_session_get_book (gnc_get_current_session ()); + qof_query_set_book (q, book); + xaccQueryAddAccountMatch (q, accounts, QOF_GUID_MATCH_ANY, QOF_QUERY_AND); + GList *transactions = g_list_copy (qof_query_run (q)); + qof_query_destroy (q); + return transactions; +} + +/* ================================================================ */ + void xaccAccountTreeScrubOrphans (Account *acc, QofPercentageFunc percentagefunc) { @@ -331,78 +350,59 @@ xaccSplitScrub (Split *split) /* ================================================================ */ -void -xaccAccountTreeScrubImbalance (Account *acc, QofPercentageFunc percentagefunc) -{ - if (!acc) return; - - if (abort_now) - (percentagefunc)(NULL, -1.0); - scrub_depth++; - xaccAccountScrubImbalance (acc, percentagefunc); - gnc_account_foreach_descendant(acc, - (AccountCb)xaccAccountScrubImbalance, percentagefunc); - scrub_depth--; -} - -void -xaccAccountScrubImbalance (Account *acc, QofPercentageFunc percentagefunc) +static void +AccountScrubImbalance (Account *acc, bool descendants, + QofPercentageFunc percentagefunc) { - GList *node, *splits; - const char *str; - const char *message = _( "Looking for imbalances in account %s: %u of %u"); - gint split_count = 0, curr_split_no = 0; + const char *message = _( "Looking for imbalances in transaction date %s: %u of %u"); if (!acc) return; - /* If it's a trading account and an imbalanced transaction is - * found the trading splits will be replaced, invalidating the - * split list in mid-traversal, see - * https://bugs.gnucash.org/show_bug.cgi?id=798346. Also the - * transactions will get scrubbed at least twice from their "real" - * accounts anyway so doing so from the trading accounts is wasted - * effort. - */ - if (xaccAccountGetType(acc) == ACCT_TYPE_TRADING) - return; - scrub_depth++; - - str = xaccAccountGetName(acc); - str = str ? str : "(null)"; - PINFO ("Looking for imbalances in account %s\n", str); + QofBook *book = qof_session_get_book (gnc_get_current_session ()); + Account *root = gnc_book_get_root_account (book); + GList *transactions = get_all_transactions (acc, descendants); + guint count = g_list_length (transactions), curr_trans = 0; - splits = xaccAccountGetSplitList(acc); - split_count = g_list_length (splits); - for (node = splits; node; node = node->next) + scrub_depth++; + for (GList *node = transactions; node; node = node->next, curr_trans++) { - Split *split = node->data; - Transaction *trans = xaccSplitGetParent(split); + Transaction *trans = node->data; if (abort_now) break; - PINFO("Start processing split %d of %d", - curr_split_no + 1, split_count); + PINFO("Start processing transaction %d of %d", curr_trans + 1, count); - if (curr_split_no % 10 == 0) + if (curr_trans % 10 == 0) { - char *progress_msg = g_strdup_printf (message, str, curr_split_no, split_count); - (percentagefunc)(progress_msg, (100 * curr_split_no) / split_count); + char *date = qof_print_date (xaccTransGetDate (trans)); + char *progress_msg = g_strdup_printf (message, date, curr_trans, count); + (percentagefunc)(progress_msg, (100 * curr_trans) / count); g_free (progress_msg); + g_free (date); } - TransScrubOrphansFast (xaccSplitGetParent (split), - gnc_account_get_root (acc)); - + TransScrubOrphansFast (trans, root); xaccTransScrubCurrency(trans); + xaccTransScrubImbalance (trans, root, NULL); - xaccTransScrubImbalance (trans, gnc_account_get_root (acc), NULL); - - PINFO("Finished processing split %d of %d", - curr_split_no + 1, split_count); - curr_split_no++; + PINFO("Finished processing transaction %d of %d", curr_trans + 1, count); } (percentagefunc)(NULL, -1.0); scrub_depth--; + + g_list_free (transactions); +} + +void +xaccAccountTreeScrubImbalance (Account *acc, QofPercentageFunc percentagefunc) +{ + AccountScrubImbalance (acc, true, percentagefunc); +} + +void +xaccAccountScrubImbalance (Account *acc, QofPercentageFunc percentagefunc) +{ + AccountScrubImbalance (acc, false, percentagefunc); } static Split * From 9047156017ea20e8e6458d73490c313739a6d2f9 Mon Sep 17 00:00:00 2001 From: Christopher Lam Date: Wed, 31 May 2023 08:23:34 +0800 Subject: [PATCH 2/2] [Scrub.c] refactor xaccAccountTreeScrubOrphans --- libgnucash/engine/Scrub.c | 84 ++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 50 deletions(-) diff --git a/libgnucash/engine/Scrub.c b/libgnucash/engine/Scrub.c index d603a2b2ae..8ac3d21b58 100644 --- a/libgnucash/engine/Scrub.c +++ b/libgnucash/engine/Scrub.c @@ -107,47 +107,27 @@ get_all_transactions (Account *account, bool descendants) /* ================================================================ */ -void -xaccAccountTreeScrubOrphans (Account *acc, QofPercentageFunc percentagefunc) -{ - if (!acc) return; - - if (abort_now) - (percentagefunc)(NULL, -1.0); - - scrub_depth ++; - xaccAccountScrubOrphans (acc, percentagefunc); - gnc_account_foreach_descendant(acc, - (AccountCb)xaccAccountScrubOrphans, percentagefunc); - scrub_depth--; -} - static void TransScrubOrphansFast (Transaction *trans, Account *root) { - GList *node; - gchar *accname; + g_return_if_fail (trans && trans->common_currency && root); - if (!trans) return; - g_return_if_fail (root); - g_return_if_fail (trans->common_currency); - - for (node = trans->splits; node; node = node->next) + for (GList *node = trans->splits; node; node = node->next) { Split *split = node->data; - Account *orph; if (abort_now) break; if (split->acc) continue; DEBUG ("Found an orphan\n"); - accname = g_strconcat (_("Orphan"), "-", - gnc_commodity_get_mnemonic (trans->common_currency), - NULL); - orph = xaccScrubUtilityGetOrMakeAccount (root, trans->common_currency, - accname, ACCT_TYPE_BANK, - FALSE, TRUE); + gchar *accname = g_strconcat + (_("Orphan"), "-", gnc_commodity_get_mnemonic (trans->common_currency), + NULL); + + Account *orph = xaccScrubUtilityGetOrMakeAccount + (root, trans->common_currency, accname, ACCT_TYPE_BANK, false, true); + g_free (accname); if (!orph) continue; @@ -155,43 +135,47 @@ TransScrubOrphansFast (Transaction *trans, Account *root) } } -void -xaccAccountScrubOrphans (Account *acc, QofPercentageFunc percentagefunc) +static void +AccountScrubOrphans (Account *acc, bool descendants, QofPercentageFunc percentagefunc) { - GList *node, *splits; - const char *str; - const char *message = _( "Looking for orphans in account %s: %u of %u"); - guint total_splits = 0; - guint current_split = 0; - if (!acc) return; scrub_depth++; - str = xaccAccountGetName (acc); - str = str ? str : "(null)"; - PINFO ("Looking for orphans in account %s\n", str); - splits = xaccAccountGetSplitList(acc); - total_splits = g_list_length (splits); + GList *transactions = get_all_transactions (acc, descendants); + gint total_trans = g_list_length (transactions); + const char *message = _( "Looking for orphans in transaction: %u of %u"); + guint current_trans = 0; - for (node = splits; node; node = node->next) + for (GList *node = transactions; node; current_trans++, node = node->next) { - Split *split = node->data; - if (current_split % 10 == 0) + Transaction *trans = node->data; + if (current_trans % 10 == 0) { - char *progress_msg = g_strdup_printf (message, str, current_split, total_splits); - (percentagefunc)(progress_msg, (100 * current_split) / total_splits); + char *progress_msg = g_strdup_printf (message, current_trans, total_trans); + (percentagefunc)(progress_msg, (100 * current_trans) / total_trans); g_free (progress_msg); if (abort_now) break; } - TransScrubOrphansFast (xaccSplitGetParent (split), - gnc_account_get_root (acc)); - current_split++; + TransScrubOrphansFast (trans, gnc_account_get_root (acc)); } (percentagefunc)(NULL, -1.0); scrub_depth--; + + g_list_free (transactions); } +void +xaccAccountScrubOrphans (Account *acc, QofPercentageFunc percentagefunc) +{ + AccountScrubOrphans (acc, false, percentagefunc); +} + +void +xaccAccountTreeScrubOrphans (Account *acc, QofPercentageFunc percentagefunc) +{ + AccountScrubOrphans (acc, true, percentagefunc); +} void xaccTransScrubOrphans (Transaction *trans)