-
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
Improve performance of InterfaceValidator #27903
Improve performance of InterfaceValidator #27903
Conversation
Hi @lbajsarowicz. Thank you for your contribution
For more details, please, review the Magento Contributor Guide documentation. |
@lbajsarowicz what if you use |
Replaced in both occurrences. Still keeping the number of occurrences in mind. |
lib/internal/Magento/Framework/Interception/Code/InterfaceValidator.php
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The refactored logic is already covered by corresponding test \Magento\Framework\Interception\Test\Unit\Code\InterfaceValidatorTest
.
Hi @dmytro-ch, thank you for the review. |
Hi @lbajsarowicz, thank you for your contribution! |
Description (*)
According to the fact that we have:
after
pluginsbefore
pluginsaround
pluginsin Magento, I changed the order of conditions in Magento Interface Validator that is executed every single time when plugins are mentioned, to improve the logic performance.
Only by changing the order of conditions, we get in average 15% of performance improvement:
![image](https://user-images.githubusercontent.com/1639941/79748281-76d97e00-830d-11ea-807f-8a8b2ff5937a.png)
Benchmarking method (pseudocode)
Additionally, replaced Elvis (
?:
) operator with null coalescing operator (??
) which does not juggle types during comparison.On line 73 you can see that I moved
=== null
comparison before the other comparison because it's cheaper to compare to null than execute additional class method.Replaced
![image](https://user-images.githubusercontent.com/1639941/79747721-7b516700-830c-11ea-9565-9e42a77bd79f.png)
substr
comparison withstrpos
which during my benchmark improved the performance slightly:After all my changes, the Class performs about 21% faster
![image](https://user-images.githubusercontent.com/1639941/79748684-20b90a80-830e-11ea-95d8-e6f05f7524d6.png)
How do you...?
The tool used for performance comparisons is blackfire.io
Related Pull Requests
Fixed Issues (if relevant)
N/A
Manual testing scenarios (*)
N/A
Questions or comments
Support
Solving this issue is supported by Mediotype
Contribution checklist (*)