-
Notifications
You must be signed in to change notification settings - Fork 23.2k
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
[IMP] pos_self_order: Avoid extra _get_attributes_by_ptal_id calls #159551
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
needs rebase |
@caburj Can rebase this to a later version, would 17.0 be a good candidate for this? It is also the version of this customer's DB. Apologies for my lack of knowledge here, but would the reason for the rebase be for stability? |
zaha-odoo
force-pushed
the
saas-16.4-opw-3758923-zaha
branch
3 times, most recently
from
March 29, 2024 14:30
267af04
to
855cdbc
Compare
caburj
reviewed
Mar 29, 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
zaha-odoo
force-pushed
the
saas-16.4-opw-3758923-zaha
branch
from
March 29, 2024 15:43
855cdbc
to
d7e0460
Compare
caburj
reviewed
Mar 30, 2024
robodoo r+ |
robodoo
pushed a commit
that referenced
this pull request
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 closes #159551 Signed-off-by: Joseph Caburnay (jcb) <jcb@odoo.com>
This was referenced Apr 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 callself.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 manyread
calls to product.attribute.valueThis 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