[FIX] core: avoid quadratic complexity for partial compute methods
For a compute store field with new records (e.g. during an 'onchange'),
the compute method can be called multiple times on the same records
without changing the dependencies. Moreover, it can lead to have N² / 2
complexity for a trivial compute on N records.
With partial (where we don't always change the value) compute method:
```
@api.depends('reward')
def _compute_has_been_rewarded(self):
for rec in self:
if rec.reward:
rec.has_been_rewarded = 'Yes'
```
If every `reward` of `self` (N records) is `False`, when the ORM needs
to recompute `has_been_rewarded` of `self`: the compute will be batched,
but only the first record in the batch will be set (to `False`) each
time (due to the current fallback - "fallback to null value if compute
gives nothing"). This means that we will call the compute method N
times, and the compute itself will loop on an average of N/2 records
(the prefetch set decreasing at each step).
Fix this quadratic behavior by setting the cache to `False` for every
record not set during the compute method (instead of just the current
record).
closes odoo/odoo#142162
X-original-commit: 1604ee983aadc0cbee0cd50cbea2b09572905b04
Signed-off-by: Raphael Collet <rco@odoo.com>
Signed-off-by: Rémy Voet (ryv) <ryv@odoo.com>
This commit is contained in:
@@ -562,6 +562,13 @@ class OrderLine(models.Model):
|
||||
short_field_name = fields.Integer(index=True)
|
||||
very_very_very_very_very_long_field_name_1 = fields.Integer(index=True)
|
||||
very_very_very_very_very_long_field_name_2 = fields.Integer(index=True)
|
||||
has_been_rewarded = fields.Char(compute='_compute_has_been_rewarded', store=True)
|
||||
|
||||
@api.depends('reward')
|
||||
def _compute_has_been_rewarded(self):
|
||||
for rec in self:
|
||||
if rec.reward:
|
||||
rec.has_been_rewarded = 'Yes'
|
||||
|
||||
def unlink(self):
|
||||
# also delete associated reward lines
|
||||
|
||||
@@ -9,6 +9,7 @@ from collections import OrderedDict
|
||||
from datetime import date, datetime, time
|
||||
import io
|
||||
from PIL import Image
|
||||
from unittest.mock import patch
|
||||
import psycopg2
|
||||
|
||||
from odoo import models, fields, Command
|
||||
@@ -4360,6 +4361,24 @@ class TestComputeQueries(common.TransactionCase):
|
||||
self.assertEqual(records.mapped('value1'), [10, 0, 0, 0])
|
||||
self.assertEqual(records.mapped('value2'), [0, 12, 0, 0])
|
||||
|
||||
def test_partial_compute_batching(self):
|
||||
""" Create several 'new' records and check that the partial compute
|
||||
method is called only once.
|
||||
"""
|
||||
order = self.env['test_new_api.order'].new({
|
||||
'line_ids': [Command.create({'reward': False})] * 100,
|
||||
})
|
||||
|
||||
OrderLine = self.env.registry['test_new_api.order.line']
|
||||
with patch.object(
|
||||
OrderLine,
|
||||
'_compute_has_been_rewarded',
|
||||
side_effect=OrderLine._compute_has_been_rewarded,
|
||||
autospec=True,
|
||||
) as patch_compute:
|
||||
order.line_ids.mapped('has_been_rewarded')
|
||||
self.assertEqual(patch_compute.call_count, 1)
|
||||
|
||||
|
||||
class test_shared_cache(TransactionCaseWithUserDemo):
|
||||
def test_shared_cache_computed_field(self):
|
||||
|
||||
+11
-7
@@ -1205,14 +1205,18 @@ class Field(MetaField('DummyField', (object,), {})):
|
||||
self.compute_value(recs)
|
||||
except (AccessError, MissingError):
|
||||
self.compute_value(record)
|
||||
try:
|
||||
value = env.cache.get(record, self)
|
||||
except CacheMiss:
|
||||
recs = record
|
||||
|
||||
missing_recs_ids = tuple(env.cache.get_missing_ids(recs, self))
|
||||
if missing_recs_ids:
|
||||
missing_recs = record.browse(missing_recs_ids)
|
||||
if self.readonly and not self.store:
|
||||
raise ValueError(f"Compute method failed to assign {record}.{self.name}") from None
|
||||
# fallback to null value if compute gives nothing
|
||||
value = self.convert_to_cache(False, record, validate=False)
|
||||
env.cache.set(record, self, value)
|
||||
raise ValueError(f"Compute method failed to assign {missing_recs}.{self.name}")
|
||||
# fallback to null value if compute gives nothing, do it for every unset record
|
||||
false_value = self.convert_to_cache(False, record, validate=False)
|
||||
env.cache.update(missing_recs, self, itertools.repeat(false_value))
|
||||
|
||||
value = env.cache.get(record, self)
|
||||
|
||||
elif self.type == 'many2one' and self.delegate and not record.id:
|
||||
# parent record of a new record: new record, with the same
|
||||
|
||||
Reference in New Issue
Block a user