From e038fec696ea7029b2f66da36fdbf337c23a18d1 Mon Sep 17 00:00:00 2001 From: Raphael Collet Date: Thu, 6 Nov 2014 15:03:00 +0100 Subject: [PATCH] [IMP] models: improve performance of _setup_fields() There was an issue in _setup_fields(): the method invokes _inherits_reload(), which recomputes inherited fields, and invokes itself recursively on children models. This may be problematic if the children models have already been set up. This optimization avoids recursive calls of method _inherits_reload(). In _setup_fields(), first all parent models are set up, then their fields are inspected to determine inherited fields, and their setup is done. This scheme guarantees that inherited fields are computed once per model. --- openerp/fields.py | 10 ++- openerp/models.py | 151 +++++++++++++++++++----------------- openerp/modules/registry.py | 3 +- 3 files changed, 86 insertions(+), 78 deletions(-) diff --git a/openerp/fields.py b/openerp/fields.py index 58838bf0531..43b63f84b13 100644 --- a/openerp/fields.py +++ b/openerp/fields.py @@ -426,6 +426,7 @@ class Field(object): # 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)) @@ -444,8 +445,8 @@ 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] fields.append(field) @@ -551,6 +552,7 @@ 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]) @@ -563,8 +565,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']))) @@ -1648,7 +1648,9 @@ class One2many(_RelationalMulti): if self.inverse_name: # link self to its inverse field and vice-versa - invf = env[self.comodel_name]._fields[self.inverse_name] + 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 # a `Many2one` field. diff --git a/openerp/models.py b/openerp/models.py index 02be95568f5..df46fce4d5d 100644 --- a/openerp/models.py +++ b/openerp/models.py @@ -700,10 +700,10 @@ class BaseModel(object): pool._store_function[model].sort(key=lambda x: x[4]) @classmethod - def _init_manual_fields(cls, pool, cr): + def _init_manual_fields(cls, cr): # Check whether the query is already done - if pool.fields_by_model is not None: - manual_fields = pool.fields_by_model.get(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() @@ -752,12 +752,8 @@ class BaseModel(object): cls._onchange_methods = defaultdict(list) for attr, func in getmembers(cls, callable): if hasattr(func, '_constrains'): - if not all(name in cls._fields for name in func._constrains): - _logger.warning("@constrains%r parameters must be field names", func._constrains) cls._constraint_methods.append(func) if hasattr(func, '_onchange'): - if not all(name in cls._fields for name in func._onchange): - _logger.warning("@onchange%r parameters must be field names", func._onchange) for name in func._onchange: cls._onchange_methods[name].append(func) @@ -818,31 +814,10 @@ class BaseModel(object): # register stuff about low-level function fields and custom fields cls._init_function_fields(pool, cr) - cls._init_manual_fields(pool, cr) - - # process _inherits - cls._inherits_check() - cls._inherits_reload() # register constraints and onchange methods cls._init_constraints_onchanges() - # check defaults - for k in cls._defaults: - assert k in cls._fields, \ - "Model %s has a default for nonexiting field %s" % (cls._name, k) - - # restart columns - for column in cls._columns.itervalues(): - column.restart() - - # validate rec_name - if cls._rec_name: - assert cls._rec_name in cls._fields, \ - "Invalid rec_name %s for model %s" % (cls._rec_name, cls._name) - elif 'name' in cls._fields: - cls._rec_name = 'name' - # prepare ormcache, which must be shared by all instances of the model cls._ormcache = {} @@ -2884,42 +2859,34 @@ class BaseModel(object): # Update objects that uses this one to update their _inherits fields # - @classmethod - def _inherits_reload_src(cls): - """ Recompute the _inherit_fields mapping on each _inherits'd child model.""" - for model in cls.pool.values(): - if cls._name in model._inherits: - model._inherits_reload() - @classmethod def _inherits_reload(cls): - """ Recompute the _inherit_fields mapping. + """ Recompute the _inherit_fields mapping, and inherited fields. """ + struct = {} + 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, + related=(parent_field, name), + related_sudo=False, + ) - This will also call itself on each inherits'd child model. - - """ - res = {} - for table in cls._inherits: - other = cls.pool[table] - for col in other._columns.keys(): - res[col] = (table, cls._inherits[table], other._columns[col], table) - for col in other._inherit_fields.keys(): - res[col] = (table, cls._inherits[table], other._inherit_fields[col][2], other._inherit_fields[col][3]) - cls._inherit_fields = res + # old-api stuff + cls._inherit_fields = struct cls._all_columns = cls._get_column_infos() - # interface inherited fields with new-style fields (note that the - # reverse order is for being consistent with _all_columns above) - for parent_model, parent_field in reversed(cls._inherits.items()): - for attr, field in cls.pool[parent_model]._fields.iteritems(): - if attr not in cls._fields: - cls._add_field(attr, field.new( - inherited=True, - related=(parent_field, attr), - related_sudo=False, - )) - - cls._inherits_reload_src() + # 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 _get_column_infos(cls): @@ -2959,34 +2926,72 @@ class BaseModel(object): @api.model def _prepare_setup_fields(self): """ Prepare the setup of fields once the models have been loaded. """ - for field in self._fields.itervalues(): - field.reset() + 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_fields(self): """ Setup the fields (dependency triggers, etc). """ - for field in self._fields.itervalues(): - try: - field.setup(self.env) - except Exception: - if not partial: - raise + cls = type(self) + if cls._setup_done: + return + cls._setup_done = True - # update columns (fields may have changed), and column_infos - for name, field in self._fields.iteritems(): + # first make sure that parent models are all set up + for parent in self._inherits: + self.env[parent]._setup_fields() + + # retrieve custom fields + if not self._context.get('_setup_fields_partial'): + cls._init_manual_fields(self._cr) + + # retrieve inherited fields + cls._inherits_check() + cls._inherits_reload() + + # set up fields + for field in cls._fields.itervalues(): + field.setup(self.env) + + # update columns (fields may have changed) + for name, field in cls._fields.iteritems(): if field.column: - self._columns[name] = field.to_column() - self._inherits_reload() + cls._columns[name] = field.to_column() # group fields by compute to determine field.computed_fields fields_by_compute = defaultdict(list) - for field in self._fields.itervalues(): + for field in cls._fields.itervalues(): if field.compute: field.computed_fields = fields_by_compute[field.compute] field.computed_fields.append(field) else: field.computed_fields = [] + # check constraints + for func in cls._constraint_methods: + if not all(name in cls._fields for name in func._constrains): + _logger.warning("@constrains%r parameters must be field names", func._constrains) + for name in cls._onchange_methods: + if name not in cls._fields: + func = cls._onchange_methods[name] + _logger.warning("@onchange%r parameters must be field names", func._onchange) + + # check defaults + for name in cls._defaults: + assert name in cls._fields, \ + "Model %s has a default for nonexiting field %s" % (cls._name, name) + + # validate rec_name + if cls._rec_name: + assert cls._rec_name in cls._fields, \ + "Invalid rec_name %s for model %s" % (cls._rec_name, cls._name) + elif 'name' in cls._fields: + cls._rec_name = 'name' + def fields_get(self, cr, user, allfields=None, context=None, write_access=True): """ fields_get([fields]) diff --git a/openerp/modules/registry.py b/openerp/modules/registry.py index 34672c44a78..7cf734d792e 100644 --- a/openerp/modules/registry.py +++ b/openerp/modules/registry.py @@ -164,8 +164,9 @@ class Registry(Mapping): # do the actual setup from a clean state self._m2m = {} + context = {'_setup_fields_partial': partial} for model in self.models.itervalues(): - model._setup_fields(cr, SUPERUSER_ID, partial=partial) + model._setup_fields(cr, SUPERUSER_ID, context=context) def clear_caches(self): """ Clear the caches