From c3216db4c14ac9a34d798de087797192fe2dec35 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Fri, 17 Aug 2018 16:16:13 +0200 Subject: [PATCH] [IMP] fields: performance of creating with m2m fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note: all costs are according to cprofile on my machine, as a fraction of total time to import, python-flamegraph has somewhat different cost distributions, YMMV. Profiles use test-import of a 2500 partners file, with name, country, street and 0~4 tags, all randomly generated/assigned. Costs were profiled as split 8.5%/32%/57% between data conversion, creating records (Model.create/Model._create) and populating stored computed fields for a total import time (unprofiled) of 1mn (±2s or something). For the creation step, 27% (of total runtime) were assigned to the m2m (fields.Many2many.create), almost all of which (25%) was assigned to accessing a record's id in a listcomp in a loop. Optimisations: * optimise the case where we're only adding / creating values: represent the relation as a dict of sets instead of a set of pairs; commands 5 and 6 will only operate on the current record's value, instead of traversing all pairs in the relation * optimise accessing the value of an id field by hand-inlining the various bits, this is mostly obviated by the other previous optimisations (number of Id.__get__ calls reduced from 5.4 millions to 0.55) but given how common accessing a record's id is (and how it's usually considered to be free) it's still a good idea: cost of accessing the id field was reduced by 75% (0.55 million accesses went from 3.32% of total runtime to 0.83%) Without profiling, time to import my test file went from ~1mn to ~45s. --- odoo/fields.py | 31 ++++++++++++++++++------------- 1 file changed, 18 insertions(+), 13 deletions(-) diff --git a/odoo/fields.py b/odoo/fields.py index 650eaebb799..19fed759435 100644 --- a/odoo/fields.py +++ b/odoo/fields.py @@ -2629,8 +2629,8 @@ class Many2many(_RelationalMulti): model = record_values[0][0] comodel = model.env[self.comodel_name] - # determine links (set of pairs (id1, id2)) - links = set() + # determine relation {id1: ids2} + rel_ids = {record.id: set() for record, value in record_values} recs, vals_list = [], [] def flush(): @@ -2638,12 +2638,12 @@ class Many2many(_RelationalMulti): if vals_list: lines = comodel.create(vals_list) for rec, line in pycompat.izip(recs, lines): - links.add((rec.id, line.id)) + rel_ids[rec.id].add(line.id) recs.clear() vals_list.clear() for record, value in record_values: - for act in (value or []): + for act in value or []: if not isinstance(act, (list, tuple)) or not act: continue if act[0] == 0: @@ -2653,23 +2653,22 @@ class Many2many(_RelationalMulti): comodel.browse(act[1]).write(act[2]) elif act[0] == 2: comodel.browse(act[1]).unlink() - links.discard((record.id, act[1])) + rel_ids[record.id].discard(act[1]) elif act[0] == 3: - links.discard((record.id, act[1])) + rel_ids[record.id].discard(act[1]) elif act[0] == 4: - links.add((record.id, act[1])) + rel_ids[record.id].add(act[1]) elif act[0] in (5, 6): if recs and recs[-1] == record: flush() - # must reify the RHS as Python can update the subject on - # the fly leading to "RuntimeError: Set changed size during iteration" - links.difference_update([pair for pair in links if pair[0] == record.id]) + rel_ids[record.id].clear() if act[0] == 6: - links.update((record.id, id2) for id2 in act[2]) + rel_ids[record.id].update(act[2]) flush() # add links + links = [(id1, id2) for id1, ids2 in rel_ids.items() for id2 in ids2] if links: query = """ INSERT INTO {rel} ({id1}, {id2}) VALUES {values} @@ -2782,9 +2781,15 @@ class Id(Field): def __get__(self, record, owner): if record is None: return self # the field is accessed through the class owner - if not record: + + # the code below is written to make record.id as quick as possible + ids = record._ids + size = len(ids) + if size is 0: return False - return record.ensure_one()._ids[0] + elif size is 1: + return ids[0] + raise ValueError("Expected singleton: %s" % record) def __set__(self, record, value): raise TypeError("field 'id' cannot be assigned")