Cache the results of isCacheable calls to improve performance #40112

igorwulff · 2025-07-30T16:04:18Z

This function call is relatively expensive when $elements has the full layout in it. Especially the PageCache module is calling this function a lot from the following places:

Magento\PageCache\Model\Layout\LayoutPlugin
Magento\PageCache\Observer\ProcessLayoutRenderElement

On my local machine I've seen the impact to be anywhere between 20-40 milliseconds, when the page has been loaded a few times to warm up cache (config, block, layout, etc...), but FPC is circumvented.

We could also opt to move this to the PageCache module, but I think it is ok to optimize this within the Layout class within the framework.

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Fixes magento/magento2#<issue_number>

Manual testing scenarios (*)

...
...

Questions or comments

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)
README.md files for modified modules are updated and included in the pull request if any README.md predefined sections require an update
All automated tests passed successfully (all builds are green)

Resolved issues:

resolves [Issue] Cache the results of isCacheable calls to improve performance #40156: Cache the results of isCacheable calls to improve performance

This function call is relatively expensive when $elements has the full layout in it. Especially the PageCache module is calling this function a lot from the following places: - Magento\PageCache\Model\Layout\LayoutPlugin - Magento\PageCache\Observer\ProcessLayoutRenderElement On my local machine I've seen the impact to be anywhere between 20-40 milliseconds, when the page has been loaded a few times to warm up cache (config, block, layout, etc...), but FPC is circumvented. We could also opt to move this to the PageCache module, but I think it is ok to optimize this within the Layout class within the framework.

m2-assistant · 2025-07-30T16:04:22Z

Hi @igorwulff. Thank you for your contribution!
Here are some useful tips on how you can test your changes using Magento test environment.
❗ Automated tests can be triggered manually with an appropriate comment:

@magento run all tests - run or re-run all required tests against the PR changes
@magento run <test-build(s)> - run or re-run specific test build(s)
For example: @magento run Unit Tests

<test-build(s)> is a comma-separated list of build names.

Allowed build names are:

Database Compare
Functional Tests CE
Functional Tests EE
Functional Tests B2B
Integration Tests
Magento Health Index
Sample Data Tests CE
Sample Data Tests EE
Sample Data Tests B2B
Static Tests
Unit Tests
WebAPI Tests
Semantic Version Checker

You can find more information about the builds here
ℹ️ Run only required test builds during development. Run all test builds before sending your pull request for review.

For more details, review the Code Contributions documentation.
Join Magento Community Engineering Slack and ask your questions in #github channel.

igorwulff · 2025-07-31T08:48:28Z

@magento run all tests

lbajsarowicz

✅ The change looks good to me.

The method you're improving, originally did not look like a good example of code, however.

I was considering whether ?bool wouldn't be better than bool|null but there's no functional difference. I'm personally more towards ?bool

thomas-kl1 · 2025-08-04T13:06:23Z

lib/internal/Magento/Framework/View/Layout.php

-            if ($blockName !== false && $this->structure->hasElement($blockName)) {
-                $cacheable = false;
-                break;
+        if (!isset($this->isCacheableCache)) {


Why not using use comparator here? $this->isCacheableCache === null
We are sure that isCacheableCache property exists, so we can avoid this check

ArjenMiedema · 2025-08-04T14:20:26Z

Just tried this patch and when you have all caches enabled, it will broke my customer account pages and checkout pages, since it won't be able to properly fetch the customer's ID. Could also be my installation, but reverting this patch did solve the issues.

engcom-Charlie · 2025-08-26T06:38:40Z

@magento create issue

engcom-Charlie · 2025-09-04T12:10:04Z

✔️ QA Passed

Steps to reproduce

Created one custom script to test this performace improvement. After caching the isCacheable result is giving faster results.

Before: ❌

After: ✔️

Many Functional CE, B2B, EE tests are failing because of the PR changes.

eg. https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/40112/72848e021a98a44835eeff89b84c343b/Functional/allure-report-ce/index.html#categories/dd9384a199a6c054c5f17cb5b8083038/422e4a77ca570a6f/ there are 156 tests are failing

Moving this PR to Extended testing to work on build stability.

engcom-Charlie · 2025-09-05T09:41:50Z

Functional B2B, CE, EE are failing, working on it.

B2B:

CE:

EE:

engcom-Charlie · 2025-09-05T09:45:21Z

@magento run all tests

engcom-Charlie · 2025-09-08T05:27:32Z

@magento run Functional Tests B2B, Functional Tests CE, Functional Tests EE

engcom-Charlie · 2025-09-08T13:35:11Z

Fixed all mentioned test failures. @igorwulff and @lbajsarowicz requesting for review again.

engcom-Hotel · 2025-09-09T06:42:29Z

@magento run all tests

engcom-Hotel

The changes seems good to me and the failed tests seems flaky, hence approving the PR.

engcom-Hotel · 2025-09-09T15:25:03Z

@magento run Unit Tests

engcom-Charlie · 2025-09-10T05:48:39Z

@magento run all tests

engcom-Charlie · 2025-09-10T09:14:48Z

The testing has been done, as mentioned in here. Since the build is failing moving it to Extended Testing.

engcom-Charlie · 2025-09-10T09:21:39Z

@magento run Functional Tests B2B, Functional Tests CE, Functional Tests EE, Integration Tests, Unit Tests

engcom-Charlie · 2025-09-10T12:54:34Z

Few Functional B2B, CE and EE are not consistent in recent 2 builds, they seems to be flaky, neither part of PR nor failing because of PR changes. The consistent once are known issues, hence moving the PR to Merge in Progress.

Functional B2B Run 1:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/40112/a65e06274e6741654cd2329d4bdb4298/Functional/allure-report-b2b/index.html#categories

Run 2:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/40112/306dca6d3075b49cc4661c18dedb5531/Functional/allure-report-b2b/index.html#categories/fec1778d90c7330e1027163b3b866d7a/765c1c435ae1eeac/

Known issue:
AC-8440: StorefrontCheckTaxAddingValidVATIdTest
ACQE-8689: AdminVerifyBehaviorOfDefaultStatusForProcessingStateTest
ACQE-8645: StorefrontCreatePurchaseOrderFromNegotiableQuoteTest

Functional CE Run 1:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/40112/a7020667e07de03520ec2f1e4a662ed3/Functional/allure-report-ce/index.html

Run 2:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/40112/330453520fb5456c5fb8241a20f4ac3e/Functional/allure-report-ce/index.html#categories/d52f54183a27cd1914dd108203055fcd/33cb6b2405a44de0/

Known issue:
AC-8440: StorefrontCheckTaxAddingValidVATIdTest
ACQE-8689: AdminVerifyBehaviorOfDefaultStatusForProcessingStateTest

Functional EE Run 1:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/40112/7eb8ef3ff1fa39887b5420d3ffa94fc9/Functional/allure-report-ee/index.html

Run 2:
https://public-results-storage-prod.magento-testing-service.engineering/reports/magento/magento2/pull/40112/068c6f4477753e642f5ab655833050c1/Functional/allure-report-ee/index.html#categories/d52f54183a27cd1914dd108203055fcd/1c9fa20c8fdcc817/

Known issue:
ACQE-8641: AdminMassOrdersReleasePendingOrderTest
AC-8440: StorefrontCheckTaxAddingValidVATIdTest
ACQE-8689: AdminVerifyBehaviorOfDefaultStatusForProcessingStateTest

engcom-Charlie · 2025-09-10T13:11:17Z

@magento run Functional Tests B2B

Merge branch '2.4-develop' into patch-13

d012449

lbajsarowicz approved these changes Aug 4, 2025

View reviewed changes

thomas-kl1 reviewed Aug 4, 2025

View reviewed changes

engcom-Hotel added this to Pull Requests Dashboard Aug 5, 2025

github-project-automation bot moved this to Pending Review in Pull Requests Dashboard Aug 5, 2025

ct-prd-projects-boards-automation bot added the Progress: pending review label Aug 5, 2025

engcom-Hotel moved this from Pending Review to Changes Requested in Pull Requests Dashboard Aug 5, 2025

ct-prd-projects-boards-automation bot added the Progress: needs update label Aug 5, 2025

engcom-Hotel added Priority: P3 May be fixed according to the position in the backlog. and removed Progress: pending review labels Aug 5, 2025

ct-prd-projects-boards-automation bot moved this from Changes Requested to Pending Review in Pull Requests Dashboard Aug 5, 2025

ct-prd-projects-boards-automation bot added Progress: pending review Progress: review and removed Progress: needs update Progress: pending review labels Aug 5, 2025

ct-prd-projects-boards-automation bot moved this from Pending Review to Review in Progress in Pull Requests Dashboard Aug 5, 2025

engcom-Hotel moved this from Review in Progress to Changes Requested in Pull Requests Dashboard Aug 7, 2025

ct-prd-projects-boards-automation bot added Progress: needs update and removed Progress: review labels Aug 7, 2025

engcom-Charlie added the Project: Community Picked PRs upvoted by the community label Aug 12, 2025

ct-prd-projects-boards-automation bot added this to Community Dashboard Aug 12, 2025

ct-prd-projects-boards-automation bot moved this to Changes Requested in Community Dashboard Aug 12, 2025

ct-prd-projects-boards-automation bot removed this from Pull Requests Dashboard Aug 12, 2025

engcom-Charlie self-assigned this Aug 25, 2025

m2-assistant bot mentioned this pull request Aug 26, 2025

[Issue] Cache the results of isCacheable calls to improve performance #40156

Open

5 tasks

engcom-Charlie moved this from Testing in Progress to Extended testing (optional) in Community Dashboard Sep 4, 2025

engcom-Charlie and others added 2 commits September 5, 2025 15:13

Fixing the functional failures

facbaf2

Merge branch '2.4-develop' into patch-13

23be1dc

engcom-Charlie moved this from Extended testing (optional) to Review in Progress in Community Dashboard Sep 8, 2025

ct-prd-projects-boards-automation bot added the Progress: review label Sep 8, 2025

Merge branch '2.4-develop' into patch-13

75b41ca

engcom-Hotel self-requested a review September 9, 2025 06:43

engcom-Hotel approved these changes Sep 9, 2025

View reviewed changes

ct-prd-projects-boards-automation bot removed the Progress: review label Sep 9, 2025

ct-prd-projects-boards-automation bot moved this from Review in Progress to Ready for Testing in Community Dashboard Sep 9, 2025

ct-prd-projects-boards-automation bot added the Progress: ready for testing label Sep 9, 2025

Merge branch '2.4-develop' into patch-13

53d4859

engcom-Charlie moved this from Ready for Testing to Testing in Progress in Community Dashboard Sep 10, 2025

engcom-Charlie added Progress: testing in progress and removed Progress: ready for testing labels Sep 10, 2025

engcom-Charlie moved this from Testing in Progress to Extended testing (optional) in Community Dashboard Sep 10, 2025

engcom-Charlie removed the Progress: testing in progress label Sep 10, 2025

engcom-Charlie moved this from Extended testing (optional) to Merge in Progress in Community Dashboard Sep 10, 2025

Cache the results of isCacheable calls to improve performance #40112

Are you sure you want to change the base?

Cache the results of isCacheable calls to improve performance #40112

Uh oh!

Conversation

igorwulff commented Jul 30, 2025 • edited by m2-assistant bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description (*)

Related Pull Requests

Fixed Issues (if relevant)

Manual testing scenarios (*)

Questions or comments

Contribution checklist (*)

Resolved issues:

Uh oh!

m2-assistant bot commented Jul 30, 2025

Uh oh!

igorwulff commented Jul 31, 2025

Uh oh!

lbajsarowicz left a comment

Choose a reason for hiding this comment

Uh oh!

thomas-kl1 Aug 4, 2025

Choose a reason for hiding this comment

Uh oh!

ArjenMiedema commented Aug 4, 2025

Uh oh!

engcom-Charlie commented Aug 26, 2025

Uh oh!

engcom-Charlie commented Sep 4, 2025

✔️ QA Passed

Uh oh!

engcom-Charlie commented Sep 5, 2025

Uh oh!

engcom-Charlie commented Sep 5, 2025

Uh oh!

engcom-Charlie commented Sep 8, 2025

Uh oh!

engcom-Charlie commented Sep 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

engcom-Hotel commented Sep 9, 2025

Uh oh!

engcom-Hotel left a comment

Choose a reason for hiding this comment

Uh oh!

engcom-Hotel commented Sep 9, 2025

Uh oh!

engcom-Charlie commented Sep 10, 2025

Uh oh!

engcom-Charlie commented Sep 10, 2025

Uh oh!

engcom-Charlie commented Sep 10, 2025

Uh oh!

engcom-Charlie commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

engcom-Charlie commented Sep 10, 2025

Uh oh!

Uh oh!

igorwulff commented Jul 30, 2025 •

edited by m2-assistant bot

Loading

engcom-Charlie commented Sep 8, 2025 •

edited

Loading

engcom-Charlie commented Sep 10, 2025 •

edited

Loading