Hi Tim,

On Do, 2008-02-07 at 16:37 -0500, Derek Atkins wrote:
---8<---
> Well, it's possible that GetBalanceAsOfDate goes forward in time
> instead of backwards in time, which means that the more transactions
> you have the longer it takes.
---8<---

may you please tell me a few more numbers?

* with current trunk
* trunk + account.patch
* trunk + account.patch + balance.patch

You can apply a patch with 'patch -Np1 < the.patch'.

The first one makes the split list a GQueue internally, the second
rewrites xaccAccountGetBalanceAsOfDate[Full], adds
xaccAccountGetBalanceAsOfDateFromTail and makes the book closing code
use that one.

Derek, in the currently active code we walk through the splits from the
head of the list, searching for the first split (A) with a posted date
>= the given one, and then return the balance of the split preceding A.
In the book closing code you used cacb->cbw->close_date+1, so I wonder
whether you worked around a bug or I fundamentally misunderstand
something here.

My code will take the latest split with the given date as posted, if
there is one.

Thanks,
-- andi5

diff --git a/src/engine/Account.c b/src/engine/Account.c
index 74012b7..c612ce3 100644
--- a/src/engine/Account.c
+++ b/src/engine/Account.c
@@ -138,7 +138,7 @@ typedef struct AccountPrivate
 
     gboolean balance_dirty;     /* balances in splits incorrect */
 
-    GList *splits;              /* list of split pointers */
+    GQueue *splits;             /* queue of split pointers */
     gboolean sort_dirty;        /* sort order of splits is bad */
 
     LotList   *lots;		/* list of lot pointers */
@@ -248,7 +248,7 @@ gnc_account_init(Account* acc)
     priv->starting_reconciled_balance = gnc_numeric_zero();
     priv->balance_dirty = FALSE;
 
-    priv->splits = NULL;
+    priv->splits = g_queue_new();
     priv->sort_dirty = FALSE;
  }
 
@@ -972,7 +972,7 @@ xaccFreeAccount (Account *acc)
   /* NB there shouldn't be any splits by now ... they should 
    * have been all been freed by CommitEdit().  We can remove this
    * check once we know the warning isn't occurring any more. */
-  if (priv->splits) 
+  if (priv->splits->length)
   {
     GList *slist;
     PERR (" instead of calling xaccFreeAccount(), please call \n"
@@ -980,14 +980,14 @@ xaccFreeAccount (Account *acc)
   
     qof_instance_reset_editlevel(acc);
 
-    slist = g_list_copy(priv->splits);
+    slist = g_list_copy(priv->splits->head);
     for (lp = slist; lp; lp = lp->next) {
       Split *s = (Split *) lp->data;
       g_assert(xaccSplitGetAccount(s) == acc);
       xaccSplitDestroy (s);
     }
     g_list_free(slist);
-    g_assert(priv->splits == NULL);
+    g_assert(priv->splits->length == 0);
   }
 
   CACHE_REPLACE(priv->accountName, NULL);
@@ -1084,7 +1084,7 @@ xaccAccountCommitEdit (Account *acc)
     PINFO ("freeing splits for account %p (%s)",
            acc, priv->accountName ? priv->accountName : "(null)");
 
-    slist = g_list_copy(priv->splits);
+    slist = g_list_copy(priv->splits->head);
     for (lp = slist; lp; lp = lp->next)
     {
       Split *s = lp->data;
@@ -1347,8 +1347,8 @@ xaccAccountEqual(const Account *aa, const Account *ab, gboolean check_guids)
   /* no parent; always compare downwards. */
 
   {
-    GList *la = priv_aa->splits;
-    GList *lb = priv_ab->splits;
+    GList *la = priv_aa->splits->head;
+    GList *lb = priv_ab->splits->head;
 
     if ((la && !lb) || (!la && lb))
     {
@@ -1449,7 +1449,7 @@ gnc_account_find_split (Account *acc, Split *s)
     g_return_val_if_fail(GNC_IS_SPLIT(s), FALSE);
 
     priv = GET_PRIVATE(acc);
-    node = g_list_find(priv->splits, s);
+    node = g_queue_find(priv->splits, s);
     return node ? TRUE : FALSE;
 }
 
@@ -1463,15 +1463,15 @@ gnc_account_insert_split (Account *acc, Split *s)
     g_return_val_if_fail(GNC_IS_SPLIT(s), FALSE);
 
     priv = GET_PRIVATE(acc);
-    node = g_list_find(priv->splits, s);
+    node = g_queue_find(priv->splits, s);
     if (node)
 	return FALSE;
 
     if (qof_instance_get_editlevel(acc) == 0) {
-	priv->splits = g_list_insert_sorted(priv->splits, s,
-					   (GCompareFunc)xaccSplitOrder);
+	g_queue_insert_sorted(priv->splits, s,
+			      (GCompareDataFunc)xaccSplitOrderData, NULL);
     } else {
-	priv->splits = g_list_prepend(priv->splits, s);
+	g_queue_push_head(priv->splits, s);
 	priv->sort_dirty = TRUE;
     }
 
@@ -1496,11 +1496,11 @@ gnc_account_remove_split (Account *acc, Split *s)
     g_return_val_if_fail(GNC_IS_SPLIT(s), FALSE);
 
     priv = GET_PRIVATE(acc);
-    node = g_list_find(priv->splits, s);
+    node = g_queue_find(priv->splits, s);
     if (NULL == node)
 	return FALSE;
 
-    priv->splits = g_list_delete_link(priv->splits, node);
+    g_queue_delete_link(priv->splits, node);
     //FIXME: find better event type
     qof_event_gen(&acc->inst, QOF_EVENT_MODIFY, NULL);
     // And send the account-based event, too
@@ -1521,7 +1521,7 @@ xaccAccountSortSplits (Account *acc, gboolean force)
     priv = GET_PRIVATE(acc);
     if (!priv->sort_dirty || (!force && qof_instance_get_editlevel(acc) > 0))
         return;
-    priv->splits = g_list_sort(priv->splits, (GCompareFunc)xaccSplitOrder);
+    g_queue_sort(priv->splits, (GCompareDataFunc)xaccSplitOrderData, NULL);
     priv->sort_dirty = FALSE;
     priv->balance_dirty = TRUE;
 }
@@ -1726,7 +1726,7 @@ xaccAccountMoveAllSplits (Account *accfrom, Account *accto)
   /* optimizations */
   from_priv = GET_PRIVATE(accfrom);
   to_priv = GET_PRIVATE(accto);
-  if (!from_priv->splits || accfrom == accto)
+  if (!from_priv->splits->length || accfrom == accto)
       return;
 
   /* check for book mix-up */
@@ -1736,7 +1736,7 @@ xaccAccountMoveAllSplits (Account *accfrom, Account *accto)
   xaccAccountBeginEdit(accfrom);
   xaccAccountBeginEdit(accto);
   /* Begin editing both accounts and all transactions in accfrom. */
-  g_list_foreach(from_priv->splits, (GFunc)xaccPreSplitMove, NULL);
+  g_queue_foreach(from_priv->splits, (GFunc)xaccPreSplitMove, NULL);
 
   /* Concatenate accfrom's lists of splits and lots to accto's lists. */
   //to_priv->splits = g_list_concat(to_priv->splits, from_priv->splits);
@@ -1753,10 +1753,10 @@ xaccAccountMoveAllSplits (Account *accfrom, Account *accto)
    * Convert each split's amount to accto's commodity.
    * Commit to editing each transaction.
    */
-  g_list_foreach(from_priv->splits, (GFunc)xaccPostSplitMove, (gpointer)accto);
+  g_queue_foreach(from_priv->splits, (GFunc)xaccPostSplitMove, (gpointer)accto);
 
   /* Finally empty accfrom. */
-  g_assert(from_priv->splits == NULL);
+  g_assert(from_priv->splits->length == 0);
   g_assert(from_priv->lots == NULL);
   xaccAccountCommitEdit(accfrom);
   xaccAccountCommitEdit(accto);
@@ -1817,7 +1817,7 @@ xaccAccountRecomputeBalance (Account * acc)
 
   PINFO ("acct=%s starting baln=%" G_GINT64_FORMAT "/%" G_GINT64_FORMAT,
 	 priv->accountName, balance.num, balance.denom);
-  for(lp = priv->splits; lp; lp = lp->next) 
+  for(lp = priv->splits->head; lp; lp = lp->next)
   {
     Split *split = (Split *) lp->data;
     gnc_numeric amt = xaccSplitGetAmount (split);
@@ -2075,7 +2075,7 @@ xaccAccountSetCommodity (Account * acc, gnc_commodity * com)
   priv->non_standard_scu = FALSE;
 
   /* iterate over splits */
-  for (lp = priv->splits; lp; lp = lp->next)
+  for (lp = priv->splits->head; lp; lp = lp->next)
   {
       Split *s = (Split *) lp->data;
       Transaction *trans = xaccSplitGetParent (s);
@@ -2869,7 +2869,7 @@ xaccAccountGetProjectedMinimumBalance (const Account *acc)
 
   priv = GET_PRIVATE(acc);
   today = gnc_timet_get_today_end();
-  for (node = g_list_last(priv->splits); node; node = node->prev)
+  for (node = priv->splits->tail; node; node = node->prev)
   {
     Split *split = node->data;
 
@@ -2930,7 +2930,7 @@ xaccAccountGetBalanceAsOfDate (Account *acc, time_t date)
   ts.tv_sec = date;
   ts.tv_nsec = 0;
 
-  lp = priv->splits;
+  lp = priv->splits->head;
   while( lp && !found )
   {
     xaccTransGetDatePostedTS( xaccSplitGetParent( (Split *)lp->data ),
@@ -2980,7 +2980,7 @@ xaccAccountGetPresentBalance (const Account *acc)
 
   priv = GET_PRIVATE(acc);
   today = gnc_timet_get_today_end();
-  for (node = g_list_last(priv->splits); node; node = node->prev)
+  for (node = priv->splits->tail; node; node = node->prev)
   {
     Split *split = node->data;
 
@@ -3305,7 +3305,7 @@ xaccAccountGetSplitList (const Account *acc)
 {
     g_return_val_if_fail(GNC_IS_ACCOUNT(acc), NULL);
     xaccAccountSortSplits((Account*)acc, FALSE);  // normally a noop
-    return GET_PRIVATE(acc)->splits;
+    return GET_PRIVATE(acc)->splits->head;
 }
 
 LotList *
@@ -4113,7 +4113,7 @@ finder_help_function(const Account *acc, const char *description,
    * list is in date order, and the most recent matches should be 
    * returned!?  */
   priv = GET_PRIVATE(acc);
-  for (slp = g_list_last(priv->splits); slp; slp = slp->prev) {
+  for (slp = priv->splits->tail; slp; slp = slp->prev) {
     Split *lsplit = slp->data;
     Transaction *ltrans = xaccSplitGetParent(lsplit);
 
@@ -4279,8 +4279,8 @@ gnc_account_merge_children (Account *parent)
       gnc_account_merge_children (acc_a);
 
       /* consolidate transactions */
-      while (priv_b->splits)
-	xaccSplitSetAccount (priv_b->splits->data, acc_a);
+      while (priv_b->splits->head)
+	xaccSplitSetAccount (priv_b->splits->head->data, acc_a);
 
       /* move back one before removal. next iteration around the loop
        * will get the node after node_b */
@@ -4322,7 +4322,7 @@ xaccAccountBeginStagedTransactionTraversals (const Account *account)
     if (!account)
         return;
     priv = GET_PRIVATE(account);
-    xaccSplitsBeginStagedTransactionTraversals(priv->splits);
+    xaccSplitsBeginStagedTransactionTraversals(priv->splits->head);
 }
 
 gboolean
@@ -4356,7 +4356,7 @@ static void do_one_split (Split *s, gpointer data)
 static void do_one_account (Account *account, gpointer data)
 {
     AccountPrivate *priv = GET_PRIVATE(account);
-    g_list_foreach(priv->splits, (GFunc)do_one_split, NULL);
+    g_queue_foreach(priv->splits, (GFunc)do_one_split, NULL);
 }
 
 /* Replacement for xaccGroupBeginStagedTransactionTraversals */
@@ -4385,7 +4385,7 @@ xaccAccountStagedTransactionTraversal (const Account *acc,
   if (!acc) return 0;
 
   priv = GET_PRIVATE(acc);
-  for(split_p = priv->splits; split_p; split_p = g_list_next(split_p)) {
+  for(split_p = priv->splits->head; split_p; split_p = split_p->next) {
     s = split_p->data;
     trans = s->parent;   
     if (trans && (trans->marker < stage)) {
@@ -4423,7 +4423,7 @@ gnc_account_tree_staged_transaction_traversal (const Account *acc,
   }
 
   /* Now this account */
-  for(split_p = priv->splits; split_p; split_p = g_list_next(split_p)) {
+  for(split_p = priv->splits->head; split_p; split_p = split_p->next) {
     s = split_p->data;
     trans = s->parent;   
     if (trans && (trans->marker < stage)) {
diff --git a/src/engine/Split.c b/src/engine/Split.c
index f491252..37e0e1f 100644
--- a/src/engine/Split.c
+++ b/src/engine/Split.c
@@ -1166,6 +1166,11 @@ xaccSplitOrder (const Split *sa, const Split *sb)
   return 0;
 }
 
+gint xaccSplitOrderData (const Split *sa, const Split *sb, gpointer unused)
+{
+  return xaccSplitOrder (sa, sb);
+}
+
 gint
 xaccSplitOrderDateOnly (const Split *sa, const Split *sb)
 {
@@ -1189,6 +1194,12 @@ xaccSplitOrderDateOnly (const Split *sa, const Split *sb)
   return -1;
 }
 
+gint xaccSplitOrderDateOnlyData (const Split *sa, const Split *sb,
+                                 gpointer unused)
+{
+  return xaccSplitOrderDateOnly (sa, sb);
+}
+
 
 static gboolean
 get_corr_account_split(const Split *sa, const Split **retval)
diff --git a/src/engine/Split.h b/src/engine/Split.h
index a079b71..29958f7 100644
--- a/src/engine/Split.h
+++ b/src/engine/Split.h
@@ -379,6 +379,12 @@ void xaccSplitMakeStockSplit(Split *s);
 gint xaccSplitOrder (const Split *sa, const Split *sb);
 gint xaccSplitOrderDateOnly (const Split *sa, const Split *sb);
 
+/**
+ * xaccSplitOrderData and xaccSplitOrderDateOnlyData work like the versions
+ * without data, but can be used as GCompareDataFuncs.
+ */
+gint xaccSplitOrderData (const Split *sa, const Split *sb, const gpointer unused);
+gint xaccSplitOrderDateOnlyData (const Split *sa, const Split *sb, const gpointer unused);
 
 /*
  * These functions compare two splits by different criteria.   
diff --git a/src/engine/Account.c b/src/engine/Account.c
index c612ce3..a5d63a3 100644
--- a/src/engine/Account.c
+++ b/src/engine/Account.c
@@ -2892,8 +2892,8 @@ xaccAccountGetProjectedMinimumBalance (const Account *acc)
 /********************************************************************\
 \********************************************************************/
 
-gnc_numeric
-xaccAccountGetBalanceAsOfDate (Account *acc, time_t date)
+static gnc_numeric
+xaccAccountGetBalanceAsOfDateFull (Account *acc, time_t date, gboolean from_tail)
 {
   /* Ideally this could use xaccAccountForEachSplit, but
    * it doesn't exist yet and I'm uncertain of exactly how
@@ -2913,7 +2913,6 @@ xaccAccountGetBalanceAsOfDate (Account *acc, time_t date)
   xaccAccountRecomputeBalance (acc); /* just in case, normally a noop */
 
   priv = GET_PRIVATE(acc);
-  balance = priv->balance;
 
   /* Since transaction post times are stored as a Timespec,
    * convert date into a Timespec as well rather than converting
@@ -2930,35 +2929,53 @@ xaccAccountGetBalanceAsOfDate (Account *acc, time_t date)
   ts.tv_sec = date;
   ts.tv_nsec = 0;
 
-  lp = priv->splits->head;
-  while( lp && !found )
-  {
-    xaccTransGetDatePostedTS( xaccSplitGetParent( (Split *)lp->data ),
-                              &trans_ts );
-    if( timespec_cmp( &trans_ts, &ts ) >= 0 )
-      found = TRUE;
-    else
-      lp = lp->next;
-  }
+  if (from_tail) {
 
-  if( lp ) {
-    if ( lp->prev ) {
-      /* Since lp is now pointing to a split which was past the reconcile
-       * date, get the running balance of the previous split.
-       */
-      balance = xaccSplitGetBalance( (Split *)lp->prev->data );
-    }		
-    else {
-      /* AsOf date must be before any entries, return zero. */
-      balance = gnc_numeric_zero();
+    /* Start from tail and find first split <= ts */
+    for (lp = priv->splits->tail; lp && !found; lp = lp->prev) {
+      Split *s = (Split*) lp->data;
+      xaccTransGetDatePostedTS(xaccSplitGetParent(s), &trans_ts);
+      if (timespec_cmp(&trans_ts, &ts) <= 0) {
+
+        /* Take its balance */
+        return xaccSplitGetBalance(s);
+      }
     }
+
+    /* Otherwise there are no splits posted before the given date */
+    return gnc_numeric_zero();
+
+  } else {
+
+    /* Start from head and find first split > ts */
+    for (lp = priv->splits->head; lp && !found; lp = lp->next) {
+      Split *sn = (Split*) lp->data;
+      xaccTransGetDatePostedTS(xaccSplitGetParent(sn), &trans_ts);
+      if (timespec_cmp(&trans_ts, &ts) > 0) {
+
+        /* Take the preceeding balance or 0 */
+        Split *s = lp->prev ? (Split*) lp->prev->data : NULL;
+        return xaccSplitGetBalance(s);
+      }
+    }
+
+    /* Otherwise there are no splits posted after the given date,
+     * so the latest account balance should be good enough.
+     */
+    return priv->balance;
   }
+}
 
-  /* Otherwise there were no splits posted after the given date,
-   * so the latest account balance should be good enough.
-   */
+gnc_numeric
+xaccAccountGetBalanceAsOfDate (Account *acc, time_t date)
+{
+  return xaccAccountGetBalanceAsOfDateFull(acc, date, FALSE);
+}
 
-  return( balance );
+gnc_numeric
+xaccAccountGetBalanceAsOfDateFromTail (Account *acc, time_t date)
+{
+  return xaccAccountGetBalanceAsOfDateFull(acc, date, TRUE);
 }
 
 /*
diff --git a/src/engine/Account.h b/src/engine/Account.h
index c0c499e..8b36a0b 100644
--- a/src/engine/Account.h
+++ b/src/engine/Account.h
@@ -523,8 +523,12 @@ gnc_numeric xaccAccountGetReconciledBalance (const Account *account);
 gnc_numeric xaccAccountGetPresentBalance (const Account *account);
 gnc_numeric xaccAccountGetProjectedMinimumBalance (const Account *account);
 /** Get the balance of the account as of the date specified */
-gnc_numeric xaccAccountGetBalanceAsOfDate (Account *account, 
+gnc_numeric xaccAccountGetBalanceAsOfDate (Account *account,
                                            time_t date);
+/** Get the balance of the account as of the date specified, but start
+    at the latest split */
+gnc_numeric xaccAccountGetBalanceAsOfDateFromTail (Account *account,
+                                                   time_t date);
 
 /* These two functions convert a given balance from one commodity to
    another.  The account argument is only used to get the Book, and
diff --git a/src/gnome-utils/dialog-book-close.c b/src/gnome-utils/dialog-book-close.c
index 37adadb..f199eca 100644
--- a/src/gnome-utils/dialog-book-close.c
+++ b/src/gnome-utils/dialog-book-close.c
@@ -132,7 +132,7 @@ static void close_accounts_cb(Account *a, gpointer data)
   if (cacb->acct_type != xaccAccountGetType(a))
     return;
 
-  bal = xaccAccountGetBalanceAsOfDate(a, cacb->cbw->close_date+1);
+  bal = xaccAccountGetBalanceAsOfDateFromTail(a, cacb->cbw->close_date);
   if (gnc_numeric_zero_p(bal))
     return;
 

Attachment: signature.asc
Description: This is a digitally signed message part

_______________________________________________
gnucash-devel mailing list
gnucash-devel@gnucash.org
https://lists.gnucash.org/mailman/listinfo/gnucash-devel

Reply via email to