Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

POC SplitListNode for gnc:account-accumulate-at-dates #620

Closed

Conversation

christopherlam
Copy link
Contributor

also upgrades gnc:account-get-balances-at-dates. These are used by category-barchart, net-charts, experimental multicolumn reports

note sure if this speeds it up significantly. after all the GList traversal takes place only once per account per report. times in seconds. the best test would involve an account north of 10,000 splits. from guile, ,time (define x (iota 10000000)) to create 10E7 list runs in 0.22s.

old GList: 0.612, 0.553, 0.50 0.50 0.523
new SplitListNode: 0.58 0.55 0.50 0.51 0.52

gjanssens and others added 4 commits December 10, 2019 21:21
This commit adds the following for the guile bindings:
- SplitListNode: a type the same memory layout as a GList node
- gnc-account-get-splits: a convenience function that returns a pointer
  to the SplitListNode that is the first split of the account.

The SplitListNode comes with a number of autogenerated support functions:
- SplitListNode-split-set: assign a new split to the SplitListNode
- SplitListNode-split-get: get the split this SplitListNode refers to
- SplitListNode-next-get: returns a SplitListNode for the next split of the given account
- SplitListNode-prev-get: returns a SplitListNode for the previous split of the given account

There are also setter variants for the last two functions. However I'm not sure
if it's safe to use them. That is it may not be safe to alter the list chaining
pointers directly from within scheme. It's probably better to rely on GList's
convenience functions for this instead.

If this is something useful, it probably needs some more finetuning and
definitely some unit tests.

Note the code uses
typedef struct
{
    Split *split;
    struct SplitListNode *next;
    struct SplitListNode *prev;
} SplitListNode;
static SplitListNode *gnc_account_get_splits (const Account *account)
{
    return (SplitListNode*) xaccAccountGetSplitList (account);
}

rather than

typedef struct splitlist_s SplitListNode;
typedef struct
{
    Split *split;
    SplitListNode *next;
    SplitListNode *prev;
} split_list_node;
static SplitListNode *gnc_account_get_splits (const Account *account)
{
    return (SplitListNode*) xaccAccountGetSplitList (account);
}

The former results in consistent use of the typename SplitListNode in guile
where the latter (while avoiding the need to add 'struct' inside the struct)
would result in a mix of SplitListNode and split-list-node in guile.
@jralls
Copy link
Member

jralls commented Dec 10, 2019

That's why you should profile before spending time optimizing. ;-)

@christopherlam
Copy link
Contributor Author

After all the noise and interruption, I'll return to optimizing for extreme readability and test coverage then 🤷‍♂️ ... we did learn a lot in the process though.

I suspect Guile 2.2's VM optimiser is also much more clever when processing proper lists.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants