Improve bundle load speeds

[thlassche]

Description

When a bundle has a lot of selection, the tax GetPriceConfigurationObserver becomes a real performance hog because it loads the selections over and over again. This fixes that by caching the selection in memory

Manual testing scenarios

Create a bundle with a lot of selections. Check the loading speed of the product page with the bundle before and after this commit. In my local testing scenarios, the loading time went from ~30s to about ~3s

[magento-cicd2]

All committers have signed the CLA.

[magento-engcom-team]

Hi @thlassche. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me 2.3-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[slavvka]

Hi @thlassche Thank you for your improvement. I would like you to clarify the following.
Is your improvement intended to cache options between the observer runs? If yes it may be unpredictable since the cached objects may be changed. If no you should clear the cache before using it since this particular observer is shared object. And this case looks rare since in most cases options of a bundle products are different objects.

[thlassche]

@slavvka I definitely agree with you, however all scenarios will hold: The observer takes the current product from registry (it already did so before my change):

$product = $this->registry->registry('current_product');

The id of that product is used as cache key for cached instances of the bundle selections, so it will not clash.

And to clarify a little bit on the potential gains of this improvement: The observer contains a recursive function (recurConfigAndUpdatePrice), and that one calls the updatePriceForBundle multiple times, which would load the same bundle selections over and over again for every call, with my improvement that is cached.

[slavvka]

The id of that product is used as cache key for cached instances of the bundle selections, so it will not clash.

I agree but products in cache may be changed between runs of the observer and already cached products may get invalid in a next run. Since caching is needed only for the current run I would explicitly clear the cache in the beginning of execute method.

And to clarify a little bit on the potential gains of this improvement: The observer contains a recursive function (recurConfigAndUpdatePrice), and that one calls the updatePriceForBundle multiple times, which would load the same bundle selections over and over again for every call, with my improvement that is cached.

Yes, now that makes sense for me. Thank you!

[thlassche]

I agree but products in cache may be changed between runs of the observer and already cached products may get invalid in a next run. Since caching is needed only for the current run I would explicitly clear the cache in the beginning of execute method.

I agree, that's better. Just changed it

[magento-engcom-team]

Hi @slavvka, thank you for the review.
ENGCOM-4085 has been created to process this Pull Request

[magento-engcom-team]

@thlassche thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

Fixed the space

[magento-engcom-team]

Hi @slavvka, thank you for the review.
ENGCOM-4085 has been created to process this Pull Request

[magento-engcom-team]

Hi @sidolov, thank you for the review.
ENGCOM-4085 has been created to process this Pull Request

[engcom-Delta]

Hi, @thlassche . Your changes in code does't do anything because my debugger don't stop at line 116 when i load a page of bundle product. And this page load time like in 2.3-develop.
Could you take a look?
Thanks!

[thlassche]

Hi @engcom-Delta,

Just tried here with a debug_print_backtrace 116, which gives:

#0  Magento\Tax\Observer\GetPriceConfigurationObserver->updatePriceForBundle() called at [/Users/thlassche/Projects/heuveling/httpdocs/vendor/magento/module-tax/Observer/GetPriceConfigurationObserver.php:90]
--
  | #1  Magento\Tax\Observer\GetPriceConfigurationObserver->recurConfigAndUpdatePrice() called at [/Users/thlassche/Projects/heuveling/httpdocs/vendor/magento/module-tax/Observer/GetPriceConfigurationObserver.php:86]
  | #2  Magento\Tax\Observer\GetPriceConfigurationObserver->recurConfigAndUpdatePrice() called at [/Users/thlassche/Projects/heuveling/httpdocs/vendor/magento/module-tax/Observer/GetPriceConfigurationObserver.php:86]
  | #3  Magento\Tax\Observer\GetPriceConfigurationObserver->recurConfigAndUpdatePrice() called at [/Users/thlassche/Projects/heuveling/httpdocs/vendor/magento/module-tax/Observer/GetPriceConfigurationObserver.php:86]
  | #4  Magento\Tax\Observer\GetPriceConfigurationObserver->recurConfigAndUpdatePrice() called at [/Users/thlassche/Projects/heuveling/httpdocs/vendor/magento/module-tax/Observer/GetPriceConfigurationObserver.php:86]
  | #5  Magento\Tax\Observer\GetPriceConfigurationObserver->recurConfigAndUpdatePrice() called at [/Users/thlassche/Projects/heuveling/httpdocs/vendor/magento/module-tax/Observer/GetPriceConfigurationObserver.php:61]
  | #6  Magento\Tax\Observer\GetPriceConfigurationObserver->execute() called at [/Users/thlassche/Projects/heuveling/httpdocs/vendor/magento/framework/Event/Invoker/InvokerDefault.php:72]
  | #7  Magento\Framework\Event\Invoker\InvokerDefault->_callObserverMethod() called at [/Users/thlassche/Projects/heuveling/httpdocs/vendor/magento/framework/Event/Invoker/InvokerDefault.php:60]
  | #8  Magento\Framework\Event\Invoker\InvokerDefault->dispatch() called at [/Users/thlassche/Projects/heuveling/httpdocs/vendor/magento/framework/Event/Manager.php:66]
  | #9  Magento\Framework\Event\Manager->dispatch() called at [/Users/thlassche/Projects/heuveling/httpdocs/generated/code/Magento/Framework/Event/Manager/Proxy.php:95]
  | #10 Magento\Framework\Event\Manager\Proxy->dispatch() called at [/Users/thlassche/Projects/heuveling/httpdocs/vendor/magento/module-bundle/Block/Catalog/Product/View/Type/Bundle.php:210]
  | #11 Magento\Bundle\Block\Catalog\Product\View\Type\Bundle->getJsonConfig() called at [/Users/thlassche/Projects/heuveling/httpdocs/vendor/magento/framework/Interception/Interceptor.php:58]

Is your Magento configured to display prices including tax?

[engcom-Delta]

@thlassche , i test with display prices including tax. Sorry, but i can't test with debug_print_backtrace because i got this message when i try to clone this PR: "Remote branch EENGCOM-4085-magento-magento2-20877 not found in upstream origin". We have to create new PR.

[engcom-Delta]

✔️ QA passed

[m2-assistant]

Hi @thlassche, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.