[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
This commit is contained in:
@@ -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))
|
||||
|
||||
@@ -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:
|
||||
|
||||
+10
-9
@@ -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), ...]
|
||||
|
||||
+11
-9
@@ -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:
|
||||
|
||||
Reference in New Issue
Block a user