Tryton - Issues

 

Issue9369

Title Improve memory usage of storing record values
Priority feature Status resolved
Superseder Nosy List ced, pokoli, resteve, reviewbot, roundup-bot, semarie
Type performance Components trytond
Assigned To ced Keywords review
Reviews 297861002
View: 297861002

Created on 2020-05-26.17:23:46 by ced, last changed by roundup-bot.

Messages
New changeset f9d3260c00d8 by Cédric Krier in branch 'default':
Delete cache content instead of setting empty dictionary
https://hg.tryton.org/tryton-env/rev/f9d3260c00d8
New changeset 6e805b3304aa by Cédric Krier in branch 'default':
Delete cache content instead of setting empty dictionary
https://hg.tryton.org/trytond/rev/6e805b3304aa
New changeset baa1fcd4a061 by Cédric Krier in branch 'default':
Use custom class to store record data
https://hg.tryton.org/tryton-env/rev/baa1fcd4a061
New changeset 22260a3783da by Cédric Krier in branch 'default':
Use custom class to store record data
https://hg.tryton.org/trytond/rev/22260a3783da
New changeset b569af4f7d88 by Cédric Krier in branch 'default':
Use custom class to store record data
https://hg.tryton.org/modules/party_relationship/rev/b569af4f7d88
msg58303 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-05-27.10:18:03
On 2020-05-27 08:33, Sergi Almacellas Abellana wrote:
> I'm wondering if it will make sense to increase the default setting for cache size. As we require less storage for caching records, we can store more records with the same memory usage.

The value was not really choose from a memory computation because we can
not assume the memory available but more from an expected common usage.
So we can not put proper value without knowing the available memories
(and also the number of concurrent requests).
So for me, the current default values are still valid and still make
sense.
msg58300 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-05-27.08:33:03
I'm wondering if it will make sense to increase the default setting for cache size. As we require less storage for caching records, we can store more records with the same memory usage.
msg58299 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-05-26.18:41:03
On 2020-05-26 18:13, Sergi Almacellas Abellana wrote:
> IIUC when the number of fields of the models is high, the performance gain is less? Am I right?

Indeed it is the opposite.

> Which is the gain with a model with more fields? Could you test one of the following:
> 
>                       model                      | count 
> -------------------------------------------------+-------
>  account.invoice                                 |    52

I tested this one and it is 40% less.
review297861002 updated at https://codereview.tryton.org/297861002/#ps309711002
msg58296 (view) Author: [hidden] (pokoli) (Tryton committer) (Tryton translator) Date: 2020-05-26.18:13:17
IIUC when the number of fields of the models is high, the performance gain is less? Am I right?

Which is the gain with a model with more fields? Could you test one of the following:

                      model                      | count 
-------------------------------------------------+-------
 account.invoice                                 |    52
 sale.sale                                       |    49
 party.party                                     |    47
 product.product                                 |    45
 purchase.purchase                               |    43
 account.move.line                               |    40
 account.account                                 |    37
 stock.move                                      |    36

country.country only has 12 fields.
review297861002 updated at https://codereview.tryton.org/297861002/#ps309701002
msg58292 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-05-26.17:34:29
With review297861002, I could measure a reduction of about 30% of memory usage to store the result of Country.search([]) (which is already filled with the column of the table country).
The gain varies depending on the number of a fields a Model has.
msg58290 (view) Author: [hidden] (ced) (Tryton committer) (Tryton translator) Date: 2020-05-26.17:23:45
For now, the values of a record are set as dictionaries in the local and transactional caches.
But dictionaries are memory expensive:

>>> sys.getsizeof({})
248
>>> sys.getsizeof([])
72
>>> sys.getsizeof(object())
16

But a class with __slots__ have a size of 16 + 8 per slot (64 without slots but it contains a __dict__ of 248).

So we see that we can store 29 ((248 - 16) / 8) slots for the same memory size as an empty dict.

So the proposal is to create a record class for each model that has a slot for each field and some methods similar to dict like get, keys, items, update etc. Those class should be used in local and transaction caches and in _values and _init_values.
The side effect is that we also receive an automatic check that prevents to fill the cache record with unknown fields (thanks to the slots).
History
Date User Action Args
2020-06-15 14:13:44roundup-botsetmessages: + msg58711
2020-06-15 14:13:42roundup-botsetmessages: + msg58710
2020-06-13 09:48:50roundup-botsetmessages: + msg58686
2020-06-13 09:48:39roundup-botsetmessages: + msg58684
2020-06-13 09:48:34roundup-botsetstatus: testing -> resolved
nosy: + roundup-bot
messages: + msg58682
2020-05-27 10:18:03cedsetmessages: + msg58303
2020-05-27 08:33:04pokolisetmessages: + msg58300
2020-05-26 19:41:14semariesetnosy: + semarie
2020-05-26 18:41:04cedsetmessages: + msg58299
2020-05-26 18:38:01reviewbotsetmessages: + msg58297

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