Skip to content
Snippets Groups Projects
Commit 04553b92 authored by Alvaro Fuentes's avatar Alvaro Fuentes
Browse files

[FIX] mrp: improve perf of `_skip_bom_line`


The main motivation for this change is to improve performance of BoM
`explode`, which heavily rely on checking the lines to skip. `explode`
is also called from product `compute_quantities_dict` which is called
multiple times during upgrades. With this fix we improved the running
time of an upgrade _test_ from 3h:30m to 2h:55m which gives a 16%
performance improvement.

The goal of `_skip_bom_line` is to check that for each attribute present
on the line, at least one value associated to that attribute must be in
the attribute values of the product(*). If none is found then we
consider that we can skip the line.

The previous implementation was inefficient. It grouped all values by
attribute, then checked one by one if at least one value is on the
product. In case one attribute does not have any value on the product it
skipped the line.

The implementation we propose here is to take the intersection of the
product and line values, then check that their attributes are the same.
The later can be done with a simple length check. In case they are
different the line must be skipped. Note that this works because only
one value is possible per attribute in a product.

Both implementations are equivalent. The second is more efficient
because does not branch and relies on (record)set operations.

For example, let's consider a product with two attribute values
`a` and `b`, and a line with multiple values `a`,`y` for
attribute 1, and `z` for attribute 2.
```
Product                      Line
+---+                      +-----+
| a | <- same attribute -> | a,y |
+---+                      +-----+
| b | <- same attribute -> |  z  |
+---+                      + ----+
```
This line must be skipped. The reason is that the value `b` is not
among the list `[z]` of values for attribute 2 on the line. The new
implementation would get the intersection of attribute values as `[a]`
from there the comparison of the attributes will fail because `[a]` has
only one attribute while the line has two.

Let's consider a second case, where there is no value on the line for
attribute 2.
```
Product                      Line
+---+                      +-----+
| a | <- same attribute -> | a,y |
+---+                      +-----+
| b | <- same attribute -> |     |
+---+                      + ----+
```
This line is not skipped because there is no value for attribute 2 on
the line. Therefore the condition(*) per attribute is not violated for
this product. The new implementation gets `[a]` as intersection of values,
but now the attributes coincide: they are both attribute 1 for the
intersection and the line.

Finally,
```
Product                      Line
+---+                      +-----+
| a | <- same attribute -> | a,y |
+---+                      +-----+
|   | <- same attribute -> | z,w |
+---+                      + ----+
```
This line is skipped because none of `[z,w]` are in the product. The new
implementation would get again `[a]` as intersection which does not
match the attributes on the line.

closes odoo/odoo#99047

Signed-off-by: default avatarWilliam Henrotin (whe) <whe@odoo.com>
parent 70f7e2d7
Branches
Tags
No related merge requests found
......@@ -453,11 +453,11 @@ class MrpBomLine(models.Model):
self.ensure_one()
if product._name == 'product.template':
return False
if self.bom_product_template_attribute_value_ids:
for ptal, iter_ptav in groupby(self.bom_product_template_attribute_value_ids.sorted('attribute_line_id'), lambda ptav: ptav.attribute_line_id):
if not any(ptav in product.product_template_attribute_value_ids for ptav in iter_ptav):
return True
return False
# The intersection of the values of the product and those of the line satisfy:
# * the number of items equals the number of attributes (since a product cannot
# have multiple values for the same attribute),
# * the attributes are a subset of the attributes of the line.
return len(product.product_template_attribute_value_ids & self.bom_product_template_attribute_value_ids) != len(self.bom_product_template_attribute_value_ids.attribute_id)
def action_see_attachments(self):
domain = [
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment