From 957c78b98f172cfb377acd0dbd707aeb3f83d4a9 Mon Sep 17 00:00:00 2001 From: Adrian Torres Date: Mon, 26 Aug 2019 18:53:57 +0000 Subject: [PATCH] [IMP] base: optimize create on models with property fields Before this commit, creating a record of a model with at least one property/company_dependent field would trigger the field's inverse method regardless of whether an actual value was being passed in for the field or not. This means that if no value was given or if the value was the same as the default value, we would waste precious time in the property field's inverse method. With this commit, we do not call the inverse method if: 1) there is no value for it in the vals dict (use default) 2) the value in the vals dict is the same as the default value As an example, when creating a res.partner record while having account installed (which introduces a property field in res.partner), the creation time goes from 24ms to 14ms. closes odoo/odoo#36267 Signed-off-by: Raphael Collet (rco) --- odoo/addons/test_performance/models/models.py | 16 ++++ .../security/ir.model.access.csv | 2 + .../tests/test_performance.py | 76 ++++++++++++++++++- odoo/models.py | 9 +++ 4 files changed, 102 insertions(+), 1 deletion(-) diff --git a/odoo/addons/test_performance/models/models.py b/odoo/addons/test_performance/models/models.py index ba38983234f..aecca064ee9 100644 --- a/odoo/addons/test_performance/models/models.py +++ b/odoo/addons/test_performance/models/models.py @@ -39,8 +39,24 @@ class LineModel(models.Model): # line values should be unique per "base" - useful for testing corner cases with unique lines tools.create_unique_index(self._cr, 'test_performance_line_uniq', self._table, ['base_id', 'value']) + class TagModel(models.Model): _name = 'test_performance.tag' _description = 'Test Performance Tag' name = fields.Char() + + +class Bacon(models.Model): + _name = 'test_performance.bacon' + _description = 'Test Performance Bacon' + + property_eggs = fields.Many2one( + 'test_performance.eggs', company_dependent=True, string='Eggs') + + +class Eggs(models.Model): + _name = 'test_performance.eggs' + _description = 'Test Performance Eggs' + + name = fields.Char() diff --git a/odoo/addons/test_performance/security/ir.model.access.csv b/odoo/addons/test_performance/security/ir.model.access.csv index 765c59168c8..510f23fa10f 100644 --- a/odoo/addons/test_performance/security/ir.model.access.csv +++ b/odoo/addons/test_performance/security/ir.model.access.csv @@ -2,3 +2,5 @@ id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink access_test_performance_base,access_test_performance_base,model_test_performance_base,,1,1,1,1 access_test_performance_line,access_test_performance_line,model_test_performance_line,,1,1,1,1 access_test_performance_tag,access_test_performance_tag,model_test_performance_tag,,1,1,1,1 +access_test_performance_bacon,access_test_performance_bacon,model_test_performance_bacon,,1,1,1,1 +access_test_performance_eggs,access_test_performance_eggs,model_test_performance_eggs,,1,1,1,1 diff --git a/odoo/addons/test_performance/tests/test_performance.py b/odoo/addons/test_performance/tests/test_performance.py index 4b0dd4fd1bb..bfd2cdcee37 100644 --- a/odoo/addons/test_performance/tests/test_performance.py +++ b/odoo/addons/test_performance/tests/test_performance.py @@ -4,7 +4,7 @@ from collections import defaultdict import json -from odoo.tests.common import TransactionCase, users, warmup +from odoo.tests.common import TransactionCase, users, warmup, tagged from odoo.tools import mute_logger @@ -414,3 +414,77 @@ class TestPerformance(TransactionCase): # now serialize to json, which should force evaluation with self.assertQueryCount(__system__=1, demo=1): json.dumps(result) + + +@tagged('bacon_and_eggs') +class TestIrPropertyOptimizations(TransactionCase): + + def setUp(self): + super().setUp() + self.Bacon = self.env['test_performance.bacon'] + self.Eggs = self.env['test_performance.eggs'] + + def test_with_falsy_default(self): + self.assertFalse(self.env['ir.property'].get('property_eggs', 'test_performance.bacon')) + + # warmup + eggs = self.Eggs.create({}) + self.Bacon.create({}) + self.Bacon.create({'property_eggs': eggs.id}) + + # create with default value + with self.assertQueryCount(1): + self.Bacon.create({}) + + with self.assertQueryCount(1): + self.Bacon.with_context(default_property_eggs=False).create({}) + + with self.assertQueryCount(1): + self.Bacon.create({'property_eggs': False}) + + # create with another value + with self.assertQueryCount(5): + self.Bacon.with_context(default_property_eggs=eggs.id).create({}) + + with self.assertQueryCount(5): + self.Bacon.create({'property_eggs': eggs.id}) + + def test_with_truthy_default(self): + eggs = self.Eggs.create({}) + field = self.env['ir.model.fields']._get('test_performance.bacon', 'property_eggs') + self.env['ir.property'].create({ + 'name': 'property_eggs_with_bacon', + 'fields_id': field.id, + 'value': eggs, + }) + + self.assertEqual(eggs, self.env['ir.property'].get('property_eggs', 'test_performance.bacon')) + + # warmup + self.Bacon.create({}) + + # create with default value + with self.assertQueryCount(1): + self.Bacon.create({}) + + with self.assertQueryCount(1): + self.Bacon.with_context(default_property_eggs=eggs.id).create({}) + + with self.assertQueryCount(1): + self.Bacon.create({'property_eggs': eggs.id}) + + # create with another value + eggs = self.Eggs.create({}) + self.Bacon.create({'property_eggs': eggs.id}) + + with self.assertQueryCount(5): + self.Bacon.with_context(default_property_eggs=eggs.id).create({}) + + with self.assertQueryCount(5): + self.Bacon.create({'property_eggs': eggs.id}) + + with self.assertQueryCount(5): + self.Bacon.with_context(default_property_eggs=False).create({}) + + with self.assertQueryCount(5): + self.Bacon.create({'property_eggs': False}) diff --git a/odoo/models.py b/odoo/models.py index 6e4264d1a69..2914fc93685 100644 --- a/odoo/models.py +++ b/odoo/models.py @@ -3500,6 +3500,15 @@ Fields: if not field: _logger.warning("%s.create() with unknown fields: %s", self._name, key) continue + if field.company_dependent: + cached_def = field.convert_to_cache(field.default(self), self) + cached_val = field.convert_to_cache(val, self) + if cached_val == cached_def: + # val is the same as the default value defined in + # 'ir.property'; by design, 'ir.property' will not + # create entries specific to these records; skipping the + # field inverse saves 4 SQL queries + continue if field.store: stored[key] = val if field.inherited: