- Issue created by @catch
- 🇬🇧United Kingdom catch
Instead of doing this in the url generator we can probably do it in AliasManager::getAliasByPath() instead.
Add the system path to a class property ready for preloading, Fiber::suspend(), when we get resumed, look for any paths to multiple/preload that haven't already been, multiple load, continue.
The main question is how many times can we ::suspend() before we resume, but we only need to pick up two path aliases each time to reduce the number of queries by 50% so the odds might be decent. The existing solution has built in staleness where subsequent page requests will individually lookup paths that weren't found when the preload cache was set.
- 🇬🇧United Kingdom catch
Put up a proof of concept here but it doesn't do anything useful on standard yet - almost nothing is rendered in a placeholder and that's the only inside-fiber rendering we do.
So this will need 📌 Create placeholders for more things Active - at least a rebase of the MR there and some combined testing, in order to see whether it can be viable.
I did add some views blocks (recent comments, recent content) to the footer along with adding some alias for things like comment/reply, and saw this working - so in theory it has potential just not in practice yet.
- Merge request !10792Combine path alias lazy loading and renderer fiber support → (Open) created by catch
- 🇬🇧United Kingdom catch
OK this actually works.
Manual testing with https://git.drupalcode.org/issue/drupal-3496369/-/tree/3496369-with-rend...
With a completely cold cache on an Umami recipe page, there are 38 calls to
Drupal\path_alias\AliasRepository::lookupBySystemPath
+ the cache get and set for the preload cache.With the MR, there are 31 calls to
Drupal\path_alias\AliasRepository::preloadPathAlias
and no cache get/set.With a warm render cache, there will be zero path alias gets at all because the resultant URLs are in the render cache anyway.
So the worst case scenario is improved, and the best case scenario is the same.
When the path alias cache is warm but the render cache is not (multiple roles viewing the same path for example), then there will probably be more database queries - but what's in the cache will depend on which role visits the page first, so it's not like preloading gets the queries down to one, there's always some stragglers.
- 🇬🇧United Kingdom catch
Then I added 📌 Create placeholders for more things Active locally and got down to 14 calls to preloadPathAlias() - this is nearly a 2/3rds reduction in path lookup queries on a cold cache. Because the cache is per-URL it will be a similar reduction across all the first visits to a URL on a site after a cache clear or each time the cache expires if the render cache for that page isn't warm."
- 🇬🇧United Kingdom catch
This issue + 📌 Create placeholders for more things Active seems to be enough without 📌 Render children in fibers Active , which is good because changing the renderer like that is a bit alarming.
However when testing I ran into the exception that 📌 Deprecate RendererInterface::render()'s sole $is_root_call parameter Needs work is trying to remove, so we will need that issue too to make this all work together.
The actual MR here still has a couple of test failures in path alias tests, but it'll be reviewable by itself without the other two issues - just won't be a performance improvement (or a very marginal one) until those also land.
Not exactly sure what test coverage looks like for this - we can probably set up some Fibers in a kernel test that call getAliasByPath() and check that the aliases are still returned correctly.
📌 Add assertions to OpenTelemetryNodePagePerformanceTest::testNodePageCoolCache() Active would hopefully cover performance testing once it's in, alongside the other two issues.