From f14ae73a98ecb40c102c8a22729df4ceb44802e9 Mon Sep 17 00:00:00 2001
From: Raphael Collet <rco@odoo.com>
Date: Thu, 14 Mar 2019 14:45:53 +0000
Subject: [PATCH] [FIX] models: do less snapshots in onchange()

As snapshots are costly to compute, we now create exactly two snapshots:
one initial snapshot and one final snapshot.  All intermediate checks
are done with respect to the initial snapshot.

Note also that checks are costly for x2many fields.  Therefore, once a
field has changed, it will never be checked again.  This optimization is
correct since, by contract, the onchanges of a field are processed at
most once.  This guarantees the absence of cycles in onchange.

For instance, assume that field A modifies fields B and C, and that
field C modifies fields D, E and F:
- before: 7 snapshots (snapshot, process A, snapshot, process B,
snapshot, process C, snapshot, process D, snapshot, process E,
snapshot, process F, snapshot)
- after: 2 snapshots (snapshot, process A, process B, process C,
process D, process E, process F, snapshot)

closes odoo/odoo#31872

Signed-off-by: Raphael Collet (rco) <rco@openerp.com>
---
 odoo/models.py | 44 ++++++++++++++++++++++++++++----------------
 1 file changed, 28 insertions(+), 16 deletions(-)

diff --git a/odoo/models.py b/odoo/models.py
index bedf7fec10e..ab958f62644 100644
--- a/odoo/models.py
+++ b/odoo/models.py
@@ -5105,6 +5105,19 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
                         if subnames else record[name]
                     )
 
+            def has_changed(self, name):
+                """ Return whether a field on record has changed. """
+                record = self['<record>']
+                subnames = self['<tree>'][name]
+                if not subnames:
+                    return self[name] != record[name]
+                else:
+                    return len(self[name]) != len(record[name]) or any(
+                        line_snapshot.has_changed(subname)
+                        for line_snapshot in self[name]
+                        for subname in subnames
+                    )
+
             def diff(self, other):
                 """ Return the values in ``self`` that differ from ``other``.
                     Requires record cache invalidation for correct output!
@@ -5182,25 +5195,24 @@ class BaseModel(MetaModel('DummyModel', (object,), {'_register': False})):
 
         result = {}
 
-        # process names in order (or the keys of values if no name given)
-        while todo:
-            name = todo.pop(0)
-            if name in done:
-                continue
-            done.add(name)
-
-            with env.do_in_onchange():
+        # process names in order
+        with env.do_in_onchange():
+            while todo:
                 # apply field-specific onchange methods
-                if field_onchange.get(name):
-                    record._onchange_eval(name, field_onchange[name], result)
+                for name in todo:
+                    if field_onchange.get(name):
+                        record._onchange_eval(name, field_onchange[name], result)
+                    done.add(name)
 
-                # make a snapshot (this forces evaluation of computed fields)
-                snapshot1 = Snapshot(record, nametree)
+                # determine which fields to process for the next pass
+                todo = [
+                    name
+                    for name in nametree
+                    if name not in done and snapshot0.has_changed(name)
+                ]
 
-                # determine which fields have been modified
-                for name in nametree:
-                    if snapshot1[name] != snapshot0[name]:
-                        todo.append(name)
+            # make the snapshot with the final values of record
+            snapshot1 = Snapshot(record, nametree)
 
         # determine values that have changed by comparing snapshots
         self.invalidate_cache()