The 2026 Drupal Business Survey is open. Your response will help shape the future of the Drupal business ecosystem.Problem/Motivation
Instantiating entity storages is expensive and we now have rules against doing it when you don't need to storage. But it's still expensive when you do, so that's only partially a solution.
The reason it's expensive is mostly getting the active entity type and field storage definitions.
Sister issue to #3616962: Do not load installed field storage definitions in ContentTranslationHandler::__construct()
Blocker for #2345611: [pp-1] Load user entity in Cookie AuthenticationProvider instead of using manual queries, which exposes this quite nicely.
Steps to reproduce
Proposed resolution
As a first step, stop getting the active definitions in the constructor, but load them lazy. I don't have a full picture yet how far we get without them, definitely can't be avoided if we have a cache miss, but we might for cache hits at least?
More complex but likely follow-up ideas would be reduce/split those objects, we've had vague ideas on that since we introduced this concept, but it's obviously not easy.
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | Screenshot_20260816_231146.png | 27.61 KB | berdir |
Issue fork drupal-3617141
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
berdirComment #3
berdirGood news: on umami with warm entity caches, just commenting those lines out saves about 2MB of memory (28 -> 26) and nothing breaks. On my larger site, it's around 6MB.
Less good news: the constructor also calls initTableLayout which initializes a whole bunch of properties and needs the active entity type and field storage definitions. To make it lazy, we need to make all of that lazy. More complications are due to the fact that entityType is defined in the parent. The only feasible option to make this work I think is property hooks on all of them, which needs phpcs workarounds.
Another related issue: #3617152: EntityLastInstalledSchemaRepository::getLastInstalledDefinition() loads all definitions
Comment #5
berdirNote on the performance tests: I think something there was off, not exactly sure what I was seeing, additional work was necessary for it to actually function on the happy path (persistent cache hit). It's still around 2MB, but the totals might be off there, for anon it now . And I haven't verified it on my real site yet, I just meant that the potential for saving is around 6MB.
It seems to work quit well now, but I had to make changes to the field storage definition updates, so that's a BC break there, but it's unlikely to have more code like that. And there's the phpcs tradeoff of course. Looking at the phpcs issue, this will not be resolved anytime soon it seems.
There are more test fails, some performance, some that might be unrelated, aware of the MR comment, but I'd like feedback on the general direction before finishing this.
Blackfire summary:
(time might be off, but memory should be fairly accurate)
Comment #6
moshe weitzman commentedI dont think that phpcs debates should block code improvements. We should be able to enter exceptions and move on. My .02.
Comment #7
catchYeah agreed with @moshe in #6. We've only got a handful of use-cases for property hooks so far, maybe we'll end up with a couple of dozen or so. If we accumulate a few cs errors in a few files, those are easy to clean up when phpcs supports property hooks, but the problems we can fix with property hooks are very hard or impossible to fix otherwise.
Comment #8
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.
Comment #9
berdirComment #10
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".
This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.
Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.