Merge pull request #4676 from odoo-dev/8.0-speedup_loading-rco

[IMP] models: speedup registry loading (25% less time)

The field setup on models is improved: only new-api fields are determined when building the model's class; the final _columns dict is derived from the fields once they are set up. This avoids setting up fields and columns at the same time, and finally overriding columns with the result of field setup.

This branch also fixes the model setup in general: with the former code, a model could be introspected while it was only partially set up, and not all its fields were known.

It incidentally fixes the "loss" of parameter partial that is necessary when loading custom fields. And during a partial setup, one2many custom fields are not introduced if their counterparts (model and many2one field) are not ready.

openerp/addons/base/ir/ir_model.py

@@ -388,9 +388,7 @@ class ir_model_fields(osv.osv):
                 if vals['model'].startswith('x_') and vals['name'] == 'x_name':
                     model._rec_name = 'x_name'
 
-                if self.pool.fields_by_model is not None:
+                self.pool.clear_manual_fields()
-                    cr.execute('SELECT * FROM ir_model_fields WHERE id=%s', (res,))
-                    self.pool.fields_by_model.setdefault(vals['model'], []).append(cr.dictfetchone())
 
                 # re-initialize model in registry
                 model.__init__(self.pool, cr)

openerp/fields.py

@@ -336,8 +336,6 @@ class Field(object):
         # determine self.default and cls._defaults in a consistent way
         self._determine_default(cls, name)
 
-        self.reset()
-
     def _determine_default(self, cls, name):
         """ Retrieve the default value for `self` in the hierarchy of `cls`, and
             determine `self.default` and `cls._defaults` accordingly.
@@ -403,33 +401,40 @@ class Field(object):
         self.inverse_fields = []
 
     def setup(self, env):
-        """ Complete the setup of `self` (dependencies, recomputation triggers,
+        """ Make sure that `self` is set up, except for recomputation triggers. """
-            and other properties). This method is idempotent: it has no effect
-            if `self` has already been set up.
-        """
         if not self.setup_done:
-            self._setup(env)
+            if self.related:
+                self._setup_related(env)
+            else:
+                self._setup_regular(env)
             self.setup_done = True
 
-    def _setup(self, env):
+    #
-        """ Do the actual setup of `self`. """
+    # Setup of non-related fields
-        if self.related:
+    #
-            self._setup_related(env)
+
+    def _setup_regular(self, env):
+        """ Setup the attributes of a non-related field. """
+        recs = env[self.model_name]
+
+        def make_depends(deps):
+            return tuple(deps(recs) if callable(deps) else deps)
+
+        # convert compute into a callable and determine depends
+        if isinstance(self.compute, basestring):
+            # if the compute method has been overridden, concatenate all their _depends
+            self.depends = ()
+            for method in resolve_all_mro(type(recs), self.compute, reverse=True):
+                self.depends += make_depends(getattr(method, '_depends', ()))
+            self.compute = getattr(type(recs), self.compute)
         else:
-            self._setup_regular(env)
+            self.depends = make_depends(getattr(self.compute, '_depends', ()))
 
-        # put invalidation/recomputation triggers on field dependencies
+        # convert inverse and search into callables
-        model = env[self.model_name]
+        if isinstance(self.inverse, basestring):
-        for path in self.depends:
+            self.inverse = getattr(type(recs), self.inverse)
-            self._setup_dependency([], model, path.split('.'))
+        if isinstance(self.search, basestring):
-
+            self.search = getattr(type(recs), self.search)
-        # put invalidation triggers on model dependencies
-        for dep_model_name, field_names in model._depends.iteritems():
-            dep_model = env[dep_model_name]
-            dep_model._setup_fields()
-            for field_name in field_names:
-                field = dep_model._fields[field_name]
-                field._triggers.add((self, None))
 
     #
     # Setup of related fields
@@ -445,7 +450,6 @@ class Field(object):
         recs = env[self.model_name]
         fields = []
         for name in self.related:
-            recs._setup_fields()
             field = recs._fields[name]
             field.setup(env)
             recs = recs[name]
@@ -524,31 +528,14 @@ class Field(object):
         return self.related_field if self.inherited else self
 
     #
-    # Setup of non-related fields
+    # Setup of field triggers
     #
 
-    def _setup_regular(self, env):
+    def setup_triggers(self, env):
-        """ Setup the attributes of a non-related field. """
+        """ Add the necessary triggers to invalidate/recompute `self`. """
-        recs = env[self.model_name]
+        model = env[self.model_name]
-
+        for path in self.depends:
-        def make_depends(deps):
+            self._setup_dependency([], model, path.split('.'))
-            return tuple(deps(recs) if callable(deps) else deps)
-
-        # convert compute into a callable and determine depends
-        if isinstance(self.compute, basestring):
-            # if the compute method has been overridden, concatenate all their _depends
-            self.depends = ()
-            for method in resolve_all_mro(type(recs), self.compute, reverse=True):
-                self.depends += make_depends(getattr(method, '_depends', ()))
-            self.compute = getattr(type(recs), self.compute)
-        else:
-            self.depends = make_depends(getattr(self.compute, '_depends', ()))
-
-        # convert inverse and search into callables
-        if isinstance(self.inverse, basestring):
-            self.inverse = getattr(type(recs), self.inverse)
-        if isinstance(self.search, basestring):
-            self.search = getattr(type(recs), self.search)
 
     def _setup_dependency(self, path0, model, path1):
         """ Make `self` depend on `model`; `path0 + path1` is a dependency of
@@ -558,7 +545,6 @@ class Field(object):
         env = model.env
         head, tail = path1[0], path1[1:]
 
-        model._setup_fields()
         if head == '*':
             # special case: add triggers on all fields of model (except self)
             fields = set(model._fields.itervalues()) - set([self])
@@ -571,8 +557,6 @@ class Field(object):
                 self.recursive = True
                 continue
 
-            field.setup(env)
-
             #_logger.debug("Add trigger on %s to recompute %s", field, self)
             field._triggers.add((self, '.'.join(path0 or ['id'])))
 
@@ -1050,8 +1034,8 @@ class Char(_String):
     type = 'char'
     size = None
 
-    def _setup(self, env):
+    def _setup_regular(self, env):
-        super(Char, self)._setup(env)
+        super(Char, self)._setup_regular(env)
         assert isinstance(self.size, (NoneType, int)), \
             "Char field %s with non-integer size %r" % (self, self.size)
 
@@ -1253,8 +1237,8 @@ class Selection(Field):
             selection = api.expected(api.model, selection)
         super(Selection, self).__init__(selection=selection, string=string, **kwargs)
 
-    def _setup(self, env):
+    def _setup_regular(self, env):
-        super(Selection, self)._setup(env)
+        super(Selection, self)._setup_regular(env)
         assert self.selection is not None, "Field %s without selection" % self
 
     def _setup_related(self, env):
@@ -1341,8 +1325,8 @@ class Reference(Selection):
     def __init__(self, selection=None, string=None, **kwargs):
         super(Reference, self).__init__(selection=selection, string=string, **kwargs)
 
-    def _setup(self, env):
+    def _setup_regular(self, env):
-        super(Reference, self)._setup(env)
+        super(Reference, self)._setup_regular(env)
         assert isinstance(self.size, (NoneType, int)), \
             "Reference field %s with non-integer size %r" % (self, self.size)
 
@@ -1378,8 +1362,8 @@ class _Relational(Field):
     domain = None                       # domain for searching values
     context = None                      # context for searching values
 
-    def _setup(self, env):
+    def _setup_regular(self, env):
-        super(_Relational, self)._setup(env)
+        super(_Relational, self)._setup_regular(env)
         if self.comodel_name not in env.registry:
             _logger.warning("Field %s with unknown comodel_name %r"
                             % (self, self.comodel_name))
@@ -1664,7 +1648,6 @@ class One2many(_RelationalMulti):
         if self.inverse_name:
             # link self to its inverse field and vice-versa
             comodel = env[self.comodel_name]
-            comodel._setup_fields()
             invf = comodel._fields[self.inverse_name]
             # In some rare cases, a `One2many` field can link to `Int` field
             # (res_model/res_id pattern). Only inverse the field if this is

openerp/models.py

@@ -481,11 +481,7 @@ class BaseModel(object):
         # basic setup of field
         field.set_class_name(cls, name)
 
-        if field.store or field.column:
+        # cls._columns will be updated once fields are set up
-            cls._columns[name] = field.to_column()
-        else:
-            # remove potential column that may be overridden by field
-            cls._columns.pop(name, None)
 
     @classmethod
     def _pop_field(cls, name):
@@ -605,13 +601,6 @@ class BaseModel(object):
             # inferred metadata; use its ancestor instead
             parent_class = type(parent_model).__base__
 
-            # don't inherit custom fields
-            columns = dict((key, val)
-                for key, val in parent_class._columns.iteritems()
-                if not val.manual
-            )
-            columns.update(cls._columns)
-
             inherits = dict(parent_class._inherits)
             inherits.update(cls._inherits)
 
@@ -635,7 +624,6 @@ class BaseModel(object):
             attrs = {
                 '_name': name,
                 '_register': False,
-                '_columns': columns,
                 '_inherits': inherits,
                 '_depends': depends,
                 '_constraints': constraints,
@@ -648,7 +636,7 @@ class BaseModel(object):
         attrs = {
             '_name': name,
             '_register': False,
-            '_columns': dict(cls._columns),
+            '_columns': {},             # filled by _setup_fields()
             '_defaults': {},            # filled by Field._determine_default()
             '_inherits': dict(cls._inherits),
             '_depends': dict(cls._depends),
@@ -705,16 +693,11 @@ class BaseModel(object):
                     pool._store_function[model].sort(key=lambda x: x[4])
 
     @classmethod
-    def _init_manual_fields(cls, cr, partial=False):
+    def _init_manual_fields(cls, cr, partial):
-        # Check whether the query is already done
+        manual_fields = cls.pool.get_manual_fields(cr, cls._name)
-        if cls.pool.fields_by_model is not None:
-            manual_fields = cls.pool.fields_by_model.get(cls._name, [])
-        else:
-            cr.execute('SELECT * FROM ir_model_fields WHERE model=%s AND state=%s', (cls._name, 'manual'))
-            manual_fields = cr.dictfetchall()
 
-        for field in manual_fields:
+        for name, field in manual_fields.iteritems():
-            if field['name'] in cls._fields:
+            if name in cls._fields:
                 continue
             attrs = {
                 'manual': True,
@@ -735,7 +718,11 @@ class BaseModel(object):
                 attrs['ondelete'] = field['on_delete']
                 attrs['domain'] = eval(field['domain']) if field['domain'] else None
             elif field['ttype'] == 'one2many':
-                if partial and field['relation'] not in cls.pool:
+                if partial and not (
+                    field['relation'] in cls.pool and (
+                        field['relation_field'] in cls.pool[field['relation']]._fields or
+                        field['relation_field'] in cls.pool.get_manual_fields(cr, field['relation'])
+                )):
                     continue
                 attrs['comodel_name'] = field['relation']
                 attrs['inverse_name'] = field['relation_field']
@@ -746,11 +733,11 @@ class BaseModel(object):
                 attrs['comodel_name'] = field['relation']
                 _rel1 = field['relation'].replace('.', '_')
                 _rel2 = field['model'].replace('.', '_')
-                attrs['relation'] = 'x_%s_%s_%s_rel' % (_rel1, _rel2, field['name'])
+                attrs['relation'] = 'x_%s_%s_%s_rel' % (_rel1, _rel2, name)
                 attrs['column1'] = 'id1'
                 attrs['column2'] = 'id2'
                 attrs['domain'] = eval(field['domain']) if field['domain'] else None
-            cls._add_field(field['name'], Field.by_type[field['ttype']](**attrs))
+            cls._add_field(name, Field.by_type[field['ttype']](**attrs))
 
     @classmethod
     def _init_constraints_onchanges(cls):
@@ -823,9 +810,6 @@ class BaseModel(object):
         # introduce magic fields
         cls._add_magic_fields()
 
-        # register stuff about low-level function fields and custom fields
-        cls._init_function_fields(pool, cr)
-
         # register constraints and onchange methods
         cls._init_constraints_onchanges()
 
@@ -2874,18 +2858,12 @@ class BaseModel(object):
     #
 
     @classmethod
-    def _inherits_reload(cls):
+    def _init_inherited_fields(cls):
-        """ Recompute the _inherit_fields mapping, and inherited fields. """
+        """ Determine inherited fields. """
-        struct = {}
+        # determine candidate inherited fields
         fields = {}
         for parent_model, parent_field in cls._inherits.iteritems():
             parent = cls.pool[parent_model]
-            # old-api struct for _inherit_fields
-            for name, column in parent._columns.iteritems():
-                struct[name] = (parent_model, parent_field, column, parent_model)
-            for name, source in parent._inherit_fields.iteritems():
-                struct[name] = (parent_model, parent_field, source[2], source[3])
-            # new-api fields for _fields
             for name, field in parent._fields.iteritems():
                 fields[name] = field.new(
                     inherited=True,
@@ -2893,15 +2871,25 @@ class BaseModel(object):
                     related_sudo=False,
                 )
 
-        # old-api stuff
-        cls._inherit_fields = struct
-        cls._all_columns = cls._get_column_infos()
-
         # add inherited fields that are not redefined locally
         for name, field in fields.iteritems():
             if name not in cls._fields:
                 cls._add_field(name, field)
 
+    @classmethod
+    def _inherits_reload(cls):
+        """ Recompute the _inherit_fields and _all_columns mappings. """
+        cls._inherit_fields = struct = {}
+        for parent_model, parent_field in cls._inherits.iteritems():
+            parent = cls.pool[parent_model]
+            for name, column in parent._columns.iteritems():
+                struct[name] = (parent_model, parent_field, column, parent_model)
+            for name, source in parent._inherit_fields.iteritems():
+                struct[name] = (parent_model, parent_field, source[2], source[3])
+
+        # old-api stuff
+        cls._all_columns = cls._get_column_infos()
+
     @classmethod
     def _get_column_infos(cls):
         """Returns a dict mapping all fields names (direct fields and
@@ -2918,14 +2906,16 @@ class BaseModel(object):
     @classmethod
     def _inherits_check(cls):
         for table, field_name in cls._inherits.items():
-            if field_name not in cls._columns:
+            field = cls._fields.get(field_name)
+            if not field:
                 _logger.info('Missing many2one field definition for _inherits reference "%s" in "%s", using default one.', field_name, cls._name)
-                cls._columns[field_name] = fields.many2one(table, string="Automatically created field to link to parent %s" % table,
+                from .fields import Many2one
-                                                             required=True, ondelete="cascade")
+                field = Many2one(table, string="Automatically created field to link to parent %s" % table, required=True, ondelete="cascade")
-            elif not cls._columns[field_name].required or cls._columns[field_name].ondelete.lower() not in ("cascade", "restrict"):
+                cls._add_field(field_name, field)
+            elif not field.required or field.ondelete.lower() not in ("cascade", "restrict"):
                 _logger.warning('Field definition for _inherits reference "%s" in "%s" must be marked as "required" with ondelete="cascade" or "restrict", forcing it to required + cascade.', field_name, cls._name)
-                cls._columns[field_name].required = True
+                field.required = True
-                cls._columns[field_name].ondelete = "cascade"
+                field.ondelete = "cascade"
 
         # reflect fields with delegate=True in dictionary cls._inherits
         for field in cls._fields.itervalues():
@@ -2938,44 +2928,48 @@ class BaseModel(object):
                 cls._inherits[field.comodel_name] = field.name
 
     @api.model
-    def _prepare_setup_fields(self):
+    def _prepare_setup(self):
-        """ Prepare the setup of fields once the models have been loaded. """
+        """ Prepare the setup of the model. """
         type(self)._setup_done = False
-        for name, field in self._fields.items():
-            if field.inherited:
-                del self._fields[name]
-            else:
-                field.reset()
 
     @api.model
-    def _setup_fields(self, partial=False):
+    def _setup_base(self, partial):
-        """ Setup the fields (dependency triggers, etc).
+        """ Determine the inherited and custom fields of the model. """
-
-            :param partial: ``True`` if all models have not been loaded yet.
-        """
         cls = type(self)
         if cls._setup_done:
             return
-        cls._setup_done = True
 
-        # first make sure that parent models are all set up
+        # first make sure that parent models determine all their fields
-        for parent in self._inherits:
+        cls._inherits_check()
-            self.env[parent]._setup_fields()
+        for parent in cls._inherits:
+            self.env[parent]._setup_base(partial)
+
+        # remove inherited fields from cls._fields
+        for name, field in cls._fields.items():
+            if field.inherited:
+                del cls._fields[name]
 
         # retrieve custom fields
-        cls._init_manual_fields(self._cr, partial=partial)
+        cls._init_manual_fields(self._cr, partial)
 
         # retrieve inherited fields
-        cls._inherits_check()
+        cls._init_inherited_fields()
-        cls._inherits_reload()
 
-        # set up fields
+        # prepare the setup of fields
         for field in cls._fields.itervalues():
-            field.setup(self.env)
+            field.reset()
 
-        # update columns (fields may have changed)
+        cls._setup_done = True
+
+    @api.model
+    def _setup_fields(self):
+        """ Setup the fields, except for recomputation triggers. """
+        cls = type(self)
+
+        # set up fields, and update their corresponding columns
         for name, field in cls._fields.iteritems():
-            if field.column:
+            field.setup(self.env)
+            if field.store or field.column:
                 cls._columns[name] = field.to_column()
 
         # group fields by compute to determine field.computed_fields
@@ -2987,6 +2981,30 @@ class BaseModel(object):
             else:
                 field.computed_fields = []
 
+    @api.model
+    def _setup_complete(self):
+        """ Setup recomputation triggers, and complete the model setup. """
+        cls = type(self)
+
+        # set up field triggers
+        for field in cls._fields.itervalues():
+            field.setup_triggers(self.env)
+
+        # add invalidation triggers on model dependencies
+        if cls._depends:
+            triggers = [(field, None) for field in cls._fields.itervalues()]
+            for model_name, field_names in cls._depends.iteritems():
+                model = self.env[model_name]
+                for field_name in field_names:
+                    field = model._fields[field_name]
+                    field._triggers.update(triggers)
+
+        # determine old-api cls._inherit_fields and cls._all_columns
+        cls._inherits_reload()
+
+        # register stuff about low-level function fields
+        cls._init_function_fields(cls.pool, self._cr)
+
         # check constraints
         for func in cls._constraint_methods:
             if not all(name in cls._fields for name in func._constrains):

openerp/modules/loading.py

@@ -126,13 +126,7 @@ def load_module_graph(cr, graph, status=None, perform_checks=True, skip_modules=
     migrations = openerp.modules.migration.MigrationManager(cr, graph)
     _logger.info('loading %d modules...', len(graph))
 
-    # Query manual fields for all models at once and save them on the registry
+    registry.clear_manual_fields()
-    # so the initialization code for each model does not have to do it
-    # one model at a time.
-    registry.fields_by_model = {}
-    cr.execute('SELECT * FROM ir_model_fields WHERE state=%s', ('manual',))
-    for field in cr.dictfetchall():
-        registry.fields_by_model.setdefault(field['model'], []).append(field)
 
     # register, instantiate and initialize models for each modules
     t0 = time.time()
@@ -162,13 +156,6 @@ def load_module_graph(cr, graph, status=None, perform_checks=True, skip_modules=
             registry.setup_models(cr, partial=True)
             init_module_models(cr, package.name, models)
 
-        # Can't put this line out of the loop: ir.module.module will be
-        # registered by init_module_models() above.
-        modobj = registry['ir.module.module']
-
-        if perform_checks:
-            modobj.check(cr, SUPERUSER_ID, [module_id])
-
         idref = {}
 
         mode = 'update'
@@ -176,6 +163,13 @@ def load_module_graph(cr, graph, status=None, perform_checks=True, skip_modules=
             mode = 'init'
 
         if hasattr(package, 'init') or hasattr(package, 'update') or package.state in ('to install', 'to upgrade'):
+            # Can't put this line out of the loop: ir.module.module will be
+            # registered by init_module_models() above.
+            modobj = registry['ir.module.module']
+
+            if perform_checks:
+                modobj.check(cr, SUPERUSER_ID, [module_id])
+
             if package.state=='to upgrade':
                 # upgrading the module information
                 modobj.write(cr, SUPERUSER_ID, [module_id], modobj.get_values_from_terp(package.data))
@@ -227,9 +221,7 @@ def load_module_graph(cr, graph, status=None, perform_checks=True, skip_modules=
 
     _logger.log(25, "%s modules loaded in %.2fs, %s queries", len(graph), time.time() - t0, openerp.sql_db.sql_counter - t0_sql)
 
-    # The query won't be valid for models created later (i.e. custom model
+    registry.clear_manual_fields()
-    # created after the registry has been loaded), so empty its result.
-    registry.fields_by_model = None
 
     cr.commit()
 

openerp/modules/registry.py

@@ -22,7 +22,7 @@
 """ Models registries.
 
 """
-from collections import Mapping
+from collections import Mapping, defaultdict
 import logging
 import os
 import threading
@@ -51,7 +51,7 @@ class Registry(Mapping):
         self._init = True
         self._init_parent = {}
         self._assertion_report = assertion_report.assertion_report()
-        self.fields_by_model = None
+        self._fields_by_model = None
 
         # modules fully loaded (maintained during init phase by `loading` module)
         self._init_modules = set()
@@ -114,6 +114,20 @@ class Registry(Mapping):
                 fields.append(model_fields[fname])
         return fields
 
+    def clear_manual_fields(self):
+        """ Invalidate the cache for manual fields. """
+        self._fields_by_model = None
+
+    def get_manual_fields(self, cr, model_name):
+        """ Return the manual fields (as a dict) for the given model. """
+        if self._fields_by_model is None:
+            # Query manual fields for all models at once
+            self._fields_by_model = dic = defaultdict(dict)
+            cr.execute('SELECT * FROM ir_model_fields WHERE state=%s', ('manual',))
+            for field in cr.dictfetchall():
+                dic[field['model']][field['name']] = field
+        return self._fields_by_model[model_name]
+
     def do_parent_store(self, cr):
         for o in self._init_parent:
             self.get(o)._parent_store_compute(cr)
@@ -166,12 +180,18 @@ class Registry(Mapping):
 
         # prepare the setup on all models
         for model in self.models.itervalues():
-            model._prepare_setup_fields(cr, SUPERUSER_ID)
+            model._prepare_setup(cr, SUPERUSER_ID)
 
         # do the actual setup from a clean state
         self._m2m = {}
         for model in self.models.itervalues():
-            model._setup_fields(cr, SUPERUSER_ID, partial=partial)
+            model._setup_base(cr, SUPERUSER_ID, partial)
+
+        for model in self.models.itervalues():
+            model._setup_fields(cr, SUPERUSER_ID)
+
+        for model in self.models.itervalues():
+            model._setup_complete(cr, SUPERUSER_ID)
 
     def clear_caches(self):
         """ Clear the caches