- Issue created by @deviantintegral
- heddn Nicaragua
I'm seeing similar performance issues on a site too. Going to see what can be done about it.
- First commit to issue fork.
- Assigned to shumer
- Merge request !34Issue #3399222 by shumer: Add a cache for config page loads β (Open) created by shumer
- last update
about 1 year ago 7 pass - π΅π±Poland shumer Wroclaw
Hey folks!
@heddn and @deviantintegral would you be able to try the code from the PR? It solves the issue for me.The other question I have, what was the use case for the ConfigPages for both of you? I mean I can hardly imagine the situation like this, so I'm interested in what are you doing with this module? Hope that will help me to avoid issues like that in future releases.
Thx
- Status changed to Needs review
about 1 year ago 1:01pm 27 December 2023 - π«π·France nicolas bouteille
Hello,
In the code of the PR 34, I see the $cache variable lives in the config() function.
Am I right to assume that the config page entity will be "cached" only during a single session ? Or maybe even a single request ? And that it only relieves the site when too many loads are done for a single request, but has no effect if multiple visitors load the config page in a short time?In my website, I would like to check a config page's value on every single request made to the website.
The config page's value will not change frequently at all. So loading the config page entity on every page request or every session is overkill.
Which is why I am considering caching the data I need and removing it from cache whenever the config page is updated.
But I was wondering⦠do I really need to do this myself or config page / Drupal is already caching stuff?So I stumbled upon this issue and it looks like no caching is done...
As mentioned above, no cache is made with loadByProperties() unlike load().
So as suggested above, shouldn't we use load() when context is null? Or is it because when context is null, we still need to fetch and specify the default context that we cannot do that?BTW, on this documentation page : https://www.drupal.org/docs/contributed-modules/config-pages/usage-of-co... β
The third option is to get a config page via storage manager:$storage = \Drupal::entityTypeManager()->getStorage('config_pages'); $entity = $storage->load($config_page_machine_name);
this does mention the context... so is the context not that important, or is the documentation incomplete?
If we cannot use load() instead of loadByProperties(), what do you think of caching the config page entity using cache_set() and remove it when it is updated? Or is it too heavy and we'd better only store simple values that we need to access frequently?
- π«π·France nicolas bouteille
Ok I have spotted that $storage->load() has been overridden in ConfigPagesStorage to actually call ConfigPages::config($id); unless the $id is numeric...
for that reason, calling $storage->load() inside ConfigPages::config would create an infinite loop
also I thought calling $storage->load('my_config_page') directly would be more efficient than using ConfigPagesLoaderService or ConfigPages::config because they use loadByProperties, but in the end all those three methods call loadByProperties in the end... - π΅π±Poland shumer Wroclaw
The way the entity is loaded here is not like how other entities in Drupal work. The reason for that is that we have Context enabled, so while the entity is being loaded, we need to evaluate which one exactly we need. We can't cache that enity between requests as the context might be different. In case of specific need you can cache the load in the custom code.
When it comes to a static cache we can add that option, but I'm still curious about the use case. I haven't seen anything like this before, despite I'm using that module since 2014... Can anybody from this topic provide me some info about the use case when this performance degradation appears?