Move JS module initialization to separate tasks

[krzksz]

Description (*)

I'm currently during a process of profilling and trying to improve Magento frontend performance as much as possible. This is one of the improvements that I found.

The Issue

As you can see on the below screenshot First CPU idle and Max Potential First Input Delay times are not great and it would be nice to improve them.

And so I run a quick performance profile, with 4x CPU slowdown for easier spotting and here's what I found:

Those two big long-running tasks are coming from Magento initializing all data-mage-init modules. The first one is responsible for collecting all the data needed for initialization which happens in the same task as initializing custom Knockout bindings, hence change in lib/web/mage/bootstrap.js. The second one is initialization of each of the modules themselves which can also be done in separate tasks like you see in changed lib/web/mage/apply/main.js.

The Fix

Here's the result after applying both changes:


As you can see both metrics improved dramatically. Next step would be to reduce this huge TTI but until it happens it would be great to make sure we are not blocking the main thread for too long preventing it from handling user interactions.

Manual testing scenarios (*)

This is not a functional change, everything should we working as before.

Questions or comments

Since this is not a functional change, as long as all tests are still passing I would consider this change covered with tests.

Contribution checklist (*)

• Pull request has a meaningful description of its purpose
• All commits are accompanied by meaningful commit messages
• All new or changed code is covered with unit/integration tests (if applicable)
• All automated tests passed successfully (all builds are green)

[m2-assistant]

Hi @krzksz. 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 give me test instance - deploy test instance based on PR changes
• @magento give me 2.4-develop instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Guide documentation.

[krzksz]

@magento give me test instance

[magento-engcom-team]

Hi @krzksz. Thank you for your request. I'm working on Magento instance for you

[ihor-sviziev]

@krzksz good catch!
Let's wait for test results

[magento-engcom-team]

Hi @lbajsarowicz, thank you for the review.
ENGCOM-7146 has been created to process this Pull Request
✳️ @lbajsarowicz, could you please add one of the following labels to the Pull Request?

Label	Description
Auto-Tests: Covered	All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered	Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required	Changes in Pull Request does not require coverage by auto-tests

[magento-engcom-team]

Hi @ptylek, thank you for the review.
ENGCOM-7146 has been created to process this Pull Request
✳️ @ptylek, could you please add one of the following labels to the Pull Request?

Label	Description
Auto-Tests: Covered	All changes in Pull Request is covered by auto-tests
Auto-Tests: Not Covered	Changes in Pull Request requires coverage by auto-tests
Auto-Tests: Not Required	Changes in Pull Request does not require coverage by auto-tests

[ihor-sviziev]

Hi @krzksz ,
Amazing improvement!
Could you fix failing static tests?

Pull Request state was updated. Re-review required.

[ihor-sviziev]

@krzksz do you think it’s possible to cover your changes with some tests?

[ihor-sviziev]

Just reviewed test failures - looks like they’re not related to changes from this PR, some failures on MSI. To make sure I just restarted this functional test

[krzksz]

@ihor-sviziev I think I fixed failing static test (there was line break missing) but I don't think this change should be covered by any additional tests.

My point is that as long as existing functional tests are passing, it means that all frontend modules are still initialized properly. Otherwise I would have to add tests that are heavily coupled to the implementation which would do more harm then good.

[krzksz]

Static tests failing with copyPaste error which is a false-positive.

[magento-engcom-team]

Hi @ihor-sviziev, thank you for the review.
ENGCOM-7146 has been created to process this Pull Request

[ihor-sviziev]

@krzksz approved!
I also think that it’s impossible to cover this change with meaningful tests, so let’s not cover it.
Thank you so much for such improvement!

[engcom-Alfa]

✔️ QA Passed

[engcom-Echo]

Failed functional tests not related to the changes in this PR

[m2-assistant]

Hi @krzksz, 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.

[ilnytskyi]

@krzksz
Would the same changes be applicable for backend js?

file:
lib/web/mage/backend/bootstrap.js

this file is seem to be used by backend
lib/web/mage/apply/main.js

[krzksz]

In theory yes, it's that storefront was my priority because it affects much more people, their experience and actual buying process.