Tryton - Issues

 

Issue9240

Title Reduce save() overhead
Priority feature Status resolved
Superseder Nosy List albertca, ced, jcavallo, pokoli, reviewbot, roundup-bot, yangoon
Type performance Components trytond
Assigned To albertca Keywords review
Reviews 319251002, 295421002
View: 319251002, 295421002

Created on 2020-04-19.11:30:46 by albertca, last changed by roundup-bot.

Messages
New changeset e1834b9e3178 by Cédric Krier in branch 'default':
Use different hash for records without id
https://hg.tryton.org/tryton-env/rev/e1834b9e3178
New changeset c091727c0892 by Cédric Krier in branch 'default':
Use different hash for records without id
https://hg.tryton.org/trytond/rev/c091727c0892
msg57934 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-05-10.14:07:49
It will be good to have feedback if review295421002 improve the saving for your cases.
New review295421002 at https://codereview.tryton.org/295421002/#ps325181002
msg57646 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-04-26.19:48:52
I think review295421002 improve the performance the same way but being less intrusive and generic for all similar cases.
msg57319 (view) Author: [hidden] (jcavallo) Date: 2020-04-20.09:05:37
I forgot we actually did something like this in our modified version of
trytond:

https://github.com/coopengo/trytond/blob/master/trytond/model/modelstorage.py#L1664

In our case we chose to always use "id(instance)", with no problems so far (and the patch has been applied for a while)
msg57300 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-04-19.15:14:12
I do not think the problem is the usage of dictionary but the __hash__ method which returns the same hash for unsaved instances. I think in such case (id is None) we could use the id of the instance.
review319251002 updated at https://codereview.tryton.org/319251002/#ps257021002
msg57290 (view) Author: [hidden] (albertca) (Tryton committer) Date: 2020-04-19.11:34:18
I missed to mention what "much slower" means. In the tested scenario we can see that create takes 1.5s vs save taking 2.5s. The patch makes save go down to the 1.5s.
msg57287 (view) Author: [hidden] (albertca) (Tryton committer) Date: 2020-04-19.11:30:45
save(records) is much slower than the almost equivalent create([x._save_values for x in records]) even when records been created under the same context, transaction and user.

Some investigation showed that the problem comes from the use of dictionaries in the save() method. The proposed patch uses lists instead and makes the extra overhead of save compared to plain create is almost imperceptible.
History
Date User Action Args
2020-06-17 00:31:20roundup-botsetmessages: + msg58744
2020-06-17 00:31:17roundup-botsetstatus: testing -> resolved
nosy: + roundup-bot
messages: + msg58743
2020-05-10 14:07:49cedsetmessages: + msg57934
2020-04-26 20:12:49reviewbotsetmessages: + msg57647
2020-04-26 20:12:48reviewbotsetreviews: 319251002 -> 319251002, 295421002
2020-04-26 19:48:52cedsetmessages: + msg57646
2020-04-20 09:05:37jcavallosetnosy: + jcavallo
messages: + msg57319
2020-04-19 15:14:13cedsetnosy: + ced
messages: + msg57300
2020-04-19 12:33:46yangoonsetnosy: + yangoon
2020-04-19 12:33:19reviewbotsetnosy: + reviewbot
messages: + msg57293

Showing 10 items. Show all history (warning: this could be VERY long)