[IMP] sale_order_product_assortment: performance in so

[aleuffre]

This commit improves performance when computing the products allowed in a Sales Order, especially when creating an order and the partner is empty

[OCA-git-bot]

Hi @CarlosRoca13,
some modules you are maintaining are being modified, check this out!

[CarlosRoca13]

Hi @aleuffre, first of all thanks for the changes 😄 can you show a comparison of the times between the current version and after adding these changes?

ping @carlosdauden

[aleuffre]

Thank you for checking the PR @CarlosRoca13 !

I tried using timeit but it wouldn't behave for some reason, so, apologies for the "homemade" solution in Odoo shell.

This is tested on a production database that is having some performance issues, that's why we're doing a performance pass on a handful of modules

Before:

>>> from datetime import datetime as dt
>>> so = env["sale.order"].search([], limit=1)
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
3.842279 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.563739 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.670987 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.658426 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.532004 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.629248 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.555761 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.684823 seconds

After:

>>> from datetime import datetime as dt
>>> so = env["sale.order"].search([], limit=1)
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
3.460797 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.454272 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.428768 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.399137 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.413592 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.444747 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.380238 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.397538 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.412193 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.373276 seconds
>>> start = dt.now(); so._compute_product_assortment_ids(); print((dt.now() - start).total_seconds(), "seconds")
0.430472 seconds

To add to this, when pressing "create" from the interface, and the partner is still empty, the new function checks that the partner is not set and is not executed, whereas the old function still performs the full search.

[CarlosRoca13]

It seems to be ok 😄

can you review too @carlosdauden @pedrobaeza

[francesco-ooops]

LGTM

[aleuffre]

Sorry, I made a new change, specifically

                filters_partner_domain = filters_partner_domain.filtered(
                    lambda x, partner=partner: partner in x.partner_ids
                    or partner.filtered_domain(x._get_eval_partner_domain())
                )

This lets us skip computing all_partner_ids and the first invocation of the method goes down from 3.8 seconds to 1.8 seconds on my DB.
Also did some cleanup on the onchange below, though the impact in terms of performance is almost negligible.

[aleuffre]

I need to fix the tests.

[carlosdauden]

This version does not have the changes applied properly, this backport was made, but it was not quite right:
#3135
as in addition to these changes there were others that did not carry over.
Just take a look at the file in 15.0:

sale-workflow/sale_order_product_assortment/models/sale_order.py

Lines 18 to 38 in ad9d3f6

	@api.depends("partner_id", "partner_shipping_id", "partner_invoice_id")
	def _compute_product_assortment_ids(self):
	self.allowed_product_ids = False
	self.has_allowed_products = False
	partner_field = (
	self.env["ir.config_parameter"]
	.sudo()
	.get_param("sale_order_product_assortment.partner_field", "partner_id")
	)
	partner = self[partner_field]
	product_domain = []
	if partner:
	for ir_filter in partner.applied_assortment_ids:
	product_domain = expression.AND(
	[product_domain, ir_filter._get_eval_domain()]
	)
	if product_domain:
	self.allowed_product_ids = self.env["product.product"].search(
	product_domain
	)
	self.has_allowed_products = True

Test the performance with these changes by simply copying and pasting.

[carlosdauden]

You need changes in product_assortment which I see are not at 14.0 either:
https://github.com/OCA/product-attribute/blob/15.0/product_assortment/models/ir_filters.py#L62-L71

[aleuffre]

This version does not have the changes applied properly, this backport was made, but it was not quite right: #3135 as in addition to these changes there were others that did not carry over. Just take a look at the file in 15.0:

sale-workflow/sale_order_product_assortment/models/sale_order.py

Lines 18 to 38 in ad9d3f6

	@api.depends("partner_id", "partner_shipping_id", "partner_invoice_id")
	def _compute_product_assortment_ids(self):
	self.allowed_product_ids = False
	self.has_allowed_products = False
	partner_field = (
	self.env["ir.config_parameter"]
	.sudo()
	.get_param("sale_order_product_assortment.partner_field", "partner_id")
	)
	partner = self[partner_field]
	product_domain = []
	if partner:
	for ir_filter in partner.applied_assortment_ids:
	product_domain = expression.AND(
	[product_domain, ir_filter._get_eval_domain()]
	)
	if product_domain:
	self.allowed_product_ids = self.env["product.product"].search(
	product_domain
	)
	self.has_allowed_products = True

Test the performance with these changes by simply copying and pasting.

I have vague memories of this PR. I think the problem, as you pointed out, was that there were changes not ported in another module in another repo, and due to the fact it can take a while to get PRs merged and we were in a hurry, it was decided to adapt the changes to the module as is.

Having to compute all_partner_ids just to see if a single partner is in a recordset of hundreds, if not thousands, every time, is really heavy, as you can imagine.

I'll check the PRs you mentioned.

Thank you for your time!

[carlosdauden]

Ok.
I'm going to approve the changes anyway in case you want to merge this, as I understand it works better than what is currently there and I don't want to block in this version either.

[aleuffre]

I still need to fix the tests, but thank you