[IMP] base: view validation performance, batch action and group exist

The main goal is to reduce the processing time
it takes for views to be validated during installation.

In order to mimic the behavior of the view creations
during installation, and to analyze the performance,
I used the below code in the shell:

```py
env.cr.rollback()
domain = [('model', '=', 'res.config.settings')]
views = self.env['ir.ui.view'].search_read(domain, [
    'id', 'active', 'arch', 'inherit_id', 'key', 'mode', 'model', 'priority', 'type', 'xml_id'
], order="id ASC")
for view in views:
    if view['inherit_id']:
        view['inherit_id'] = self.env['ir.ui.view'].browse(view['inherit_id'][0]).xml_id

self.env['ir.ui.view'].browse(reversed([view['id'] for view in views])).unlink()
with odoo.tools.profiler.Profiler():
    for view in views:
        new_view = self.env['ir.ui.view'].create({
            key: self.env.ref(value).id if key in ['inherit_id'] and value else value
            for key, value in view.items()
            if key not in ['id', 'xml_id']
        })
        if view['xml_id']:
            module, name = view['xml_id'].split('.')
            self.env['ir.model.data'].create({
                'module': module,
                'name': name,
                'model': 'ir.ui.view',
                'res_id': new_view.id,
            })
env.cr.rollback()
```

Using the above, and analyzing the speedscope profile,
we can see the lines

```py
action = self.env['ir.actions.actions'].browse(action_id).exists()
```
from `_validate_tag_button`, and

```py
if not self.env['ir.model.data']._xmlid_to_res_id(group.strip(), raise_if_not_found=False):
```
from `_validate_attrs`

take a significant time.

Batching them the actions and groups existence
(as recommended as further improvement in the comment which is deleted by this revision),
as well as a more efficient algorithm for `_get_node_groups`,
reduce the processing time of the views validation.

On my computer, the above code creating all the views for `res.config.settings`,
which is an extreme case because the view has 100 inherited views,
allows to reduce the validation procesing time from 9.42s to 8.18s.

I include in the pull request the speedscope profiles screenshots.

Regarding the algorithm for `_get_node_groups` itself:
```py
In [0]: %timeit tuple(tuple(n.get('groups').split(',')) for n in node.xpath('ancestor-or-self::*[@groups]'))
10.1 µs ± 91 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)

In [1]: %timeit groups = [tuple(n.get('groups').split(',')) for n in chain([node], node.iterancestors()) if n.get('groups')];groups.reverse();tuple(groups)
3.46 µs ± 29.2 ns per loop (mean ± std. dev. of 7 runs, 100000 loops each)
```

closes odoo/odoo#107742

Signed-off-by: Denis Ledoux (dle) <dle@odoo.com>
This commit is contained in:
Denis Ledoux
2022-12-15 15:45:04 +01:00
parent c6c79473dd
commit 8d3940eab9
2 changed files with 92 additions and 29 deletions
+42 -28
View File
@@ -14,6 +14,8 @@ import time
import uuid
import warnings
from itertools import chain
from lxml import etree
from lxml.etree import LxmlError
from lxml.builder import E
@@ -1606,28 +1608,7 @@ actual arch.
msg = "%s on %s has parameters and cannot be called from a button"
self._log_view_warning(msg % (name, name_manager.model._name), node)
elif type_ == 'action':
# logic mimics /web/action/load behaviour
action = False
try:
action_id = int(name)
except ValueError:
model, action_id = self.env['ir.model.data']._xmlid_to_res_model_res_id(name, raise_if_not_found=False)
if not action_id:
msg = _("Invalid xmlid %(xmlid)s for button of type action.", xmlid=name)
self._raise_view_error(msg, node)
if not issubclass(self.pool[model], self.pool['ir.actions.actions']):
msg = _(
"%(xmlid)s is of type %(xmlid_model)s, expected a subclass of ir.actions.actions",
xmlid=name, xmlid_model=model,
)
self._raise_view_error(msg, node)
action = self.env['ir.actions.actions'].browse(action_id).exists()
if not action:
msg = _(
"Action %(action_reference)s (id: %(action_id)s) does not exist for button of type action.",
action_reference=name, action_id=action_id,
)
self._raise_view_error(msg, node)
name_manager.must_exist_action(name, node)
name_manager.has_action(name)
@@ -1791,11 +1772,7 @@ actual arch.
elif attr == 'groups':
for group in expr.replace('!', '').split(','):
# further improvement: add all groups to name_manager in
# order to batch check them at the end
if not self.env['ir.model.data']._xmlid_to_res_id(group.strip(), raise_if_not_found=False):
msg = "The group %r defined in view does not exist!"
self._log_view_warning(msg % group, node)
name_manager.must_exist_group(group.strip(), node)
elif attr in ('col', 'colspan'):
# col check is mainly there for the tag 'group', but previous
@@ -2846,6 +2823,8 @@ class NameManager:
self.mandatory_fields = collections.defaultdict(dict) # {field_name: {'groups': 'use}}
self.mandatory_parent_fields = collections.defaultdict(dict) # {field_name: {'groups': use}}
self.mandatory_names = dict() # {name: use}
self.must_exist_actions = {}
self.must_exist_groups = {}
self.parent = parent
self.children = []
if self.parent:
@@ -2888,8 +2867,14 @@ class NameManager:
def must_have_name(self, name, use):
self.mandatory_names[name] = use
def must_exist_action(self, action_id, node):
self.must_exist_actions[action_id] = node
def must_exist_group(self, name, node):
self.must_exist_groups[name] = node
def _get_node_groups(self, node):
return tuple(tuple(n.get('groups').split(',')) for n in node.xpath('ancestor-or-self::*[@groups]'))
return tuple(tuple(n.get('groups').split(',')) for n in chain([node], node.iterancestors()) if n.get('groups'))
def check(self, view):
# context for translations below
@@ -2908,6 +2893,35 @@ class NameManager:
message = _("Field `%(name)s` does not exist", name=name)
view._raise_view_error(message)
for name, node in self.must_exist_actions.items():
# logic mimics /web/action/load behaviour
action = False
try:
action_id = int(name)
except ValueError:
model, action_id = view.env['ir.model.data']._xmlid_to_res_model_res_id(name, raise_if_not_found=False)
if not action_id:
msg = _("Invalid xmlid %(xmlid)s for button of type action.", xmlid=name)
view._raise_view_error(msg, node)
if not issubclass(view.pool[model], view.pool['ir.actions.actions']):
msg = _(
"%(xmlid)s is of type %(xmlid_model)s, expected a subclass of ir.actions.actions",
xmlid=name, xmlid_model=model,
)
view._raise_view_error(msg, node)
action = view.env['ir.actions.actions'].browse(action_id).exists()
if not action:
msg = _(
"Action %(action_reference)s (id: %(action_id)s) does not exist for button of type action.",
action_reference=name, action_id=action_id,
)
view._raise_view_error(msg, node)
for name, node in self.must_exist_groups.items():
if not view.env['ir.model.data']._xmlid_to_res_id(name, raise_if_not_found=False):
msg = _("The group %(name)r defined in view does not exist!", name=name)
view._log_view_warning(msg, node)
for name, groups_uses in self.mandatory_fields.items():
use = next(iter(groups_uses.values()))
if name == 'id': # always available
+50 -1
View File
@@ -14,7 +14,7 @@ from psycopg2.extras import Json
from odoo.exceptions import AccessError, ValidationError
from odoo.tests import common
from odoo.tools import mute_logger, view_validation
from odoo.tools import get_cache_key_counter, mute_logger, view_validation
from odoo.addons.base.models.ir_ui_view import (
transfer_field_to_modifiers, transfer_node_to_modifiers, simplify_modifiers,
)
@@ -315,6 +315,55 @@ class TestViewInheritance(ViewCase):
# 3: _combine: arch_db
self.view_ids['A'].get_combined_arch()
def test_view_validate_button_action_query_count(self):
_, _, counter = get_cache_key_counter(self.env['ir.model.data']._xmlid_lookup, 'base.action_ui_view')
hit, miss = counter.hit, counter.miss
with self.assertQueryCount(8):
base_view = self.assertValid("""
<form string="View">
<header>
<button type="action" name="base.action_ui_view"/>
<button type="action" name="base.action_ui_view_custom"/>
<button type="action" name="base.action_ui_view"/>
</header>
<field name="name"/>
</form>
""")
self.assertEqual(counter.hit, hit)
self.assertEqual(counter.miss, miss + 2)
with self.assertQueryCount(9):
self.assertValid("""
<field name="name" position="replace"/>
""", inherit_id=base_view.id)
self.assertEqual(counter.hit, hit)
self.assertEqual(counter.miss, miss + 4)
def test_view_validate_attrs_groups_query_count(self):
_, _, counter = get_cache_key_counter(self.env['ir.model.data']._xmlid_lookup, 'base.group_system')
hit, miss = counter.hit, counter.miss
with self.assertQueryCount(5):
base_view = self.assertValid("""
<form string="View">
<field name="name" groups="base.group_system"/>
<field name="priority" groups="base.group_system"/>
<field name="inherit_id" groups="base.group_system"/>
</form>
""")
self.assertEqual(counter.hit, hit)
self.assertEqual(counter.miss, miss + 1)
with self.assertQueryCount(6):
self.assertValid("""
<field name="name" position="replace">
<field name="key" groups="base.group_system"/>
</field>
""", inherit_id=base_view.id)
self.assertEqual(counter.hit, hit)
self.assertEqual(counter.miss, miss + 2)
class TestApplyInheritanceSpecs(ViewCase):
""" Applies a sequence of inheritance specification nodes to a base