From a91cb08c5d188ae0332092aee7494b4e8318030f Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Fri, 15 Jan 2021 12:22:20 +0000 Subject: [PATCH] [IMP] core: avoid flushing fields to read The idea is to avoid flushing the fields to fetch in method _read(). This delays UPDATE queries, and makes the prefetching mechanism simpler and more effective. We do this by not overwriting the cache values by the values fetched from database. This simple idea allows to fetch more fields and more records without having to care about pending computations and updates. But it requires the cache consistency to be much more strict, because nothing will "fix" the cache inconsistencies "by chance". And it also requires pending updates to be present in cache. Part-of: odoo/odoo#66938 --- .../tests/test_performance.py | 28 ++----------------- odoo/api.py | 9 ++++++ odoo/fields.py | 19 +++++++------ odoo/models.py | 20 +++++++------ 4 files changed, 33 insertions(+), 43 deletions(-) diff --git a/odoo/addons/test_performance/tests/test_performance.py b/odoo/addons/test_performance/tests/test_performance.py index b36d9955406..a0b651c860f 100644 --- a/odoo/addons/test_performance/tests/test_performance.py +++ b/odoo/addons/test_performance/tests/test_performance.py @@ -455,17 +455,12 @@ class TestPerformance(SavepointCaseWithUserDemo): with self.assertQueries([], flush=False): records[1].value = 42 - # fetching 'name' prefetches all fields except 'value_pc' on all records - # (because 'value_pc' must be computed); reading those fields causes - # field 'value' to be flushed first + # fetching 'name' prefetches all fields on all records queries = [ - ''' UPDATE "test_performance_base" - SET "value"=%s, "write_date"=%s, "write_uid"=%s - WHERE id IN %s - ''', ''' SELECT "test_performance_base"."id" AS "id", "test_performance_base"."name" AS "name", "test_performance_base"."value" AS "value", + "test_performance_base"."value_pc" AS "value_pc", "test_performance_base"."partner_id" AS "partner_id", "test_performance_base"."total" AS "total", "test_performance_base"."create_uid" AS "create_uid", @@ -482,24 +477,7 @@ class TestPerformance(SavepointCaseWithUserDemo): with self.assertQueries([], flush=False): result_value = [record.value for record in records] - # fetching 'value_pc' (missing from above) prefetches all fields on all - # records except the one to compute - queries = [ - ''' SELECT "test_performance_base"."id" AS "id", - "test_performance_base"."name" AS "name", - "test_performance_base"."value" AS "value", - "test_performance_base"."partner_id" AS "partner_id", - "test_performance_base"."total" AS "total", - "test_performance_base"."create_uid" AS "create_uid", - "test_performance_base"."create_date" AS "create_date", - "test_performance_base"."write_uid" AS "write_uid", - "test_performance_base"."write_date" AS "write_date", - "test_performance_base"."value_pc" AS "value_pc" - FROM "test_performance_base" - WHERE "test_performance_base".id IN %s - ''', - ] - with self.assertQueries(queries, flush=False): + with self.assertQueries([], flush=False): result_value_pc = [record.value_pc for record in records] result = list(zip(result_name, result_value, result_value_pc)) diff --git a/odoo/api.py b/odoo/api.py index 7f4382bc71d..2950c352711 100644 --- a/odoo/api.py +++ b/odoo/api.py @@ -918,6 +918,15 @@ class Cache(object): field_cache = self._set_field_cache(records, field) field_cache.update(zip(records._ids, values)) + def insert_missing(self, records, field, values): + """ Set the values of ``field`` for the records in ``records`` that + don't have a value yet. In other words, this does not overwrite + existing values in cache. + """ + field_cache = self._set_field_cache(records, field) + for id_, val in zip(records._ids, values): + field_cache.setdefault(id_, val) + def remove(self, record, field): """ Remove the value of ``field`` for ``record``. """ try: diff --git a/odoo/fields.py b/odoo/fields.py index 2c65fbc393f..0ebb328d140 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -1697,6 +1697,11 @@ class _String(Field): # invalidate the field in all languages because the fallback value # for translations is modified cache.invalidate([(self, records.ids)]) + if single_lang or lang == 'en_US': + # modifying with lang=None also updates the installed language, + # and modifying with a lang also updates for lang=None + others = records.with_context(lang=None if lang else single_lang) + cache.update(others, self, itertools.repeat(cache_value)) cache.update(records, self, itertools.repeat(cache_value)) @@ -2236,9 +2241,7 @@ class Binary(Field): att.res_id: att.datas for att in records.env['ir.attachment'].sudo().search(domain) } - cache = records.env.cache - for record in records: - cache.set(record, self, data.get(record.id, False)) + records.env.cache.insert_missing(records, self, map(data.get, records._ids)) def create(self, record_values): assert self.attachment @@ -3476,9 +3479,8 @@ class One2many(_RelationalMulti): group[get_id(line[inverse])].append(line.id) # store result in cache - cache = records.env.cache - for record in records: - cache.set(record, self, tuple(group[record.id])) + values = [tuple(group[id_]) for id_ in records._ids] + records.env.cache.insert_missing(records, self, values) def write_real(self, records_commands_list, create=False): """ Update real records. """ @@ -3850,9 +3852,8 @@ class Many2many(_RelationalMulti): group[row[0]].append(row[1]) # store result in cache - cache = records.env.cache - for record in records: - cache.set(record, self, tuple(group[record.id])) + values = [tuple(group[id_]) for id_ in records._ids] + records.env.cache.insert_missing(records, self, values) def write_real(self, records_commands_list, create=False): # records_commands_list = [(records, commands), ...] diff --git a/odoo/models.py b/odoo/models.py index 52ed7113aa2..6f9119b80eb 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -3401,12 +3401,9 @@ class BaseModel(metaclass=MetaModel): if f.prefetch == field.prefetch # discard fields with groups that the user may not access if not (f.groups and not self.user_has_groups(f.groups)) - # discard fields that must be recomputed - if not (f.compute and self.env.records_to_compute(f)) ] if field.name not in fnames: fnames.append(field.name) - self = self - self.env.records_to_compute(field) else: fnames = [field.name] self._read(fnames) @@ -3421,10 +3418,6 @@ class BaseModel(metaclass=MetaModel): return self.check_access_rights('read') - # if a read() follows a write(), we must flush updates, as read() will - # fetch from database and overwrites the cache (`test_update_with_id`) - self.flush_recordset(field_names) - # determine columns fields and those with their own read() method column_fields = [] other_fields = [] @@ -3445,6 +3438,12 @@ class BaseModel(metaclass=MetaModel): if column_fields: cr, context = self.env.cr, self.env.context + # If a read() follows a write(), we must flush the updates that have + # an impact on checking security rules, as they are injected into + # the query. However, we don't need to flush the fields to fetch, + # as explained below when putting values in cache. + self._flush_search([], order='id') + # make a query object for selecting ids, and apply security rules to it query = Query(cr, self._table, self._table_query) self._apply_ir_rules(query, 'read') @@ -3481,6 +3480,9 @@ class BaseModel(metaclass=MetaModel): ids = next(column_values) fetched = self.browse(ids) + # If we assume that the value of a pending update is in cache, we + # can avoid flushing pending updates if the fetched values do not + # overwrite values in cache. for field in column_fields: values = next(column_values) # post-process translations @@ -3488,8 +3490,8 @@ class BaseModel(metaclass=MetaModel): if any(values): translate = field.get_trans_func(fetched) values = [translate(id_, value) for id_, value in zip(ids, values)] - # store values in cache - self.env.cache.update(fetched, field, values) + # store values in cache, but without overwriting + self.env.cache.insert_missing(fetched, field, values) # process non-column fields for field in other_fields: