From 6875d23647fd0676c30f4776c366ba778bf8a194 Mon Sep 17 00:00:00 2001 From: "Hugo Carlier (Huca)" Date: Mon, 8 May 2023 15:15:03 +0000 Subject: [PATCH] [REF] resource: improve filter_domain_leaf performances MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In some methods, like group expand methods, it is not uncommon to transform a domain designed for model A to search records from another model B. In general this transformation requires two operations: - Rename some fields in the domain - Remove leaves from the domain that relate to fields not existing in model B The current function filter_domain_leaf allows to realize this second operation. It currently relies on a recursive implementation with a complexity of order O(n²). This commit introduces a re-writing of the function filter_domain_leaf that: 1. Changes the recursion based implementation to an explicit stack implementation 2. Introduces a field mapping dictionary as argument The explicit stack implementation allows to avoid stack overflow when working with long domains. The complexity order is also improved to O(n) instead of O(n²) which leads to performance gain. For a domain length of ~100 elements, the gain (timewise) is reaching 50%. It is >95% for domain length over 10 000 elements. The field mapping dictionary allows to avoid pre-filtering of domains when using filter_domain_leaf. In cases where it is used, the domain then just has to be browsed once in the filter_domain_leaf function. task-3299357 closes odoo/odoo#120834 Related: odoo/enterprise#40847 Signed-off-by: Xavier Bol (xbo) --- .../project_task_burndown_chart_report.py | 3 +- addons/resource/models/utils.py | 71 ++++++++++++------- addons/resource/tests/test_utils.py | 10 +++ 3 files changed, 55 insertions(+), 29 deletions(-) diff --git a/addons/project/report/project_task_burndown_chart_report.py b/addons/project/report/project_task_burndown_chart_report.py index 121988e7a16..721a0c1cfc1 100644 --- a/addons/project/report/project_task_burndown_chart_report.py +++ b/addons/project/report/project_task_burndown_chart_report.py @@ -218,8 +218,7 @@ class ReportProjectTaskBurndownChart(models.AbstractModel): * A domain that only contains fields that are specific to `project.task.burndown.chart.report` * A domain that only contains fields that are specific to `project.task` - Fields that are not part of the constraint are replaced by either a `FALSE_LEAF` or a `TRUE_LEAF` in order - to ensure the complete domain evaluation. See `remove_domain_leaf` for more details. + See `filter_domain_leaf` for more details on the new domains. :param domain: The domain that has been passed to the read_group. :return: A tuple containing the non `project.task` specific domain and the `project.task` specific domain. diff --git a/addons/resource/models/utils.py b/addons/resource/models/utils.py index 8cce56d4afa..fe435f6395d 100644 --- a/addons/resource/models/utils.py +++ b/addons/resource/models/utils.py @@ -7,7 +7,7 @@ from itertools import chain from pytz import utc from odoo import fields -from odoo.osv.expression import normalize_domain, NOT_OPERATOR, DOMAIN_OPERATORS +from odoo.osv.expression import normalize_domain, is_leaf, NOT_OPERATOR from odoo.tools.float_utils import float_round # Default hour per day value. The one should @@ -52,7 +52,7 @@ def _boundaries(intervals, opening, closing): yield (start, opening, recs) yield (stop, closing, recs) -def filter_domain_leaf(domain, leaf_check): +def filter_domain_leaf(domain, field_check, field_name_mapping=None): """ filter_domain_lead only keep the leaves of a domain that verify a given check. Logical operators that involves a leaf that is undetermined (because it does not pass the check) are ignored. @@ -63,34 +63,51 @@ def filter_domain_leaf(domain, leaf_check): params: - domain: the domain that needs to be filtered - - leaf_check: the function that the field used in the leaf needs to verify to keep the leaf + - field_check: the function that the field name used in the leaf needs to verify to keep the leaf + - field_name_mapping: dictionary of the form {'field_name': 'new_field_name', ...}. Occurences of 'field_name' + in the first element of domain leaves will be replaced by 'new_field_name'. This is usefull when adapting a + domain from one model to another when some field names do not match the names of the corresponding fields in + the new model. returns: The filtered version of the domain """ - def _filter_domain_leaf_recursive(domain, leaf_check, operator=False): - """ - return domain, rest_domain -> rest_domain should be empty if the operation is finished - """ - if len(domain) == 0: - return ([], []) - if not operator: - first_elem = domain[0] - if first_elem not in DOMAIN_OPERATORS: #End of a current leaf - return ([], domain[1:]) if not leaf_check(first_elem[0]) else ([first_elem], domain[1:]) - operator = first_elem - domain = domain[1:] - - leaf_1, rest_domain = _filter_domain_leaf_recursive(domain, leaf_check) - if operator == NOT_OPERATOR: - return ([operator, *leaf_1], rest_domain) if leaf_1 else ([], rest_domain) - leaf_2, rest_domain = _filter_domain_leaf_recursive(rest_domain, leaf_check) - if leaf_1 == [] or leaf_2 == []: - return ((leaf_1 or leaf_2), rest_domain) - return ([operator, *leaf_1, *leaf_2], rest_domain) - domain = normalize_domain(domain) - operator = domain[0] if len(domain) > 1 else False - domain = domain[1:] if len(domain) > 1 else domain - return _filter_domain_leaf_recursive(domain, leaf_check, operator=operator)[0] + field_name_mapping = field_name_mapping or {} + + stack = [] # stack of elements (leaf or operator) to conserve (reversing it gives a domain) + ignored_elems = [] # history of ignored elements in the domain (not added to the stack) + # if the top of the stack ignored_elems is: + # - True: indicates that the last browsed elem has been ignored + # - False: indicates that the last browsed elem has been added to the stack + # When an operator is applied to some elements, they are removed from the ignored_elems stack + # (and replaced by the ignored_elems flag of the operator) + while domain: + next_elem = domain.pop() # Browsing the domain backward simplifies the filtering + if is_leaf(next_elem): + field_name, op, value = next_elem + field_name = field_name_mapping.get(field_name, field_name) + if field_check(field_name): + stack.append((field_name, op, value)) + ignored_elems.append(False) + else: + ignored_elems.append(True) + elif next_elem == NOT_OPERATOR: + ignore_operation = ignored_elems.pop() + if not ignore_operation: + stack.append(NOT_OPERATOR) + ignored_elems.append(False) + else: + ignored_elems.append(True) + else: # OR/AND operation + ignore_operand1 = ignored_elems.pop() + ignore_operand2 = ignored_elems.pop() + if not ignore_operand1 and not ignore_operand2: + stack.append(next_elem) + ignored_elems.append(False) + elif ignore_operand1 and ignore_operand2: + ignored_elems.append(True) + else: + ignored_elems.append(False) # the AND/OR operation is replaced by one of its operand which cannot be ignored + return list(reversed(stack)) class Intervals(object): """ Collection of ordered disjoint intervals with some associated records. diff --git a/addons/resource/tests/test_utils.py b/addons/resource/tests/test_utils.py index 0511ce2e022..2c5db580d6c 100644 --- a/addons/resource/tests/test_utils.py +++ b/addons/resource/tests/test_utils.py @@ -47,3 +47,13 @@ class TestExpression(TransactionCase): for idx, fields in enumerate(fields_to_remove): results = [normalize_domain(utils.filter_domain_leaf(dom, lambda field: field not in fields)) for dom in domains] self.assertEqual(results, expected_results[idx]) + + # Testing field mapping 1 + self.assertEqual( + ['&', '!', ('field3', '=', False), ('field3', '!=', 'test')], + normalize_domain(utils.filter_domain_leaf( + ['|', ('field1', 'in', [1, 2]), '!', ('field2', '=', False), ('field3', '!=', 'test')], + lambda field: field == 'field3', + field_name_mapping={'field2': 'field3'}, + )) + )