Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FW][IMP] pos_self_order: Avoid extra _get_attributes_by_ptal_id calls #162962

Closed

Conversation

fw-bot
Copy link
Contributor

@fw-bot fw-bot commented Apr 23, 2024

Description of the issue/feature this PR addresses:
This commit is a performance fix to improve the speed of opening the pos kiosk or QR code.

_get_attributes method of pos_self_order's product.product extension will call self.env["pos.session"]._get_attributes_by_ptal_id() every time it is called. For N products, _get_attributes is called N times. This can lead to slow performance for high enough N, because _get_attributes_by_ptal_id method is slow, because it makes many read calls to product.attribute.value

This commit lifts the call to _get_attributes_by_ptal_id higher in the call stack, so that it is only called once as opposed to N times. It passes its result into _get_attributes via context, which will re-call _get_attributes_by_ptal_id if it wasn't in context for backwards compatibility reasons.

attributes must be deep copied within _get_attributes, because _add_price_info_to_attributes mutates the values within, which would invalidate future calls. The deep copy gives a fresh instance for each call, and only copies the applicable attributes so it shouldn't be large.

In this particular customer's DB they have 1376 product.product records and their pos config's pricelist (id 36) has 1572 rules in it.

Overall, based on the benchmarks below, this commit makes loading the pos about 4-5 times faster.

Benchmarks:

Before commit

Customer DB
product.product count == 1376
SQL query count ~= 5034
Time to load pos ~= 44 sec

Customer DB with more products
product.product count == 3792
SQL query count ~= 9359
Time to load pos ~= 96 sec

After commit

Customer DB
product.product count == 1376
SQL query count ~= 2360
Time to load pos ~= 8 sec

Customer DB with more products
product.product count == 3792
SQL query count ~= 3906
Time to load pos ~= 22 sec

Current behavior before PR:
Slow loading of pos kiosk/QR code

Desired behavior after PR is merged:
Faster loading of pos kiosk/QR code

opw-3758923

Forward-Port-Of: #159551

@robodoo
Copy link
Contributor

robodoo commented Apr 23, 2024

Pull request status dashboard.

@fw-bot
Copy link
Contributor Author

fw-bot commented Apr 23, 2024

@zaha-odoo @caburj cherrypicking of pull request #159551 failed.

stdout:

Auto-merging addons/pos_self_order/models/product_product.py
CONFLICT (content): Merge conflict in addons/pos_self_order/models/product_product.py

stderr:

11:52:39.736691 git.c:463               trace: built-in: git cherry-pick d7e046002cfc8ac69a70d73bf265e28df0b210cc
error: could not apply d7e046002cfc... [IMP] pos_self_order: Avoid extra _get_attributes_by_ptal_id calls
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".
----------
status:

Either perform the forward-port manually (and push to this branch, proceeding as usual) or close this PR (maybe?).

In the former case, you may want to edit this PR message as well.

⚠️ after resolving this conflict, you will need to merge it via @robodoo.

More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port

@C3POdoo C3POdoo added the OE the report is linked to a support ticket (opw-...) label Apr 23, 2024
@robodoo robodoo added forwardport This PR was created by @fw-bot conflict There was an error while creating this forward-port PR labels Apr 23, 2024
This commit is a performance fix to improve the speed of opening the pos kiosk or QR code.

`_get_attributes` method of pos_self_order's product.product extension will call `self.env["pos.session"]._get_attributes_by_ptal_id()` every time it is called. For *N* products, `_get_attributes` is called *N* times. This can lead to slow performance for high enough *N*, because `_get_attributes_by_ptal_id` method is slow, because it makes many `read` calls to product.attribute.value

This commit lifts the call to `_get_attributes_by_ptal_id` higher in the call stack, so that it is only called once as opposed to *N* times. It passes its result into `_get_attributes` via context, which will re-call `_get_attributes_by_ptal_id` if it wasn't in context for backwards compatibility reasons.

attributes must be deep copied within `_get_attributes`, because `_add_price_info_to_attributes` mutates the values within, which would invalidate future calls. The deep copy gives a fresh instance for each call, and only copies the applicable attributes so it shouldn't be large.

In this particular customer's DB they have 1376 product.product records and their pos config's pricelist (id 36) has 1572 rules in it.

Overall, based on the benchmarks below, this commit makes loading the pos about 4-5 times faster.

Benchmarks:

__Before commit__

_Customer DB_
product.product count == 1376
SQL query count ~= 5034
Time to load pos ~= 44 sec

_Customer DB with more products_
product.product count == 3792
SQL query count ~= 9359
Time to load pos ~= 96 sec

__After commit__

_Customer DB_
product.product count == 1376
SQL query count ~= 2360
Time to load pos ~= 8 sec

_Customer DB with more products_
product.product count == 3792
SQL query count ~= 3906
Time to load pos ~= 22 sec

X-original-commit: f09db06
@zaha-odoo zaha-odoo force-pushed the 17.0-saas-16.4-opw-3758923-zaha-wBHz-fw branch from e4c70bf to 8b8cf80 Compare April 23, 2024 15:21
@C3POdoo C3POdoo requested review from a team and vlst-odoo and removed request for a team April 23, 2024 15:24
@zaha-odoo
Copy link
Contributor

@robodoo r+

@robodoo robodoo closed this in ebf695e Apr 24, 2024
willylohws pushed a commit to willylohws/odoo that referenced this pull request May 1, 2024
This commit is a performance fix to improve the speed of opening the pos kiosk or QR code.

`_get_attributes` method of pos_self_order's product.product extension will call `self.env["pos.session"]._get_attributes_by_ptal_id()` every time it is called. For *N* products, `_get_attributes` is called *N* times. This can lead to slow performance for high enough *N*, because `_get_attributes_by_ptal_id` method is slow, because it makes many `read` calls to product.attribute.value

This commit lifts the call to `_get_attributes_by_ptal_id` higher in the call stack, so that it is only called once as opposed to *N* times. It passes its result into `_get_attributes` via context, which will re-call `_get_attributes_by_ptal_id` if it wasn't in context for backwards compatibility reasons.

attributes must be deep copied within `_get_attributes`, because `_add_price_info_to_attributes` mutates the values within, which would invalidate future calls. The deep copy gives a fresh instance for each call, and only copies the applicable attributes so it shouldn't be large.

In this particular customer's DB they have 1376 product.product records and their pos config's pricelist (id 36) has 1572 rules in it.

Overall, based on the benchmarks below, this commit makes loading the pos about 4-5 times faster.

Benchmarks:

__Before commit__

_Customer DB_
product.product count == 1376
SQL query count ~= 5034
Time to load pos ~= 44 sec

_Customer DB with more products_
product.product count == 3792
SQL query count ~= 9359
Time to load pos ~= 96 sec

__After commit__

_Customer DB_
product.product count == 1376
SQL query count ~= 2360
Time to load pos ~= 8 sec

_Customer DB with more products_
product.product count == 3792
SQL query count ~= 3906
Time to load pos ~= 22 sec

closes odoo#162962

X-original-commit: f09db06
Signed-off-by: Joseph Caburnay (jcb) <jcb@odoo.com>
Signed-off-by: Zachary Hanham (zaha) <zaha@odoo.com>
@fw-bot fw-bot deleted the 17.0-saas-16.4-opw-3758923-zaha-wBHz-fw branch May 8, 2024 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
conflict There was an error while creating this forward-port PR forwardport This PR was created by @fw-bot OE the report is linked to a support ticket (opw-...)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants