[IMP] models: new implementation of prefetching

6 files changed, 101 insertions, 122 deletions

diff --git a/odoo/addons/base/models/res_partner.py b/odoo/addons/base/models/res_partner.py
index dd7635d5312..fa23086b16a 100644
--- a/odoo/addons/base/models/res_partner.py
+++ b/odoo/addons/base/models/res_partner.py
@@ -479,7 +479,7 @@ class Partner(models.Model):
         as if they were related fields """
         commercial_partner = self.commercial_partner_id
         if commercial_partner != self:
-            sync_vals = commercial_partner.with_prefetch()._update_fields_values(self._commercial_fields())
+            sync_vals = commercial_partner._update_fields_values(self._commercial_fields())
             self.write(sync_vals)
 
     @api.multi
diff --git a/odoo/addons/base/tests/test_api.py b/odoo/addons/base/tests/test_api.py
index b885744b06e..fe1c557154f 100644
--- a/odoo/addons/base/tests/test_api.py
+++ b/odoo/addons/base/tests/test_api.py
@@ -287,11 +287,11 @@ class TestAPI(common.TransactionCase):
         self.assertTrue(len(partners) > 1)
 
         # all the records in partners are ready for prefetching
-        self.assertItemsEqual(partners.ids, partners._prefetch['res.partner'])
+        self.assertItemsEqual(partners.ids, partners._prefetch_ids)
 
         # reading ONE partner should fetch them ALL
         for partner in partners:
-            partner.state_id
+            state = partner.state_id
             break
         partner_ids_with_field = [partner.id
                                   for partner in partners
@@ -299,45 +299,42 @@ class TestAPI(common.TransactionCase):
         self.assertItemsEqual(partner_ids_with_field, partners.ids)
 
         # partners' states are ready for prefetching
-        state_ids = {sid
-                       for partner in partners
-                       for sid in partner._cache['state_id']}
+        state_ids = {sid for partner in partners for sid in partner._cache['state_id']}
         self.assertTrue(len(state_ids) > 1)
-        self.assertItemsEqual(state_ids, partners._prefetch['res.country.state'])
+        self.assertItemsEqual(state_ids, state._prefetch_ids)
 
         # reading ONE partner country should fetch ALL partners' countries
         for partner in partners:
             if partner.state_id:
                 partner.state_id.name
                 break
-        state_ids_with_field = [state.id
-                                  for state in partners.state_id
-                                  if 'name' in state._cache]
+        state_ids_with_field = [st.id for st in partners.state_id if 'name' in st._cache]
         self.assertItemsEqual(state_ids_with_field, state_ids)
 
     @mute_logger('odoo.models')
-    def test_60_prefetch_object(self):
+    def test_60_prefetch_model(self):
         """ Check the prefetching model. """
         partners = self.env['res.partner'].search([], limit=models.PREFETCH_MAX)
         self.assertTrue(partners)
 
         def same_prefetch(a, b):
-            self.assertIs(a._prefetch, b._prefetch)
+            self.assertEqual(set(a._prefetch_ids), set(b._prefetch_ids))
+
         def diff_prefetch(a, b):
-            self.assertIsNot(a._prefetch, b._prefetch)
+            self.assertNotEqual(set(a._prefetch_ids), set(b._prefetch_ids))
 
-        # the recordset operations below should create new prefetch objects
+        # the recordset operations below use different prefetch sets
         diff_prefetch(partners, partners.browse())
-        diff_prefetch(partners, partners.browse(partners.ids))
         diff_prefetch(partners, partners[0])
         diff_prefetch(partners, partners[:10])
 
-        # the recordset operations below should pass the prefetch object
+        # the recordset operations below share the prefetch set
+        same_prefetch(partners, partners.browse(partners.ids))
         same_prefetch(partners, partners.sudo(self.env.ref('base.user_demo')))
         same_prefetch(partners, partners.with_context(active_test=False))
-        same_prefetch(partners, partners[:10].with_prefetch(partners._prefetch))
+        same_prefetch(partners, partners[:10].with_prefetch(partners._prefetch_ids))
 
-        # iterating and reading relational fields should pass the prefetch object
+        # iteration and relational fields should use the same prefetch set
         self.assertEqual(type(partners).country_id.type, 'many2one')
         self.assertEqual(type(partners).bank_ids.type, 'one2many')
         self.assertEqual(type(partners).category_id.type, 'many2many')
@@ -356,16 +353,10 @@ class TestAPI(common.TransactionCase):
         }
         partners = partners.create(vals0) + partners.create(vals1)
         for partner in partners:
-            same_prefetch(partners, partner)
-            same_prefetch(partners, partner.country_id)
-            same_prefetch(partners, partner.bank_ids)
-            same_prefetch(partners, partner.category_id)
-
-        # same with empty recordsets
-        empty = partners.browse()
-        same_prefetch(empty, empty.country_id)
-        same_prefetch(empty, empty.bank_ids)
-        same_prefetch(empty, empty.category_id)
+            same_prefetch(partner, partners)
+            same_prefetch(partner.country_id, partners.country_id)
+            same_prefetch(partner.bank_ids, partners.bank_ids)
+            same_prefetch(partner.category_id, partners.category_id)
 
     @mute_logger('odoo.models')
     def test_60_prefetch_read(self):
diff --git a/odoo/addons/test_new_api/tests/test_new_fields.py b/odoo/addons/test_new_api/tests/test_new_fields.py
index b79d75a57f3..d9253aaba69 100644
--- a/odoo/addons/test_new_api/tests/test_new_fields.py
+++ b/odoo/addons/test_new_api/tests/test_new_fields.py
@@ -921,7 +921,7 @@ class TestFields(common.TransactionCase):
         cat1, cat2 = cats
         self.assertEqual(cat2.name, 'ACCESS')
         # both categories should be ready for prefetching
-        self.assertItemsEqual(cat2._prefetch[Category._name], cats.ids)
+        self.assertItemsEqual(cat2._prefetch_ids, cats.ids)
         # but due to our (lame) overwrite of `read`, it should not forbid us to read records we have access to
         self.assertFalse(cat2.discussions)
         self.assertEqual(cat2.parent, cat1)
diff --git a/odoo/api.py b/odoo/api.py
index 13d6f89f97c..4a177e668dc 100644
--- a/odoo/api.py
+++ b/odoo/api.py
@@ -814,7 +814,7 @@ class Environment(Mapping):
 
     def __getitem__(self, model_name):
         """ Return an empty recordset from the given model. """
-        return self.registry[model_name]._browse((), self)
+        return self.registry[model_name]._browse(self, (), ())
 
     def __iter__(self):
         """ Return an iterator on model names. """
@@ -1065,6 +1065,14 @@ class Cache(object):
         value = self._data[key][field].get(record.id, SpecialValue(None))
         return default if isinstance(value, SpecialValue) else value
 
+    def get_values(self, records, field, default=None):
+        """ Return the regular values of ``field`` for ``records``. """
+        key = records.env.cache_key(field)
+        field_cache = self._data[key][field]
+        for record_id in records._ids:
+            value = field_cache.get(record_id, SpecialValue(None))
+            yield default if isinstance(value, SpecialValue) else value
+
     def get_special(self, record, field, default=None):
         """ Return the special value of ``field`` for ``record``. """
         key = record.env.cache_key(field)
diff --git a/odoo/fields.py b/odoo/fields.py
index 00a204c9404..2f8e64459a1 100644
--- a/odoo/fields.py
+++ b/odoo/fields.py
@@ -24,7 +24,7 @@ import psycopg2
 
 from .sql_db import LazyCursor
 from .tools import float_repr, float_round, frozendict, html_sanitize, human_size, pg_varchar,\
-    ustr, OrderedSet, pycompat, sql, date_utils, groupby
+    ustr, OrderedSet, pycompat, sql, date_utils, unique
 from .tools import DEFAULT_SERVER_DATE_FORMAT as DATE_FORMAT
 from .tools import DEFAULT_SERVER_DATETIME_FORMAT as DATETIME_FORMAT
 from .tools.translate import html_translate, _
@@ -568,7 +568,7 @@ class Field(MetaField('DummyField', (object,), {})):
         """ Traverse the fields of the related field `self` except for the last
         one, and return it as a pair `(last_record, last_field)`. """
         for name in self.related[:-1]:
-            record = record[name][:1].with_prefetch(record._prefetch)
+            record = first(record[name])
         return record, self.related_field
 
     def _compute_related(self, records):
@@ -1109,7 +1109,6 @@ class Field(MetaField('DummyField', (object,), {})):
                 self.compute_value(record)
             else:
                 recs = record._in_cache_without(self)
-                recs = recs.with_prefetch(record._prefetch)
                 self.compute_value(recs)
 
         else:
@@ -1995,17 +1994,13 @@ class Reference(Selection):
 
     def convert_to_cache(self, value, record, validate=True):
         # cache format: (res_model, res_id) or False
-        def process(res_model, res_id):
-            record._prefetch[res_model].add(res_id)
-            return (res_model, res_id)
-
         if isinstance(value, BaseModel):
             if not validate or (value._name in self.get_values(record.env) and len(value) <= 1):
-                return process(value._name, value.id) if value else False
+                return (value._name, value.id) if value else False
         elif isinstance(value, str):
             res_model, res_id = value.split(',')
             if record.env[res_model].browse(int(res_id)).exists():
-                return process(res_model, int(res_id))
+                return (res_model, int(res_id))
             else:
                 return False
         elif not value:
@@ -2013,7 +2008,7 @@ class Reference(Selection):
         raise ValueError("Wrong value for %s: %r" % (self, value))
 
     def convert_to_record(self, value, record):
-        return value and record.env[value[0]].browse([value[1]], record._prefetch)
+        return value and record.env[value[0]].browse([value[1]])
 
     def convert_to_read(self, value, record, use_name_get=True):
         return "%s,%s" % (value._name, value.id) if value else False
@@ -2164,26 +2159,24 @@ class Many2one(_Relational):
 
     def convert_to_cache(self, value, record, validate=True):
         # cache format: tuple(ids)
-        def process(ids):
-            return record._prefetch[self.comodel_name].update(ids) or ids
-
         if type(value) in IdType:
-            return process((value,))
+            return (value,)
         elif isinstance(value, BaseModel):
             if not validate or (value._name == self.comodel_name and len(value) <= 1):
-                return process(value._ids)
+                return value._ids
             raise ValueError("Wrong value for %s: %r" % (self, value))
         elif isinstance(value, tuple):
             # value is either a pair (id, name), or a tuple of ids
-            return process(value[:1])
+            return value[:1]
         elif isinstance(value, dict):
-            return process(record.env[self.comodel_name].new(value)._ids)
+            return record.env[self.comodel_name].new(value)._ids
         else:
             return ()
 
     def convert_to_record(self, value, record):
         # use registry to avoid creating a recordset for the model
-        return record.env.registry[self.comodel_name]._browse(value, record.env, record._prefetch)
+        prefetch_ids = PrefetchValueIds(record, self)
+        return record.pool[self.comodel_name]._browse(record.env, value, prefetch_ids)
 
     def convert_to_read(self, value, record, use_name_get=True):
         if use_name_get and value:
@@ -2267,12 +2260,9 @@ class _RelationalMulti(_Relational):
 
     def convert_to_cache(self, value, record, validate=True):
         # cache format: tuple(ids)
-        def process(ids):
-            return record._prefetch[self.comodel_name].update(ids) or ids
-
         if isinstance(value, BaseModel):
             if not validate or (value._name == self.comodel_name):
-                return process(value._ids)
+                return value._ids
         elif isinstance(value, (list, tuple)):
             # value is a list/tuple of commands, dicts or record ids
             comodel = record.env[self.comodel_name]
@@ -2302,14 +2292,15 @@ class _RelationalMulti(_Relational):
                 else:
                     ids.add(command)
             # return result as a tuple
-            return process(tuple(ids))
+            return tuple(ids)
         elif not value:
             return ()
         raise ValueError("Wrong value for %s: %s" % (self, value))
 
     def convert_to_record(self, value, record):
         # use registry to avoid creating a recordset for the model
-        return record.env.registry[self.comodel_name]._browse(value, record.env, record._prefetch)
+        prefetch_ids = PrefetchValueIds(record, self)
+        return record.pool[self.comodel_name]._browse(record.env, value, prefetch_ids)
 
     def convert_to_read(self, value, record, use_name_get=True):
         return value.ids
@@ -2500,16 +2491,14 @@ class One2many(_RelationalMulti):
                 comodel.create(to_create)
                 to_create.clear()
             if to_relink:
-                prefetch = comodel.browse(to_relink)._prefetch
-                comodel_sudo = comodel.sudo().with_context(prefetch_fields=False)
-                # group lines by record, and relink them in batch
-                groups = groupby(to_relink, to_relink.get)
-                for record_id, line_ids in groups:
-                    lines = comodel_sudo.browse(line_ids, prefetch).filtered(
-                        lambda line: int(line[inverse]) != record_id
-                    )
-                    if lines:
-                        comodel.browse(lines._ids).write({inverse: record_id})
+                # group line ids to update by record id, and update them
+                groups = defaultdict(list)
+                lines = comodel.browse(to_relink).sudo().with_context(prefetch_fields=False)
+                for line, record_id in zip(lines, to_relink.values()):
+                    if int(line[inverse]) != record_id:
+                        groups[record_id].append(line.id)
+                for record_id, line_ids in groups.items():
+                    comodel.browse(line_ids).write({inverse: record_id})
                 to_relink.clear()
 
         with model.env.norecompute():
@@ -2870,6 +2859,27 @@ class Id(Field):
     def __set__(self, record, value):
         raise TypeError("field 'id' cannot be assigned")
 
+
+class PrefetchValueIds(object):
+    """ An iterable on the ids of the cached values of a relational field for
+        the prefetch set of a record.
+    """
+    __slots__ = ('_record', '_field')
+
+    def __init__(self, record, field):
+        self._record = record
+        self._field = field
+
+    def __iter__(self):
+        record = self._record
+        records = record.browse(record._prefetch_ids)
+        return unique(
+            id_
+            for ids in record.env.cache.get_values(records, self._field, ())
+            for id_ in ids
+        )
+
+
 # imported here to avoid dependency cycle issues
 from odoo import SUPERUSER_ID
 from .exceptions import AccessError, MissingError, UserError
diff --git a/odoo/models.py b/odoo/models.py
index 365e4d1a332..85d027fff8b 100644
--- a/odoo/models.py
+++ b/odoo/models.py
@@ -2812,14 +2812,10 @@ Fields:
 
         # retrieve results from records; this takes values from the cache and
         # computes remaining fields
-        self = self.with_prefetch(self._prefetch.copy())
         data = [(record, {'id': record._ids[0]}) for record in self]
         use_name_get = (load == '_classic_read')
         for name in (stored + inherited + computed):
             convert = self._fields[name].convert_to_read
-            # restrict the prefetching of self's model to self; this avoids
-            # computing fields on a larger recordset than self
-            self._prefetch[self._name] = set(self._ids)
             for record, vals in data:
                 # missing records have their vals empty
                 if not vals:
@@ -2867,7 +2863,6 @@ Fields:
 
         # fetch records with read()
         assert self in records and field in fs
-        records = records.with_prefetch(self._prefetch)
         result = []
         try:
             result = records.read([f.name for f in fs], load='_classic_write')
@@ -2878,7 +2873,7 @@ Fields:
         # check the cache, and update it if necessary
         if not self.env.cache.contains_value(self, field):
             for values in result:
-                record = self.browse(values.pop('id'), self._prefetch)
+                record = self.browse(values.pop('id'))
                 record._cache.update(record._convert_to_cache(values, validate=False))
             if not self.env.cache.contains(self, field):
                 exc = AccessError("No value found for %s.%s" % (self, field.name))
@@ -2954,7 +2949,7 @@ Fields:
                             values[index] = translate(ids[index], values[index])
 
             # store result in cache
-            target = self.browse([], self._prefetch)
+            target = self.browse()
             for field, values in zip(fields_pre, field_values_list):
                 convert = field.convert_to_cache
                 # Note that the target record passed to convert below is empty.
@@ -4690,34 +4685,34 @@ Fields:
     #
 
     @classmethod
-    def _browse(cls, ids, env, prefetch=None, add_prefetch=True):
+    def _browse(cls, env, ids, prefetch_ids):
         """ Create a recordset instance.
 
-        :param ids: a tuple of record ids
         :param env: an environment
-        :param prefetch: an optional prefetch object
+        :param ids: a tuple of record ids
+        :param prefetch_ids: a collection of record ids (for prefetching)
         """
         records = object.__new__(cls)
         records.env = env
         records._ids = ids
-        if prefetch is None:
-            prefetch = defaultdict(set)         # {model_name: set(ids)}
-        records._prefetch = prefetch
-        if add_prefetch:
-            prefetch[cls._name].update(ids)
+        records._prefetch_ids = prefetch_ids
         return records
 
-    def browse(self, arg=None, prefetch=None):
+    def browse(self, ids=None):
         """ browse([ids]) -> records
 
         Returns a recordset for the ids provided as parameter in the current
         environment.
 
-        Can take no ids, a single id or a sequence of ids.
+        Can take no ids, a single id or an iterable of ids.
         """
-        ids = _normalize_ids(arg)
-        #assert all(isinstance(id, IdType) for id in ids), "Browsing invalid ids: %s" % ids
-        return self._browse(ids, self.env, prefetch)
+        if not ids:
+            ids = ()
+        elif ids.__class__ in IdType:
+            ids = (ids,)
+        else:
+            ids = tuple(ids)
+        return self._browse(self.env, ids, ids)
 
     #
     # Internal properties, for manipulating the instance's implementation
@@ -4763,7 +4758,7 @@ Fields:
 
         :type env: :class:`~odoo.api.Environment`
         """
-        return self._browse(self._ids, env, self._prefetch)
+        return self._browse(env, self._ids, self._prefetch_ids)
 
     def sudo(self, user=SUPERUSER_ID):
         """ sudo([user=SUPERUSER])
@@ -4819,13 +4814,15 @@ Fields:
         context = dict(args[0] if args else self._context, **kwargs)
         return self.with_env(self.env(context=context))
 
-    def with_prefetch(self, prefetch=None):
-        """ with_prefetch([prefetch]) -> records
+    def with_prefetch(self, prefetch_ids=None):
+        """ with_prefetch([prefetch_ids]) -> records
 
-        Return a new version of this recordset that uses the given prefetch
-        object, or a new prefetch object if not given.
+        Return a new version of this recordset that uses the given prefetch ids,
+        or ``self``'s ids if not given.
         """
-        return self._browse(self._ids, self.env, prefetch)
+        if prefetch_ids is None:
+            prefetch_ids = self._ids
+        return self._browse(self.env, self._ids, prefetch_ids)
 
     def _convert_to_cache(self, values, update=False, validate=True):
         """ Convert the ``values`` dictionary into cached values.
@@ -4835,7 +4832,7 @@ Fields:
             :param validate: whether values must be checked
         """
         fields = self._fields
-        target = self if update else self.browse([], self._prefetch)
+        target = self if update else self.browse()
         return {
             name: fields[name].convert_to_cache(value, target, validate=validate)
             for name, value in values.items()
@@ -5015,7 +5012,7 @@ Fields:
     def __iter__(self):
         """ Return an iterator over ``self``. """
         for id in self._ids:
-            yield self._browse((id,), self.env, self._prefetch, False)
+            yield self._browse(self.env, (id,), self._prefetch_ids)
 
     def __contains__(self, item):
         """ Test whether ``item`` (record or field name) is an element of ``self``.
@@ -5147,9 +5144,9 @@ Fields:
             # important: one must call the field's getter
             return self._fields[key].__get__(self, type(self))
         elif isinstance(key, slice):
-            return self._browse(self._ids[key], self.env)
+            return self.browse(self._ids[key])
         else:
-            return self._browse((self._ids[key],), self.env)
+            return self.browse((self._ids[key],))
 
     def __setitem__(self, key, value):
         """ Assign the field ``key`` to ``value`` in record ``self``. """
@@ -5171,7 +5168,7 @@ Fields:
             (:class:`Field` instance), including ``self``.
             Return at most ``limit`` records.
         """
-        recs = self.browse(self._prefetch[self._name])
+        recs = self.browse(self._prefetch_ids)
         ids = [self.id]
         for record_id in self.env.cache.get_missing_ids(recs - self, field):
             if not record_id:
@@ -5711,33 +5708,6 @@ PGERROR_TO_OE = defaultdict(
     '23505': convert_pgerror_unique,
 })
 
-def _normalize_ids(arg, atoms=set(IdType)):
-    """ Normalizes the ids argument for ``browse`` (v7 and v8) to a tuple.
-
-    Various implementations were tested on the corpus of all browse() calls
-    performed during a full crawler run (after having installed all website_*
-    modules) and this one was the most efficient overall.
-
-    A possible bit of correctness was sacrificed by not doing any test on
-    Iterable and just assuming that any non-atomic type was an iterable of
-    some kind.
-
-    :rtype: tuple
-    """
-    # much of the corpus is falsy objects (empty list, tuple or set, None)
-    if not arg:
-        return ()
-
-    # `type in set` is significantly faster (because more restrictive) than
-    # isinstance(arg, set) or issubclass(type, set); and for new-style classes
-    # obj.__class__ is equivalent to but faster than type(obj). Not relevant
-    # (and looks much worse) in most cases, but over millions of calls it
-    # does have a very minor effect.
-    if arg.__class__ in atoms:
-        return arg,
-
-    return tuple(arg)
-
 
 def lazy_name_get(self):
     """ Evaluate self.name_get() lazily. """