gnucash maint: [window-autoclear.c] cleanup, optimize, prevent UI lag

Christopher Lam clam at code.gnucash.org
Thu Oct 22 21:12:49 EDT 2020


Updated	 via  https://github.com/Gnucash/gnucash/commit/78c8b03c (commit)
	from  https://github.com/Gnucash/gnucash/commit/4bebfed9 (commit)



commit 78c8b03c5ec033221449d7d8f99ffef07a40af43
Author: Christopher Lam <christopher.lck at gmail.com>
Date:   Wed Oct 14 20:38:47 2020 +0800

    [window-autoclear.c] cleanup, optimize, prevent UI lag
    
    abort:
    1. if too many splits lead to >1,000,000 reachable amounts
    2. if the end_balance is the same as cleared_balance
    
    also:
    - g_free lists properly
    - move knapsack algorithm to gnc-ui-balances.c
    - show newly cleared splits in a new register: "Cleared Transactions"
    - remove unneeded #includes

diff --git a/gnucash/gnome/window-autoclear.c b/gnucash/gnome/window-autoclear.c
index 4bdf14874..294cb7a98 100644
--- a/gnucash/gnome/window-autoclear.c
+++ b/gnucash/gnome/window-autoclear.c
@@ -25,18 +25,14 @@
 #include <gtk/gtk.h>
 #include <glib/gi18n.h>
 
-#include "Scrub.h"
-#include "dialog-account.h"
-#include "dialog-transfer.h"
 #include "dialog-utils.h"
 #include "gnc-amount-edit.h"
-#include "gnc-component-manager.h"
-#include "gnc-date-edit.h"
 #include "gnc-event.h"
 #include "gnc-gnome-utils.h"
 #include "gnc-main-window.h"
 #include "gnc-plugin-page-register.h"
 #include "gnc-ui.h"
+#include "gnc-ui-balances.h"
 #include "window-autoclear.h"
 
 #define WINDOW_AUTOCLEAR_CM_CLASS "window-autoclear"
@@ -95,191 +91,73 @@ gnc_autoclear_make_window_name(Account *account)
     return title;
 }
 
-static gboolean
-ght_gnc_numeric_equal(gconstpointer v1, gconstpointer v2)
+static void
+show_cleared_splits (GList *splits)
 {
-    gnc_numeric n1 = *(gnc_numeric *)v1, n2 = *(gnc_numeric *)v2;
-    return gnc_numeric_equal(n1, n2);
-}
+    GNCLedgerDisplay *ledger;
+    GncPluginPage *page;
+    Query *book_query, *guid_query;
 
-static guint
-ght_gnc_numeric_hash(gconstpointer v1)
-{
-    gnc_numeric n1 = *(gnc_numeric *)v1;
-    gdouble d1 = gnc_numeric_to_double(n1);
-    return g_double_hash (&d1);
-}
-
-typedef struct _sack_foreach_data_t
-{
-    gnc_numeric split_value;
-    GList *reachable_list;
-} *sack_foreach_data_t;
+    book_query = qof_query_create_for (GNC_ID_SPLIT);
+    guid_query = qof_query_create_for (GNC_ID_SPLIT);
+    qof_query_set_book (book_query, gnc_get_current_book ());
 
-static void sack_foreach_func(gpointer key, gpointer value, gpointer user_data)
-{
-    sack_foreach_data_t data = (sack_foreach_data_t)user_data;
-    gnc_numeric thisvalue = *(gnc_numeric *)key;
-
-    gnc_numeric reachable_value = gnc_numeric_add_fixed(thisvalue, data->split_value);
-    data->reachable_list = g_list_prepend
-        (data->reachable_list, g_memdup (&reachable_value, sizeof (gnc_numeric)));
-    PINFO("    Sack: found %s, added %s\n", gnc_numeric_to_string(thisvalue), gnc_numeric_to_string(reachable_value));
+    for (GList *iter = splits; iter; iter = iter->next)
+    {
+        GncGUID guid = xaccSplitReturnGUID (iter->data);
+        xaccQueryAddGUIDMatch (guid_query, &guid, GNC_ID_SPLIT, QOF_QUERY_OR);
+    }
+    book_query = qof_query_merge (book_query, guid_query, QOF_QUERY_AND);
+    ledger = gnc_ledger_display_query (book_query, SEARCH_LEDGER, REG_STYLE_JOURNAL);
+    gnc_ledger_display_refresh (ledger);
+    page = gnc_plugin_page_register_new_ledger (ledger);
+    main_window_update_page_name (page, _("Cleared Transactions"));
+    gnc_main_window_open_page (NULL, page);
+    qof_query_destroy (book_query);
+    qof_query_destroy (guid_query);
 }
 
 void
 gnc_autoclear_window_ok_cb (GtkWidget *widget,
                             AutoClearWindow *data)
 {
-    GList *node, *nc_list = 0, *toclear_list = 0;
+    GList *toclear_list;
     gnc_numeric toclear_value;
-    GHashTable *sack;
+    gchar *errmsg = NULL;
 
-    gtk_label_set_text(data->status_label, _("Searching for splits to clear ..."));
+    g_return_if_fail (widget && data);
 
-    /* Value we have to reach */
     toclear_value = gnc_amount_edit_get_amount(data->end_value);
-    if (gnc_reverse_balance(data->account))
-        toclear_value = gnc_numeric_neg(toclear_value);
-    toclear_value = gnc_numeric_convert(toclear_value, xaccAccountGetCommoditySCU(data->account), GNC_HOW_RND_NEVER);
-
-    /* Extract which splits are not cleared and compute the amount we have to clear */
-    for (node = xaccAccountGetSplitList(data->account); node; node = node->next)
-    {
-        Split *split = (Split *)node->data;
-        char recn;
-        gnc_numeric value;
 
-        recn = xaccSplitGetReconcile (split);
-        value = xaccSplitGetAmount (split);
+    if (gnc_reverse_balance(data->account))
+        toclear_value = gnc_numeric_neg (toclear_value);
 
-        if (recn == NREC)
-            nc_list = g_list_prepend (nc_list, split);
-        else
-            toclear_value = gnc_numeric_sub_fixed(toclear_value, value);
-    }
+    toclear_value = gnc_numeric_convert
+        (toclear_value, xaccAccountGetCommoditySCU(data->account), GNC_HOW_RND_ROUND);
 
-    /* Pretty print information */
-    PINFO("Amount to clear: %s\n", gnc_numeric_to_string(toclear_value));
-    PINFO("Available splits:\n");
-    for (node = nc_list; node; node = node->next)
-    {
-        Split *split = (Split *)node->data;
-        gnc_numeric value = xaccSplitGetAmount (split);
-        PINFO("  %s\n", gnc_numeric_to_string(value));
-    }
+    toclear_list = gnc_account_get_autoclear_splits
+        (data->account, toclear_value, &errmsg);
 
-    /* Run knapsack */
-    /* Entries in the hash table are:
-     *  - key   = amount to which we know how to clear (freed by GHashTable)
-     *  - value = last split we used to clear this amount (not managed by GHashTable)
-     */
-    PINFO("Knapsacking ...\n");
-    sack = g_hash_table_new_full (ght_gnc_numeric_hash, ght_gnc_numeric_equal, g_free, NULL);
-    for (node = nc_list; node; node = node->next)
+    if (errmsg)
     {
-        Split *split = (Split *)node->data;
-        gnc_numeric split_value = xaccSplitGetAmount(split);
-
-        GList *node;
-        struct _sack_foreach_data_t data[1];
-        data->split_value = split_value;
-        data->reachable_list = 0;
-
-        PINFO("  Split value: %s\n", gnc_numeric_to_string(split_value));
-
-        /* For each value in the sack, compute a new reachable value */
-        g_hash_table_foreach (sack, sack_foreach_func, data);
-
-        /* Add the value of the split itself to the reachable_list */
-        data->reachable_list = g_list_prepend
-            (data->reachable_list, g_memdup (&split_value, sizeof (gnc_numeric)));
-
-        /* Add everything to the sack, looking out for duplicates */
-        for (node = data->reachable_list; node; node = node->next)
-        {
-            gnc_numeric *reachable_value = node->data;
-            Split *toinsert_split = split;
-
-            PINFO("    Reachable value: %s ", gnc_numeric_to_string(*reachable_value));
-
-            /* Check if it already exists */
-            if (g_hash_table_lookup_extended(sack, reachable_value, NULL, NULL))
-            {
-                /* If yes, we are in trouble, we reached an amount using two solutions */
-                toinsert_split = NULL;
-                PINFO("dup");
-            }
-            g_hash_table_insert (sack, reachable_value, toinsert_split);
-            PINFO("\n");
-        }
-        g_list_free(data->reachable_list);
+        gtk_label_set_text (data->status_label, errmsg);
+        gnc_amount_edit_set_amount (data->end_value, toclear_value);
+        gtk_editable_select_region (GTK_EDITABLE (data->end_value), 0, -1);
+        g_free (errmsg);
     }
-
-    /* Check solution */
-    PINFO("Rebuilding solution ...\n");
-    while (!gnc_numeric_zero_p(toclear_value))
+    else
     {
-        gpointer psplit = NULL;
-
-        PINFO("  Left to clear: %s\n", gnc_numeric_to_string(toclear_value));
-        if (g_hash_table_lookup_extended(sack, &toclear_value, NULL, &psplit))
-        {
-            if (psplit != NULL)
-            {
-                /* Cast the gpointer to the kind of pointer we actually need */
-                Split *split = (Split *)psplit;
-                toclear_list = g_list_prepend(toclear_list, split);
-                toclear_value = gnc_numeric_sub_fixed(toclear_value,
-                                                      xaccSplitGetAmount(split));
-                PINFO("    Cleared: %s -> %s\n",
-                      gnc_numeric_to_string(xaccSplitGetAmount(split)),
-                      gnc_numeric_to_string(toclear_value));
-            }
-            else
-            {
-                /* We couldn't reconstruct the solution */
-                PINFO("    Solution not unique.\n");
-                gtk_label_set_text(data->status_label, _("Cannot uniquely clear splits. Found multiple possibilities."));
-                gtk_editable_select_region (GTK_EDITABLE (data->end_value), 0, -1);
-                return;
-            }
-        }
-        else
-        {
-            PINFO("    No solution found.\n");
-            gtk_label_set_text(data->status_label, _("The selected amount cannot be cleared."));
-            gtk_editable_select_region (GTK_EDITABLE (data->end_value), 0, -1);
-            return;
-        }
+        xaccAccountBeginEdit (data->account);
+        for (GList *node = toclear_list; node; node = node->next)
+            xaccSplitSetReconcile (node->data, CREC);
+        xaccAccountCommitEdit (data->account);
+        show_cleared_splits (toclear_list);
+        g_list_free (toclear_list);
+
+        /* Close window */
+        gtk_widget_destroy (data->window);
+        g_free (data);
     }
-    g_hash_table_destroy (sack);
-
-    /* Show solution */
-    PINFO("Clearing splits:\n");
-    for (node = toclear_list; node; node = node->next)
-    {
-        Split *split = node->data;
-        char recn;
-        gnc_numeric value;
-
-        recn = xaccSplitGetReconcile (split);
-        value = xaccSplitGetAmount (split);
-
-        PINFO("  %c %s\n", recn, gnc_numeric_to_string(value));
-
-        xaccSplitSetReconcile (split, CREC);
-    }
-    if (toclear_list == 0)
-        PINFO("  None\n");
-
-    /* Free lists */
-    g_list_free(nc_list);
-    g_list_free(toclear_list);
-
-    /* Close window */
-    gtk_widget_destroy(data->window);
-    g_free(data);
 }
 
 void
@@ -291,6 +169,12 @@ gnc_autoclear_window_cancel_cb (GtkWidget *widget,
     g_free(data);
 }
 
+static void clear_status_label_cb (GtkEditable *editable, AutoClearWindow *data)
+{
+    gtk_label_set_text (data->status_label, "");
+}
+
+
 /********************************************************************\
  * autoClearWindow                                                  *
  *   opens up the window to auto-clear an account                   *
@@ -328,6 +212,9 @@ autoClearWindow (GtkWidget *parent, Account *account)
     g_signal_connect(GTK_WIDGET(data->end_value), "activate",
                      G_CALLBACK(gnc_autoclear_window_ok_cb), data);
 
+    g_signal_connect (GTK_WIDGET(data->end_value), "changed",
+                      G_CALLBACK(clear_status_label_cb), data);
+
     box   = GTK_BOX(gtk_builder_get_object (builder, "end_value_box"));
     gtk_box_pack_start(box, GTK_WIDGET(data->end_value), TRUE, TRUE, 0);
 
diff --git a/libgnucash/app-utils/gnc-ui-balances.c b/libgnucash/app-utils/gnc-ui-balances.c
index b02fe76cb..27ce97666 100644
--- a/libgnucash/app-utils/gnc-ui-balances.c
+++ b/libgnucash/app-utils/gnc-ui-balances.c
@@ -31,6 +31,7 @@
 #include <glib.h>
 
 #include "Account.h"
+#include "Split.h"
 #include "gncOwner.h"
 #include "qof.h"
 
@@ -338,3 +339,157 @@ gnc_ui_owner_get_print_report_balance (GncOwner *owner,
     print_info = gnc_commodity_print_info (report_commodity, TRUE);
     return g_strdup (xaccPrintAmount (balance, print_info));
 }
+
+
+/* the following functions are used in window-autoclear: */
+
+#define MAXIMUM_SACK_SIZE 1000000
+
+static gboolean
+ght_gnc_numeric_equal(gconstpointer v1, gconstpointer v2)
+{
+    gnc_numeric n1 = *(gnc_numeric *)v1, n2 = *(gnc_numeric *)v2;
+    return gnc_numeric_equal(n1, n2);
+}
+
+static guint
+ght_gnc_numeric_hash(gconstpointer v1)
+{
+    gnc_numeric n1 = *(gnc_numeric *)v1;
+    gdouble d1 = gnc_numeric_to_double(n1);
+    return g_double_hash (&d1);
+}
+
+typedef struct _sack_foreach_data_t
+{
+    gnc_numeric split_value;
+    GList *reachable_list;
+} *sack_foreach_data_t;
+
+static void sack_foreach_func(gpointer key, gpointer value, gpointer user_data)
+{
+    sack_foreach_data_t data = (sack_foreach_data_t) user_data;
+    gnc_numeric thisvalue = *(gnc_numeric *) key;
+    gnc_numeric reachable_value = gnc_numeric_add_fixed (thisvalue, data->split_value);
+
+    data->reachable_list = g_list_prepend
+        (data->reachable_list, g_memdup (&reachable_value, sizeof (gnc_numeric)));
+}
+
+GList *
+gnc_account_get_autoclear_splits (Account *account, gnc_numeric toclear_value,
+                                  gchar **errmsg)
+{
+    GList *nc_list = NULL, *toclear_list = NULL;
+    GHashTable *sack;
+    gchar *msg = NULL;
+    guint sack_size = 0;
+
+    g_return_val_if_fail (GNC_IS_ACCOUNT (account), NULL);
+
+    sack = g_hash_table_new_full (ght_gnc_numeric_hash, ght_gnc_numeric_equal,
+                                  g_free, NULL);
+
+    /* Extract which splits are not cleared and compute the amount we have to clear */
+    for (GList *node = xaccAccountGetSplitList (account); node; node = node->next)
+    {
+        Split *split = (Split *)node->data;
+
+        if (xaccSplitGetReconcile (split) == NREC)
+            nc_list = g_list_prepend (nc_list, split);
+        else
+            toclear_value = gnc_numeric_sub_fixed
+                (toclear_value, xaccSplitGetAmount (split));
+    }
+
+    if (gnc_numeric_zero_p (toclear_value))
+    {
+        msg = _("Account is already at Auto-Clear Balance.");
+        goto skip_knapsack;
+    }
+
+    /* Run knapsack */
+    /* Entries in the hash table are:
+     *  - key   = amount to which we know how to clear (freed by GHashTable)
+     *  - value = last split we used to clear this amount (not managed by GHashTable)
+     */
+    for (GList *node = nc_list; node; node = node->next)
+    {
+        Split *split = (Split *)node->data;
+        gnc_numeric split_value = xaccSplitGetAmount (split);
+
+        struct _sack_foreach_data_t s_data[1];
+        s_data->split_value = split_value;
+        s_data->reachable_list = NULL;
+
+        /* For each value in the sack, compute a new reachable value */
+        g_hash_table_foreach (sack, sack_foreach_func, s_data);
+
+        /* Add the value of the split itself to the reachable_list */
+        s_data->reachable_list = g_list_prepend
+            (s_data->reachable_list, g_memdup (&split_value, sizeof (gnc_numeric)));
+
+        /* Add everything to the sack, looking out for duplicates */
+        for (GList *s_node = s_data->reachable_list; s_node; s_node = s_node->next)
+        {
+            gnc_numeric *reachable_value = s_node->data;
+
+            /* Check if it already exists */
+            if (g_hash_table_lookup_extended (sack, reachable_value, NULL, NULL))
+            {
+                /* If yes, we are in trouble, we reached an amount
+                   using two solutions */
+                g_hash_table_insert (sack, reachable_value, NULL);
+            }
+            else
+            {
+                g_hash_table_insert (sack, reachable_value, split);
+                sack_size++;
+
+                if (sack_size > MAXIMUM_SACK_SIZE)
+                {
+                    msg = _("Too many uncleared splits");
+                    goto skip_knapsack;
+                }
+            }
+        }
+        g_list_free (s_data->reachable_list);
+    }
+
+    /* Check solution */
+    while (!gnc_numeric_zero_p (toclear_value))
+    {
+        Split *split = NULL;
+
+        if (!g_hash_table_lookup_extended (sack, &toclear_value,
+                                           NULL, (gpointer) &split))
+        {
+            msg = _("The selected amount cannot be cleared.");
+            goto skip_knapsack;
+        }
+
+        if (!split)
+        {
+            msg = _("Cannot uniquely clear splits. Found multiple possibilities.");
+            goto skip_knapsack;
+        }
+
+        toclear_list = g_list_prepend (toclear_list, split);
+        toclear_value = gnc_numeric_sub_fixed (toclear_value,
+                                               xaccSplitGetAmount (split));
+    }
+
+ skip_knapsack:
+    g_hash_table_destroy (sack);
+    g_list_free (nc_list);
+
+    if (msg)
+    {
+        *errmsg = g_strdup (msg);
+        g_list_free (toclear_list);
+        return NULL;
+    }
+
+    *errmsg = NULL;
+    return toclear_list;
+}
diff --git a/libgnucash/app-utils/gnc-ui-balances.h b/libgnucash/app-utils/gnc-ui-balances.h
index 3479f7477..67d5f05e0 100644
--- a/libgnucash/app-utils/gnc-ui-balances.h
+++ b/libgnucash/app-utils/gnc-ui-balances.h
@@ -146,5 +146,12 @@ gchar * gnc_ui_owner_get_print_balance (GncOwner *owner,
 gchar * gnc_ui_owner_get_print_report_balance (GncOwner *owner,
         gboolean *negative);
 
-
+/** Account splits are analysed; attempts to find a unique combination
+ *  of uncleared splits which would set cleared balance to
+ *  toclear_value. If this is not possible, *errmsg will be error
+ *  message. errmsg must be a pointer to a gchar. If it is set, it
+ *  must be freed by the caller.
+ */
+GList * gnc_account_get_autoclear_splits (Account *account, gnc_numeric toclear_value,
+                                          gchar **errmsg);
 #endif /* GNC_UI_BALANCES_H_ */



Summary of changes:
 gnucash/gnome/window-autoclear.c       | 225 ++++++++-------------------------
 libgnucash/app-utils/gnc-ui-balances.c | 155 +++++++++++++++++++++++
 libgnucash/app-utils/gnc-ui-balances.h |   9 +-
 3 files changed, 219 insertions(+), 170 deletions(-)



More information about the gnucash-changes mailing list