From 10412d351cc4ba66ff4d10879d22535fc4e3e222 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Wed, 11 Oct 2017 13:39:02 +0200 Subject: [PATCH] [FIX] fields: prefetching of related fields (#20017) Prefetching is underused when all fields are traversed one record at a time. So instead, traverse all records one field at a time. This guarantees batch prefetching/computation on every field being accessed. --- odoo/fields.py | 45 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/odoo/fields.py b/odoo/fields.py index 8cc8a748205..81697970311 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -73,6 +73,10 @@ def copy_cache(records, env): if field.relational and isinstance(value, tuple): todo[field.comodel_name].update(value) +def first(records): + """ Return the first record in ``records``, with the same prefetching. """ + return next(iter(records)) if len(records) > 1 else records + def resolve_mro(model, name, predicate): """ Return the list of successively overridden values of attribute ``name`` @@ -579,12 +583,41 @@ class Field(object): """ Compute the related field ``self`` on ``records``. """ # when related_sudo, bypass access rights checks when reading values others = records.sudo() if self.related_sudo else records - for record, other in zip(records, others): - if not record.id and record.env != other.env: - # draft records: copy record's cache to other's cache first - copy_cache(record, other.env) - other, field = self.traverse_related(other) - record[self.name] = other[field.name] + # copy the cache of draft records into others' cache + if records.env != others.env: + copy_cache(records - records.filtered('id'), others.env) + # + # Traverse fields one by one for all records, in order to take advantage + # of prefetching for each field access. In order to clarify the impact + # of the algorithm, consider traversing 'foo.bar' for records a1 and a2, + # where 'foo' is already present in cache for a1, a2. Initially, both a1 + # and a2 are marked for prefetching. As the commented code below shows, + # traversing all fields one record at a time will fetch 'bar' one record + # at a time. + # + # b1 = a1.foo # mark b1 for prefetching + # v1 = b1.bar # fetch/compute bar for b1 + # b2 = a2.foo # mark b2 for prefetching + # v2 = b2.bar # fetch/compute bar for b2 + # + # On the other hand, traversing all records one field at a time ensures + # maximal prefetching for each field access. + # + # b1 = a1.foo # mark b1 for prefetching + # b2 = a2.foo # mark b2 for prefetching + # v1 = b1.bar # fetch/compute bar for b1, b2 + # v2 = b2.bar # value already in cache + # + # This difference has a major impact on performance, in particular in + # the case where 'bar' is a computed field that takes advantage of batch + # computation. + # + values = list(others) + for name in self.related[:-1]: + values = [first(value[name]) for value in values] + # assign final values to records + for record, value in zip(records, values): + record[self.name] = value[self.related_field.name] def _inverse_related(self, records): """ Inverse the related field ``self`` on ``records``. """