- Issue created by @jsacksick
- Merge request !245Issue #3533739 by jsacksick: Implement static caching around SchedulerManager::getPlugins(). → (Merged) created by jsacksick
- 🇮🇱Israel jsacksick
Currently, when a provider is passed, the persistent cache is bypassed. With this change, we always use the static cache if present, then fallback to the persistent cache. The provider filtering is done after the fact. I think this makes more sense than bypassing the persistent cache entirely when a $provider is passed.
- 🇬🇧United Kingdom jonathan1055
This looks interesting, thanks for working on it.
I've just merged 🐛 Unhandled Warning: Undefined array key "translatable" Active and would like to tag a new release soon, so maybe we can get this improvement in too? - 🇮🇱Israel jsacksick
I think this is an interesting fix, the
getPlugins()
method is called a lot and currently each call to that method will make a request to the cache backend (in my case Redis), For example, 8 cache get calls on a simple add to cart request.I'm actually wondering if the persistent cache fully makes sense here, because
doGetPlugins()
calls$this->getPluginDefinitions();
Which gets the definitions from the plugin manager:$this->pluginManager->getDefinitions();
This invokes the
getDefinitions()
method on theDefaultPluginManager
which has the following logic:/** * {@inheritdoc} */ public function getDefinitions() { $definitions = $this->getCachedDefinitions(); if (!isset($definitions)) { $definitions = $this->findDefinitions(); $this->setCachedDefinitions($definitions); } return $definitions; }
So the plugin definitions already have static cache + persistent cache handling. So we define our own cache on top of the core cache handling.
Our own persistent cache probably still makes sense otherwise we'll perform the following logic over and over:
$definitions = $this->getPluginDefinitions(); foreach ($definitions as $definition) { $plugin = $this->pluginManager->createInstance($definition['id']); $dependency = $plugin->dependency(); // Ignore plugins if there is a dependency module and it is not enabled. if ($dependency && !$this->moduleHandler->moduleExists($dependency)) { continue; } $this->plugins[$plugin->entityType()] = $plugin; }
- 🇬🇧United Kingdom jonathan1055
I notice you have fixed a dependency injection, but we have 📌 Fix \Drupal calls should be avoided in classes, use dependency injection Active already, so I'd prefer if we keep this issue in scope. Was there a particular need for that to be fixed here? If not I will undo that, otherwise there will be a conflict.
- 🇬🇧United Kingdom jonathan1055
I've merged in 2.x and resolved the conflict, so there's nothing more to do regarding #8.
-
jonathan1055 →
committed 89456f77 on 2.x authored by
jsacksick →
Issue #3533739 by jsacksick: Implement static caching around...
-
jonathan1055 →
committed 89456f77 on 2.x authored by
jsacksick →
- 🇬🇧United Kingdom jonathan1055
Merged. Thank you very much for doing this work, it is a good improvement.