[Kmymoney-devel] AccountsModel loadInstitution and loadSubaccounts recursiveness

[Alvaro Soliverez]

On Sat, Sep 25, 2010 at 10:39 AM, Cristian Oneţ <onet.cristian at gmail.com> wrote:
> On Sat, Sep 25, 2010 at 4:11 PM, Alvaro Soliverez <asoliverez at kde.org> wrote:
>> On Sat, Sep 25, 2010 at 9:41 AM, Cristian Oneţ <onet.cristian at gmail.com> wrote:
>>> On Tue, Sep 21, 2010 at 6:16 PM, Cristian Oneţ <onet.cristian at gmail.com> wrote:
>>>> On Tue, Sep 21, 2010 at 6:06 PM, Alvaro Soliverez <asoliverez at kde.org> wrote:
>>>>> On Mon, Sep 20, 2010 at 11:15 AM, Cristian Oneţ <onet.cristian at gmail.com> wrote:
>>>>>> On Sun, Sep 19, 2010 at 4:44 PM, Alvaro Soliverez <asoliverez at kde.org> wrote:
>>>>>>> Hello Cristian,
>>>>>> Hi Alvaro,
>>>>>>
>>>>>>> I'm checking the results of the callgrind profiling.
>>>>>>>
>>>>>>> I've found some oddness.
>>>>>>>
>>>>>>> There are two methods in the AccountsModel that get called and incur
>>>>>>> the most time, loadSubAccounts and loadInstitution.
>>>>>>> The AccountModel::load() method calls both, but in turn, the
>>>>>>> loadSubAccounts method calls loadInstitution too.
>>>>>>>
>>>>>>> I haven't looked at it in enough detail to verify if that's correct or
>>>>>>> it can be avoided, because I was looking for something else.
>>>>>>>
>>>>>>> Could you look into it and check whether that's correct?
>>>>>>
>>>>>> Yes it is correct. The AccountModel::loadInstitution function creates
>>>>>> the needed items in the model for the institutions view for each
>>>>>> account (that is why it's called from load sub-accounts also). The
>>>>>> best way to optimize the accounts model (and other parts of the
>>>>>> application) would be to add more fine grained change signaling then
>>>>>> the now existing dataChanged signal. So until we don't know what has
>>>>>> changed we need to sync all the data between the model and the data
>>>>>> storage and that takes time. I still need to take a look at this if we
>>>>>> can gain some speed without doing the extra development of changing
>>>>>> dataChanged but I think the time has come to do just that. Because
>>>>>> that will be needed in the ledger also where for example everything is
>>>>>> reloaded if the status of the transaction is changed.
>>>>>>
>>>>>
>>>>> Is there any expensive calculation on the model, besides retrieving
>>>>> the balance, which should be handled by the balance cache?
>>>>
>>>> Not that I know of. If rebuilding the account tree on each data
>>>> changed is not expensive then it could be that the balance computation
>>>> is and with the cache it can be improved.
>>>
>>> After running some tests it seems that synchronizing the account tree
>>> is the most expensive operation so adding the balance cache wont bring
>>> big speed improvements in this area but a more fine grained data
>>> change signal would. I will start to work on a patch that implements
>>> this.
>>>
>>
>> What does this synchronizing do? Can you handle it with a signal which
>> is only emmited when an account is added/modified/rdeleted?
>
> Synchronizing means to sync the data between the model and the
> underlying storage. If the storage signals that data has changed the
> accounts model must reload (not an actual reload but more of a
> synchronization) to contain the new data. To fix this it would be nice
> to add account added/modified/deleted signals to the MyMoneyFileObject
> similar signals should be added for other types of changes (changes
> that affect the balance or the value for instance). So this is a
> pretty big task and I don't know if it will be worth it.
>

How about adding two sets of signals? One that is emitted whenever
there is an account change, and another that covers transactions?

Also, I've found another issue, which should be probably solved elsewhere.

The accountModel calls balance(accountId).  That, in turn, adds a
default QDate() when calling the engine. That prevents the cache from
being used, even if possible.

So, every call for balance reloads all transactions for that account,
even if it could use the today's balance that is already cached.

Thomas can confirm if my observations are true. If so, I would change,
where possible, the calls to balance() to get today's balance.

Regards,
Alvaro