CapitalGain calculation is not efficient

Bob B's Avatar

Bob B

08 Apr, 2022 10:15 PM

I'm trying to create a report for every security SELL transaction that includes the resulting capital gain/loss. There are many thousands of these transactions. I've determined that the method InvestUtils.getCostBasisCapGain is the source of the performance loss.

This method uses a list of buyTxns (CapitalGainBuy) which I have cached for each account so I only have to get them once per security account.

I've determined that the getCostBasisCapGain uses a package-private class CostCalculation which gets all the transactions for the security account again and this is the performance bottleneck.

Is there any way around this? I can't make a copy of the CostCalculation class (and pass it the transaction set) because when I decompile it, some internal classes do not decompile.

Is there any way around this? Have I missed some faster way to get the capital gain for the transaction?

Thanks to anyone who can provide suggestions.

  1. 1 Posted by Stuart Beesley ... on 08 Apr, 2022 10:31 PM

    Stuart Beesley (Mr Toolbox)'s Avatar

    .

  2. 2 Posted by Bob B on 09 Apr, 2022 01:15 AM

    Bob B's Avatar

    I figured out how to make a local copy of CostCalculation and I modified it to pass in the cached transaction set. My timings for this function are now down from 15 seconds to 9. I can deal with that but Just for fun I will see if I can get it down any more but I'm off for a few days.:)

  3. 3 Posted by Bob B on 13 Apr, 2022 09:01 PM

    Bob B's Avatar

    I'm digging down into performance of CapitalGain calculations. This is the code (build 4071) from InvestUtil which is called to get a capital gain result. One of the parameters is a list of

    List<CapitalGainBuy> buyTxns
    
    . I can't see where that list is ever used in the method. This is a pretty expensive calculation when done repeatedly. Have I missed something and it is somehow actually used? If it's not used, I could just pass in an empty list and get the same result.
    private static CapitalGainResult getAverageCostCapGain(Account sec, List<CapitalGainBuy> buyTxns, SplitTxn sellTxn) {
            if (sellTxn != null && sellTxn.getValue() <= 0L) {
                return (new CostCalculation(sec, sellTxn.getDateInt())).getGainInfo(sellTxn);
            } else {
                System.err.println("Encountered non-sale txn computing average cost: " + sellTxn);
                return new CapitalGainResult("sell_invalid");
            }
        }
    
  4. 4 Posted by Stuart Beesley ... on 13 Apr, 2022 09:11 PM

    Stuart Beesley (Mr Toolbox)'s Avatar

    Just try it?

    Try passing None/Null (or empty list) as that parameter. What happens?

  5. 5 Posted by Stuart Beesley ... on 13 Apr, 2022 10:49 PM

    Stuart Beesley (Mr Toolbox)'s Avatar

    So it looks like buyTxns is not used within .getAverageCostCapGain() - so try passing null.

    However, .getCostBasisCapGain() also takes buyTxns to pass onto . getAverageCostCapGain() - so again pass null to that.

    As you have 'recreated' these classes you should be able to see how these are called and whether buyTxns is needed higher up the calling chain.

    I would just try it.

  6. 6 Posted by Bob B on 14 Apr, 2022 04:06 PM

    Bob B's Avatar

    It continues to work when passing in a NULL using the standard md functions.

  7. 7 Posted by Stuart Beesley ... on 14 Apr, 2022 04:20 PM

    Stuart Beesley (Mr Toolbox)'s Avatar

    Did it speed up?

  8. 8 Posted by Bob B on 24 Apr, 2022 06:19 PM

    Bob B's Avatar

    Yes by about half. I've decided to use my own capital gain calculations instead of the ones provided in InvestUtil for two reasons:

    1) my calculations complete in about 500 milliseconds (for 1500 sales txns) compared to about 7 seconds using MD and passing in the null value.

    2) I found two bugs in the MD calculations. One where there are multiple sales transactions in a single day and one when there is a stock split between buying and selling. I've fully documented the second (attached if you are interested). I will report this bug.

    NOTE: my code does not support allocating by lots and does not calculate short vs long term gains (not needed in Canada).

  9. System closed this discussion on 24 Jul, 2022 06:20 PM.

Comments are currently closed for this discussion. You can start a new one.

Keyboard shortcuts

Generic

? Show this help
ESC Blurs the current field

Comment Form

r Focus the comment reply box
^ + ↩ Submit the comment

You can use Command ⌘ instead of Control ^ on Mac

Recent Discussions

27 Sep, 2022 02:26 PM
27 Sep, 2022 12:37 PM
27 Sep, 2022 12:23 PM
27 Sep, 2022 10:40 AM
27 Sep, 2022 07:22 AM