From 9b7a4e1815de580575bed6657f9f8915907ce534 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Tue, 9 Jun 2020 17:51:55 +0000 Subject: [PATCH] [IMP] core: smaller and faster field cache Consider a context-dependent field and two possible cache structures for storing the data of that field for N records and K context keys. The common use-case is to assume that N is much larger than K. We look at how many dicts are used in each cache structure, where we label dicts with N entries as "large", and dicts with K entries as "small". Structure | Number of dicts ---------------------------+------------------- cache[field][id][key] | 1 large + N small cache[field][key][id] | 1 small + K large It is known that "small" dicts have a large relative overhead when compared to "large" dicts. Given this fact, the second cache structure will generate less memory overhead. Here are numbers for N=16, showing the size in bytes taken by the cache structure. Larger values of N lead to larger size differences, always in favor of the second structure. Structure | K=1 | K=2 | K=3 | K=4 | K=5 | K=6 -----------------------+------+------+------+------+------+------ cache[field][id][key] | 4488 | 4488 | 4488 | 4488 | 4488 | 6536 cache[field][key][id] | 888 | 1536 | 2184 | 2832 | 3480 | 4256 Moreover, some memory operations are performed on many records and a single context key. Those operations are clearly simpler and faster with the second cache structure. --- odoo/api.py | 140 +++++++++++++++++++++++----------------------------- 1 file changed, 61 insertions(+), 79 deletions(-) diff --git a/odoo/api.py b/odoo/api.py index 9dd5873d556..bace379051b 100644 --- a/odoo/api.py +++ b/odoo/api.py @@ -771,23 +771,23 @@ NOTHING = object() class Cache(object): """ Implementation of the cache of records. """ def __init__(self): - # {field: {record_id: value}} + # {field: {record_id: value}, field: {context_key: {record_id: value}}} self._data = defaultdict(dict) def contains(self, record, field): """ Return whether ``record`` has a value for ``field``. """ - if field.depends_context: - key = record.env.cache_key(field) - return key in self._data.get(field, {}).get(record.id, {}) - return record.id in self._data.get(field, ()) + field_cache = self._data.get(field, ()) + if field_cache and field.depends_context: + field_cache = field_cache.get(record.env.cache_key(field), ()) + return record.id in field_cache def get(self, record, field, default=NOTHING): """ Return the value of ``field`` for ``record``. """ try: - value = self._data[field][record._ids[0]] + field_cache = self._data[field] if field.depends_context: - value = value[record.env.cache_key(field)] - return value + field_cache = field_cache[record.env.cache_key(field)] + return field_cache[record._ids[0]] except KeyError: if default is NOTHING: raise CacheMiss(record, field) @@ -795,53 +795,48 @@ class Cache(object): def set(self, record, field, value): """ Set the value of ``field`` for ``record``. """ + field_cache = self._data[field] if field.depends_context: - key = record.env.cache_key(field) - self._data[field].setdefault(record._ids[0], {})[key] = value - else: - self._data[field][record._ids[0]] = value + field_cache = field_cache.setdefault(record.env.cache_key(field), {}) + field_cache[record._ids[0]] = value def update(self, records, field, values): """ Set the values of ``field`` for several ``records``. """ + field_cache = self._data[field] if field.depends_context: - key = records.env.cache_key(field) - field_cache = self._data[field] - for record_id, value in zip(records._ids, values): - field_cache.setdefault(record_id, {})[key] = value - else: - self._data[field].update(zip(records._ids, values)) + field_cache = field_cache.setdefault(records.env.cache_key(field), {}) + field_cache.update(zip(records._ids, values)) def remove(self, record, field): """ Remove the value of ``field`` for ``record``. """ try: - del self._data[field][record.id] + field_cache = self._data[field] + if field.depends_context: + field_cache = field_cache[record.env.cache_key(field)] + del field_cache[record._ids[0]] except KeyError: pass def get_values(self, records, field): """ Return the cached values of ``field`` for ``records``. """ field_cache = self._data[field] - key = records.env.cache_key(field) if field.depends_context else None + if field.depends_context: + field_cache = field_cache.get(records.env.cache_key(field), {}) for record_id in records._ids: try: - if key is not None: - yield field_cache[record_id][key] - else: - yield field_cache[record_id] + yield field_cache[record_id] except KeyError: pass def get_until_miss(self, records, field): """ Return the cached values of ``field`` for ``records`` until a value is not found. """ field_cache = self._data[field] - key = records.env.cache_key(field) if field.depends_context else None + if field.depends_context: + field_cache = field_cache.get(records.env.cache_key(field), {}) vals = [] for record_id in records._ids: try: - if key is not None: - vals.append(field_cache[record_id][key]) - else: - vals.append(field_cache[record_id]) + vals.append(field_cache[record_id]) except KeyError: break return vals @@ -849,14 +844,12 @@ class Cache(object): def get_records_different_from(self, records, field, value): """ Return the subset of ``records`` that has not ``value`` for ``field``. """ field_cache = self._data[field] - key = records.env.cache_key(field) if field.depends_context else None + if field.depends_context: + field_cache = field_cache.get(records.env.cache_key(field), {}) ids = [] for record_id in records._ids: try: - if key is not None: - val = field_cache[record_id][key] - else: - val = field_cache[record_id] + val = field_cache[record_id] except KeyError: ids.append(record_id) else: @@ -869,35 +862,27 @@ class Cache(object): for name, field in record._fields.items(): if name == 'id': continue - values = self._data.get(field, {}) - if record.id not in values: - continue - if field.depends_context and record.env.cache_key(field) not in values[record.id]: - continue - yield field + field_cache = self._data.get(field, {}) + if field.depends_context: + field_cache = field_cache.get(record.env.cache_key(field), ()) + if record.id in field_cache: + yield field def get_records(self, model, field): """ Return the records of ``model`` that have a value for ``field``. """ field_cache = self._data[field] if field.depends_context: - key = model.env.cache_key(field) - ids = [id_ for id_, value in field_cache.items() if key in value] - else: - ids = list(field_cache) - return model.browse(ids) + field_cache = field_cache.get(model.env.cache_key(field), ()) + return model.browse(field_cache) def get_missing_ids(self, records, field): """ Return the ids of ``records`` that have no value for ``field``. """ field_cache = self._data[field] if field.depends_context: - key = records.env.cache_key(field) - for record_id in records._ids: - if key not in field_cache.get(record_id, ()): - yield record_id - else: - for record_id in records._ids: - if record_id not in field_cache: - yield record_id + field_cache = field_cache.get(records.env.cache_key(field), ()) + for record_id in records._ids: + if record_id not in field_cache: + yield record_id def invalidate(self, spec=None): """ Invalidate the cache, partially or totally depending on ``spec``. """ @@ -908,8 +893,9 @@ class Cache(object): if ids is None: self._data.pop(field, None) else: - field_cache = self._data.get(field) - if field_cache: + field_cache = self._data.get(field, {}) + field_caches = field_cache.values() if field.depends_context else [field_cache] + for field_cache in field_caches: for id in ids: field_cache.pop(id, None) @@ -918,41 +904,37 @@ class Cache(object): # flush fields to be recomputed before evaluating the cache env['res.partner'].recompute() - # make a full copy of the cache, and invalidate it - dump = defaultdict(dict) - key_cache = self._data - for field, field_cache in key_cache.items(): - for record_id, value in field_cache.items(): - if record_id: - dump[field][record_id] = value - + # make a copy of the cache, and invalidate it + dump = dict(self._data) self.invalidate() # re-fetch the records, and compare with their former cache invalids = [] - for field, field_dump in dump.items(): + + def check(model, field, field_dump): records = env[field.model_name].browse(field_dump) for record in records: + if not record.id: + continue try: cached = field_dump[record.id] - if field.depends_context: - for context_keys, value in cached.items(): - context = dict(zip(field.depends_context, context_keys)) - value = field.convert_to_record(value, record) - fetched = record.with_context(context)[field.name] - if fetched != value: - info = {'cached': value, 'fetched': fetched} - invalids.append((record, field, info)) - else: - cached = field_dump[record.id] - fetched = record[field.name] - value = field.convert_to_record(cached, record) - if fetched != value: - info = {'cached': value, 'fetched': fetched} - invalids.append((record, field, info)) + value = field.convert_to_record(cached, record) + fetched = record[field.name] + if fetched != value: + info = {'cached': value, 'fetched': fetched} + invalids.append((record, field, info)) except (AccessError, MissingError): pass + for field, field_dump in dump.items(): + model = env[field.model_name] + if field.depends_context: + for context_keys, field_cache in field_dump.items(): + context = dict(zip(field.depends_context, context_keys)) + check(model.with_context(context), field, field_cache) + else: + check(model, field, field_dump) + if invalids: raise UserError('Invalid cache for fields\n' + pformat(invalids))