Load symfony container in front by boherm · Pull Request #32719 · PrestaShop/PrestaShop

boherm · 2023-05-25T16:36:13Z

Questions	Answers
Branch?	develop
Description?	Add symfony container for front and dotenv files for configurations! We can enable this experimental feature with new dotenv file!
Type?	new feature
Category?	FO
BC breaks?	yes
Deprecations?	no
How to test?	Set `PS_CONTAINER_V2_FRONT=true` in `.env` file.
Fixed ticket?	Fixes #31846
Related PRs
Sponsor company

Notes: You can run tests ui with this PR: PrestaShop/ga.tests.ui.pr#34.

Tests:

CI ✅
Autotests (without new container activated) ⏳
Autotests (with new container activated) ⏳

Some performances tests:
(with PHP 8.1, no opcache, no docker, with apache & mysql 8.0.33 on MacBook Pro M2 16go Ram)

Page	Dev	V2	Time	I/O wait	CPU	Memory	Network
FO - Homepage	❌	❌	152 ms	46.9 ms	105 ms	6.3 MB	205 kB
	❌	✅	167 ms	44.9 ms	122 ms	7.5 MB	210 kB
	✅	❌	148 ms	30.1 ms	118 ms	6.3 MB	205 kB
	✅	✅	498 ms	58.2 ms	440 ms	12.1 MB	210 kB
FO - Product page	❌	❌	224 ms	76.2 ms	148 ms	8.4 MB	347 kB
	❌	✅	247 ms	79.5 ms	167 ms	9.5 MB	352 kB
	✅	❌	254 ms	91.5 ms	162 ms	8.4 MB	347 kB
	✅	✅	616 ms	98.4 ms	518 ms	14.7 MB	352 kB
FO - Products listing	❌	❌	431 ms	160 ms	271 ms	10.4 MB	650 kB
	❌	✅	426 ms	162 ms	264 ms	11.5 MB	655 kB
	✅	❌	418 ms	160 ms	258 ms	10.4 MB	650 kB
	✅	✅	801 ms	176 ms	625 ms	18 MB	655 kB
FO - Cart when connected with one product	❌	❌	220 ms	73.8 ms	146 ms	7.5 MB	317 kB
	❌	✅	318 ms	109 ms	208 ms	14.4 MB	339 kB
	✅	❌	207 ms	64.1 ms	143 ms	7.5 MB	317 kB
	✅	✅	638 ms	88.2 ms	550 ms	14.4 MB	322 kB
FO - Cart when connected with 19 products	❌	❌	852 ms	312 ms	541 ms	13.8 MB	1270 kB
	❌	✅	884 ms	330 ms	554 ms	15.2 MB	1270 kB
	✅	❌	847 ms	313 ms	533 ms	14.0 MB	1270 kB
	✅	✅	1370 ms	330 ms	1040 ms	28.5 MB	1270 kB
FO - Cart when connected with 40 products	❌	❌	1740 ms	610 ms	1130 ms	21,6 MB	2360 kB
	❌	✅	1780 ms	627 ms	1160 ms	22.2 MB	2360 kB
	✅	❌	1690 ms	588 ms	1100 ms	20.9 MB	2360 kB
	✅	✅	2480 ms	658 ms	1820 ms	45.9 MB	2360 kB

Conclusions about performances tests:

This new container can slow down some pages.
- Front migration can optimize performance in some way.
Use one or multiples services within the container v2, don't get slower the front.
Use new container with dev mode may slow down PrestaShop in front.

nicosomb · 2023-05-26T08:45:53Z

FYI, @FabienPapet opened (and closed) this PR #29997

FabienPapet · 2023-07-17T20:43:15Z

+    public function __construct(string $environment, bool $debug)
+    {
+        parent::__construct($environment, $debug);
+    }


FabienPapet · 2023-07-17T20:46:31Z

+        $container->addCompilerPass(new LoadServicesFromModulesPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, self::LOAD_MODULE_SERVICES_PASS_PRIORITY);
+        $container->addCompilerPass(new LoadServicesFromModulesPass($this->appType), PassConfig::TYPE_BEFORE_OPTIMIZATION, self::LOAD_MODULE_SERVICES_PASS_PRIORITY);


This is super dangerous. The DI should never know if we are in a frontend or a backend context. This information should come from the request for example, but should not be used inside a compiler pass.

lartist · 2023-07-17T15:45:11Z

We should remove this file since we already have .env

lartist · 2023-07-18T02:22:48Z

+    public function __construct(string $environment, bool $debug)
+    {
+        parent::__construct($environment, $debug);
+        $this->appType = AppKernel::TYPE_ADMIN;


I think appType it's not handy. We should remove it and based the comparison on $kernel::class (which should be either AdminKernel or FrontKernel)

lartist · 2023-07-18T02:23:30Z

Where is this class used?

Hlavtox · 2023-07-18T05:09:24Z

FO cart is already extremely to slow, to a point where it's on the edge of usability, we can't offer to have this slowdown :(

boherm · 2023-07-18T08:10:26Z

Hi @Hlavtox, yes and it's a big slow.
But, I really don't why, I can't reproduce this time again... Now, I don't have a big time delta like this.

To know what is going on, I want to reinstall a fresh PrestaShop instance and retests some pages like I did.
I wonder if it's a Symfony kernel cache issue that can be really ready after dozen requests.

tleon

LGTM, tho I don't think I know the topic enought to aprouve this on my own. Would love to have other inputs

mflasquin · 2023-08-22T14:18:49Z

+services:
+  _defaults:
+    public: true
+
+  prestashop.core.filter.front_end_object.main:
+    class: PrestaShop\PrestaShop\Core\Filter\FrontEndObject\MainFilter
+    autoconfigure: true
+    arguments:
+      - cart: '@prestashop.core.filter.front_end_object.cart'
+        customer: '@prestashop.core.filter.front_end_object.customer'
+        shop: '@prestashop.core.filter.front_end_object.shop'
+        configuration: '@prestashop.core.filter.front_end_object.configuration'
+
+  prestashop.core.filter.front_end_object.product:
+    class: PrestaShop\PrestaShop\Core\Filter\FrontEndObject\ProductFilter
+
+  prestashop.core.filter.front_end_object.cart:
+    class: PrestaShop\PrestaShop\Core\Filter\FrontEndObject\CartFilter
+    arguments:
+      - '@prestashop.core.filter.front_end_object.product_collection'
+
+  prestashop.core.filter.front_end_object.search_result_product:
+    class: PrestaShop\PrestaShop\Core\Filter\FrontEndObject\SearchResultProductFilter
+
+  prestashop.core.filter.front_end_object.customer:
+    class: PrestaShop\PrestaShop\Core\Filter\FrontEndObject\CustomerFilter
+
+  prestashop.core.filter.front_end_object.shop:
+    class: PrestaShop\PrestaShop\Core\Filter\FrontEndObject\ShopFilter
+
+  prestashop.core.filter.front_end_object.configuration:
+    class: PrestaShop\PrestaShop\Core\Filter\FrontEndObject\ConfigurationFilter
+
+  prestashop.core.filter.front_end_object.product_collection:
+    class: PrestaShop\PrestaShop\Core\Filter\CollectionFilter
+    calls:
+      - method: queue
+        arguments:
+          - [ '@prestashop.core.filter.front_end_object.product' ]
+
+  prestashop.core.filter.front_end_object.search_result_product_collection:
+    class: PrestaShop\PrestaShop\Core\Filter\CollectionFilter
+    calls:
+      - method: queue
+        arguments:
+          - [ '@prestashop.core.filter.front_end_object.search_result_product' ]
+
+  prestashop.adapter.module.repository.module_repository:
+    class: 'PrestaShop\PrestaShop\Adapter\Module\Repository\ModuleRepository'
+    arguments:
+      - !php/const _PS_ROOT_DIR_
+      - !php/const _PS_MODULE_DIR_
+
+  prestashop.translation.translator_language_loader:
+    class: PrestaShopBundle\Translation\TranslatorLanguageLoader
+    arguments:
+      - '@prestashop.adapter.module.repository.module_repository'


Regarding this ADR, can you use FQCN for services names https://github.com/PrestaShop/ADR/blob/master/0019-FQCN-Autowiring.md ?

This is a configuration for legacy services used in front, and I can't really switch to FQCN without some big changes that can introduce some big breaking changes (and if we delete theses alias).
But, I think that can be done in another PR and why not optimize this system too.
What do you think about it?

matks

Implementation is OK for me 👍

There's a little comment from @mflasquin I have nothing to add

Hlavtox · 2023-08-23T13:23:14Z

@matks It cannot be merged if it slows down cart loading 3 times in production mode

boherm · 2023-08-28T13:58:12Z

@Hlavtox, after some new benchmarks with this system, I don't be able to reproduce the loading X3 in production mode.
In fact, I've got in all new tests arround 310ms in cart page with this implementation and in production mode.
(I've update the table in description with new values)

So I think that can be ok to merge seen that this implementation is behind a feature flag.

M0rgan01

Thank you for the performance comparison table :) The changes announce a good thing

boherm · 2023-08-29T10:00:27Z

Autotests (without new container activated)
Autotests (with new container activated)

aniszr

Hello @boherm

I tested your PR and it LGTM!
The modifications done on the.env file didn't cause any problems on the FO:heavy_check_mark:

See attached screenshots:

Auto Test (without container activated): https://github.com/aniszr/ga.tests.ui.pr/actions/runs/6010082838
Auto Test (container activated): https://github.com/boherm/ga.tests.ui.pr/actions/runs/6010749136
QA ✔️

Thanks!

prestonBot · 2023-08-29T12:33:01Z

QA approved, well done! Message to the maintainers: do not forget to milestone it before the merge.

mflasquin · 2023-08-29T14:05:31Z

Big thanks to @boherm for this PR !

matks · 2023-08-29T15:23:57Z

@mflasquin Don't forget the milestone

mickaelandrieu · 2024-06-12T12:30:00Z

This was already a feature (see https://github.com/PrestaShop/PrestaShop/blob/develop/classes/container/LegacyCompilerPass.php), you have to tag a service with the tag "front" to make it available in every legacy controller.

What's the difference with this new container ?

An old exemple of integration => https://github.com/friends-of-presta/masterclass/blob/master/config/services.yml#L53

PululuK · 2025-02-11T15:34:54Z

Hummm good point @mickaelandrieu !

boherm requested a review from a team as a code owner May 25, 2023 16:36

prestonBot added develop Branch Feature Type: New Feature labels May 25, 2023

boherm marked this pull request as draft May 25, 2023 16:36

prestonBot added the BC break Type: Introduces a backwards-incompatible break label May 25, 2023

boherm mentioned this pull request May 25, 2023

Add Symfony container front switch PrestaShop/ga.tests.ui.pr#34

Closed

boherm force-pushed the #31846-symfony-front-container branch 4 times, most recently from b2d8220 to bcf3bcd Compare May 31, 2023 12:44

FabienPapet reviewed May 31, 2023

View reviewed changes

Comment thread classes/Hook.php Outdated

boherm force-pushed the #31846-symfony-front-container branch from bcf3bcd to 2611455 Compare May 31, 2023 12:49

boherm closed this May 31, 2023

boherm reopened this May 31, 2023

boherm force-pushed the #31846-symfony-front-container branch 2 times, most recently from 148aba0 to b58a3ea Compare May 31, 2023 13:53

kpodemski reviewed Jun 1, 2023

View reviewed changes

Comment thread src/Adapter/Presenter/Product/ProductLazyArray.php Outdated

boherm force-pushed the #31846-symfony-front-container branch 3 times, most recently from 62a492b to fb55a48 Compare July 6, 2023 15:51

boherm marked this pull request as ready for review July 7, 2023 07:22

boherm force-pushed the #31846-symfony-front-container branch from fb55a48 to 3ea9881 Compare July 11, 2023 14:49

FabienPapet reviewed Jul 17, 2023

View reviewed changes

lartist reviewed Jul 18, 2023

View reviewed changes

tleon reviewed Jul 19, 2023

View reviewed changes

boherm force-pushed the #31846-symfony-front-container branch from 6a9d8b7 to 311bc21 Compare August 21, 2023 13:08

boherm marked this pull request as ready for review August 22, 2023 07:29

boherm changed the title ~~[WIP] Load symfony container in front~~ Load symfony container in front Aug 22, 2023

mflasquin reviewed Aug 22, 2023

View reviewed changes

boherm added 2 commits August 22, 2023 17:56

Load symfony container in front

72b204a

Add PS_FF_FRONT_CONTAINER_V2 feature flag

1369ab7

boherm force-pushed the #31846-symfony-front-container branch from 5cdb1e7 to 1369ab7 Compare August 22, 2023 15:56

matks approved these changes Aug 23, 2023

View reviewed changes

M0rgan01 approved these changes Aug 29, 2023

View reviewed changes

ps-jarvis added the Waiting for QA Status: action required, waiting for test feedback label Aug 29, 2023

kpodemski approved these changes Aug 29, 2023

View reviewed changes

aniszr approved these changes Aug 29, 2023

View reviewed changes

aniszr self-assigned this Aug 29, 2023

aniszr added QA ✔️ Status: check done, code approved and removed Waiting for QA Status: action required, waiting for test feedback labels Aug 29, 2023

mflasquin merged commit bea5f35 into PrestaShop:develop Aug 29, 2023

matks added the Key feature Notable feature to be highlighted label Aug 29, 2023

matks added this to the 9.0.0 milestone Aug 29, 2023

matks mentioned this pull request Sep 21, 2023

Cannot use symfony services in front end hook #29013

Open

2 tasks

matks mentioned this pull request Nov 28, 2023

Script admin/init.php fails on develop #34717

Closed

2 tasks

boherm mentioned this pull request Nov 28, 2023

Fix use of front_legacy_context constant in FrontController #34721

Merged

boherm deleted the #31846-symfony-front-container branch July 16, 2024 09:34

kpodemski added the Documentation ✔️ Developer documentation is up-to-date label Jul 31, 2024

		$container->addCompilerPass(new LoadServicesFromModulesPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, self::LOAD_MODULE_SERVICES_PASS_PRIORITY);
		$container->addCompilerPass(new LoadServicesFromModulesPass($this->appType), PassConfig::TYPE_BEFORE_OPTIMIZATION, self::LOAD_MODULE_SERVICES_PASS_PRIORITY);

Uh oh!

Conversation

boherm commented May 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicosomb commented May 26, 2023

Uh oh!

Uh oh!

Uh oh!

FabienPapet Jul 17, 2023

Choose a reason for hiding this comment

Uh oh!

FabienPapet Jul 17, 2023

Choose a reason for hiding this comment

Uh oh!

lartist Jul 17, 2023

Choose a reason for hiding this comment

Uh oh!

lartist Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

lartist Jul 18, 2023

Choose a reason for hiding this comment

Uh oh!

Hlavtox commented Jul 18, 2023

Uh oh!

boherm commented Jul 18, 2023

Uh oh!

tleon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mflasquin Aug 22, 2023

Choose a reason for hiding this comment

Uh oh!

boherm Aug 23, 2023

Choose a reason for hiding this comment

Uh oh!

matks left a comment

Choose a reason for hiding this comment

Uh oh!

Hlavtox commented Aug 23, 2023

Uh oh!

boherm commented Aug 28, 2023

Uh oh!

M0rgan01 left a comment

Choose a reason for hiding this comment

Uh oh!

boherm commented Aug 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aniszr left a comment

Choose a reason for hiding this comment

Uh oh!

prestonBot commented Aug 29, 2023

Uh oh!

mflasquin commented Aug 29, 2023

Uh oh!

matks commented Aug 29, 2023

Uh oh!

mickaelandrieu commented Jun 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

PululuK commented Feb 11, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

15 participants

boherm commented May 25, 2023 •

edited

Loading

boherm commented Aug 29, 2023 •

edited

Loading

mickaelandrieu commented Jun 12, 2024 •

edited

Loading