A Diumenge, 18 de novembre de 2012 00:34:17, Albert Cervera i Areny va
escriure:
> A Dissabte, 17 de novembre de 2012 23:29:08, Cédric Krier va escriure:
> > On 17/11/12 20:48 +0100, Albert Cervera i Areny wrote:
> > > A Diumenge, 4 de març de 2012 03:49:24, [email protected] va
>
> escriure:
> > > > Reviewers: ,
> > > >
> > > >
> > > >
> > > > Please review this at http://codereview.tryton.org/266002/
> > > >
> > > > Affected files:
> > > > M trytond/model/modelsql.py
> > > > M trytond/model/modelstorage.py
> > > > M trytond/tests/test_tryton.py
> > >
> > > I recently updated this patch to tip. It'd be great if somebody could
> > > review it. Most of the code does not require in-depth knowledge of
> > > trytond core.
> >
> > We talked at the TUL that we should stabilize the API or at least make
> > backward compatible changes.
> > So I'm wondering if we should not do it for this patch.
> >
> > Some options are possible:
> > - allow create to work with a list or a dict. I don't like this
> >
> > solution because we will have the same issue as for
> > write/delete/browse
> >
> > - rename create into create_list and add a method create that
> >
> > convert the dict into a list.
> >
> > Or we decide that the change worth the non-backward compatible API
> > change.
>
> I know API changes are a pain but I really think this one is worth it for
> performance reasons. Let me show you the results of creating a thousand
> parties and a thousand products (with their templates) in my machine with
> and without the patch (average of three runs) with sqlite:
>
> Parties:
>
> tip = 4.57 (42% slower than patched version)
> patched = 3.22 (70% of tip)
>
> Products:
>
> tip = 32.18 (45% slower than patched version)
> patched = 22.25 (69% of tip)
>
> I also tried creating records one by one with the patched version and no
> performance loss was detected compared with tip.
I did some more testing and I wanted to share my findings:
The party test I did created records like this:
{
'active': True,
'categories': [],
'lang': False,
'name': str(x),
'code': str(x),
'addresses': [],
}
Addresses was important to set because otherwise party's default value would
create a new address and I wanted to check performance creating a single
record. Something similar happened with lang in which case it's default value
checks for party.configuration singleton.
And after cedk's comments that the bottleneck was not in the INSERT operation,
I decided to try to find out if that was actually the case. I realized there
was something strange in my results until I found out that actually INSERT was
not taking all that much time but still there was a huge difference between the
time spent in the create operation between sqlite and PostgreSQL backends.
I found out that the problem was in the defaults_get() operation because I had
not provided the 'code_readonly' field. The field is of type Function so the
first issue is that I think there's no need for the defaults_get() operation to
return this value in the case of a create().
Even if the code_readonly was read the performance difference between providing
this value and not doing so in PostgreSQL was still huge. Create took 14.5
seconds if code_readonly was not provided while it took only 0.5 seconds if
code_readoly = False was provided!!! That is it takes only 0.5 seconds to
create 1000 records which looks pretty good to me. The question is why the
cache system didn't work for the first case
'code_readonly' default function simply reads information from
party.configuration but why such a huge difference? Why did the cache not work
there? The reason is simple: when a singleton has no record, its read
operation executes a default_get() which in most cases means reading all its
fields which tend to be Property's and that operation is relatively slow.
Enough to make a 0.5s create() become 14.5s!
I think we may be hit by this problem in other places.
The question about wheather the more complex implementation that executes a
single insert for all records is worth it or not may be answered by the
following numbers.
With PostgreSQL backend:
Creating a 1000 records at once:
Multiple inserts: 1.45 seconds (77% speedup over current implementation)
Single insert: 0.51 seconds (65% speedup over multiple inserts, 92% speedup
over current implementation)
Creating a 1000 records one by one (1000 create calls):
Multiple inserts: 6.29 seconds
Single insert: 6.38 seconds (1.4% slowdown over multiple inserts, 1.7%
slowdown over current implementation)
Current implementation: 6.27 seconds
With SQLite backend:
Creating a 1000 records at once:
Simple: 0.45 seconds (73% speedup over current implementation)
Complex: 0.42 seconds (6% speedup over multiple inserts, 74% speedup over
current implementation)
Creating a 1000 records one by one (1000 create calls):
Simple: 1.65 seconds
Complex: 1.66 seconds (0.6% slowdown over multiple inserts, 1.2% slowdown over
current implementation)
Current implementation: 1.64 seconds
Some conclusions:
- With PostgreSQL, speedup is even higher than my first numbers if no strange
default_get() issues are involved.
- With PostgreSQL, even if the main bottle neck with current implementation is
addressed by the multiple inserts (simpler) implementation, there's still room
for improvement using a single insert (92% instead of 77% speedup).
- We should find a solution for singletons when there's no record so we can
still use the cache.
--
Albert Cervera i Areny
http://www.NaN-tic.com
Tel: +34 93 553 18 03
http://twitter.com/albertnan
http://www.nan-tic.com/blog
--
--
[email protected] mailing list
