← Back to team overview

openerp-expert-accounting team mailing list archive

[Bug 568537] Re: The __compute method of account_account needs optmization to improve Accounting Performance!

 

Hi, 
I use latest stable 5.0.10 from Launchpad with Ubuntu 8.04.
If I create a new database with service profile and UK minimal chart (I tried also with the 2 Swiss chart, it is same) and try to list the accounts from "Financial management / Configuration / Financial Accounting / Financial Accounts / List of Accoutns" I get an error:

[2010-06-02 22:53:24,572] ERROR:web-services:[01]: 
[02]: Environment Information : 
[03]: System : Linux-2.6.24-27-generic-i686-with-debian-lenny-sid
[04]: OS Name : posix
[05]: Distributor ID:	Ubuntu
[06]: Description:	Ubuntu 8.04.4 LTS
[07]: Release:	8.04
[08]: Codename:	hardy
[09]: Operating System Release : 2.6.24-27-generic
[10]: Operating System Version : #1 SMP Wed Mar 24 10:04:52 UTC 2010
[11]: Operating System Architecture : 32bit
[12]: Operating System Locale : en_GB.UTF8
[13]: Python Version : 2.5.2
[14]: OpenERP-Server Version : 5.0.10
[15]: Last revision No. & ID : 
[16]: Traceback (most recent call last):
[17]:   File "/usr/lib/python2.5/site-packages/openerp-server/osv/osv.py", line 58, in wrapper
[18]:     return f(self, dbname, *args, **kwargs)
[19]:   File "/usr/lib/python2.5/site-packages/openerp-server/osv/osv.py", line 119, in execute
[20]:     res = pool.execute_cr(cr, uid, obj, method, *args, **kw)
[21]:   File "/usr/lib/python2.5/site-packages/openerp-server/osv/osv.py", line 111, in execute_cr
[22]:     return getattr(object, method)(cr, uid, *args, **kw)
[23]:   File "/usr/lib/python2.5/site-packages/openerp-server/osv/orm.py", line 2228, in read
[24]:     result = self._read_flat(cr, user, select, fields, context, load)
[25]:   File "/usr/lib/python2.5/site-packages/openerp-server/osv/orm.py", line 2354, in _read_flat
[26]:     res2 = self._columns[val[0]].get(cr, self, ids, val, user, context=context, values=res)
[27]:   File "/usr/lib/python2.5/site-packages/openerp-server/osv/fields.py", line 659, in get
[28]:     res = self._fnct(obj, cr, user, ids, name, self._arg, context)
[29]:   File "/usr/lib/python2.5/site-packages/openerp-server/addons/account/account.py", line 280, in __compute
[30]:     ids2.reverse()
[31]: NameError: global name 'ids2' is not defined

I didn't had such error for previous versions of OpenERP.
After reading this post, I tried to replace:

        # consolidate accounts with direct children
        ids2.reverse()
        brs = list(self.browse(cr, uid, ids2, context=context))
with
        # consolidate accounts with direct children
        ids.reverse()
        brs = list(self.browse(cr, uid, ids, context=context))

Then it works correctly (but slow).

-- 
The __compute method of account_account needs optmization to improve Accounting Performance!
https://bugs.launchpad.net/bugs/568537
You received this bug notification because you are a member of OpenERP
Accounting Experts, which is a direct subscriber.

Status in OpenObject Addons Modules: In Progress
Status in OpenObject Addons 5.0 series: Fix Released
Status in OpenObject Addons trunk series: In Progress

Bug description:
The current __compute method of account_account is wasting a lot of time reordering the list of accounts to compute.

We can make the next block of code about five times faster with a small one-line optimization (just adding a "ids2.reverse()" line):

--------
        brs = list(self.browse(cr, uid, ids2, context=context))
        sums = {}
        while brs:
            current = brs[0]
            can_compute = True
            for child in current.child_id:
                if child.id not in sums:
                    can_compute = False
                    try:
                        brs.insert(0, brs.pop(brs.index(child)))
                    except ValueError:
                        brs.insert(0, child)
            if can_compute:
                brs.pop(0)
                for fn in field_names:
                    sums.setdefault(current.id, {})[fn] = accounts.get(current.id, {}).get(fn, 0.0)
                    if current.child_id:
                        sums[current.id][fn] += sum(sums[child.id][fn] for child in current.child_id)
--------     

That code is computing the value of each account as the sums of the account values plus its children values.

The problem is that the list of the accounts is sorted on the worst posible way! So most of time is wasted reordering that list.

The list of accounts comes from _get_children_and_consol, that returns a list of accounts in the form [parent, child1, child2, child1_of_child2, child2_of_child2, child3].
So the block of code shown above, that always tries to compute the first element of the list, but he won't be able to do it without computing the children accounts first: so it ends up poping accounts from the list and puting them back at the begining of the list in the reverse order... that is, it wastes a lot of time just to reverse the list on the most expensive way!

Adding a ids2.reverse() line before the block of code, will mean that the list of accounts will be in the form [child3, child2_of_child2, child1_of_child2, child2, child1, parent] so no poping&inserting will be necesary!

We have timed that block of code before adding the "ids2.reverse()" and after it:
                  
                      BLOCK                           FULL METHOD
Original:       2.1701090335825           2.370021998875  
Optimized:    0.37584179639849996   0.50867897272100004          (4.65 times faster!)

Note: Average times after several runs, getting the debit and credit of the root account on a database with 1663 accounts and 6930 account move lines.

I think this should be fixed ASAP: It will soothe the currents problems with accounting reports (like the general ledger performance problems reported on bug 514808 and bug 551630).