-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
[Performance] Add some class cache to StatusLabel #36652
Comments
Hi @Nuranto. Thank you for your report.
Make sure that the issue is reproducible on the vanilla Magento instance following Steps to reproduce. To deploy vanilla Magento instance on our environment, Add a comment to the issue:
For more details, review the Magento Contributor Assistant documentation. Add a comment to assign the issue: To learn more about issue processing workflow, refer to the Code Contributions.
🕙 You can find the schedule on the Magento Community Calendar page. 📞 The triage of issues happens in the queue order. If you want to speed up the delivery of your contribution, join the Community Contributions Triage session to discuss the appropriate ticket. ✏️ Feel free to post questions/proposals/feedback related to the Community Contributions Triage process to the corresponding Slack Channel |
@magento give me 2.4-develop instance |
Hi @faizan-shk. Thank you for your request. I'm working on Magento instance for you. |
Hi @faizan-shk, here is your Magento Instance: https://97fdb24466287dc965e6af7b521ac4de.instances.magento-community.engineering |
Hi @engcom-Hotel. Thank you for working on this issue.
|
Hello @Nuranto, Thanks for the report and collaboration! We have tried to reproduce this issue with the Magento latest development branch ie 2.4-develop with sample data installed. And the issue is reproducible for us. We have followed the below steps in order to reproduce the issue:
Hence confirming the issue. Thanks |
✅ Jira issue https://jira.corp.adobe.com/browse/AC-7573 is successfully created for this GitHub issue. |
✅ Confirmed by @engcom-Hotel. Thank you for verifying the issue. |
Just tested MR #36716 : thanks @rogerdz, but I'm afraid it does not solve the issue with my use case above. magento2/app/code/Magento/Sales/Model/Order/Status.php Lines 134 to 141 in c130c2d
My issue was not about queries on My issue is that I got a huge number of select calls on The solution I proposed in my initial message was this :
This one does solve the issue. But after reflexion, maybe a better solution would be to create an order status repository with a registry, and refactor all calls to |
Summary
Magento 2.4.5-p1
A DB call is made every time we call
Magento\Sales\Model\Order\StatusLabel::getStatusFrontendLabel
, even if we call it for the same status code.Examples
If history contains 10 comments with "New" status and 10 comments with "Complete" status. 20 calls
select sales_order_status.* from sales_order_status where (sales_order_status.status = ?)
will occur instead of 2 (or 1...)Let say you want to display history comments on an order list page so :
That's 4000 calls to DB instead of 2 (or 1..). (I consider each order has same history as previous example to simplify the calculation/example)
Proposed solution
(pseudo code. The cache key will of course need to contain area and store)
if($this->labels[$code]) return $this->labels[$code]; else return $this->labels[$code] = loadLabelForCode(code);
if($this->labels[$code]) return $this->labels[$code]; else return $this->labels = loadLabels();
Solution 1 is calling DB only once per needed label.
Solution 2 is calling DB only once. Gives better performance in most cases, but not if the runtime only need one label.
Both solutions are far better than the existing one though.
See https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Sales/Model/Order/StatusLabel.php#L57
Additional steps to reproduce
We have tried to reproduce this issue with the Magento latest development branch ie 2.4-develop with sample data installed. And the issue is reproducible for us. We have followed the below steps in order to reproduce the issue:
magento2/app/code/Magento/Sales/Model/ResourceModel/Order/Status.php
Line 90 in 3e5be99
complete
order status. It can be one time by using cache.Release note
No response
Triage and priority
The text was updated successfully, but these errors were encountered: