#3496369: Multiple load path aliases without the preload cache uses Fibers to collect a list of path aliases to load, then loads them when the first fiber in a set is resumed again. This works when multiple placeholders load different path aliases, but not when you have a listing of 50 nodes in a view - because that will be a single placeholder (or not a placeholder at all).
If we render views rows in Fibers, then the path alias preloading should also work for each item in a views listing, which is potentially a reduction of dozens of database queries on cold render caches.
Blocked on #2511330: Deprecate RendererInterface::render()'s sole $is_root_call parameter and #3518179: Renderer::executeInRenderContext() needs to handle nested calls and suspended fibers.
Show commandsStart 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
catchTo make this work, should look something like this:
- we need to get the entity ID + langcode in the plugin to pass to the lazy builder, this can be done by slightly reworking/copying getEntityTranslationByRelationship().
- a new service providing the lazy builder callback itself, which takes the entity id + langcode, loads the entity/translation and renders it.
Comment #3
berdirI thought about this, but I'm not quite sure if this is really the right thing to do.
This would pull all these entities out of dynamic page cache as well, is that really what we want to do? Maybe, but also seems to have fairly big implications that we should carefully think through.
should we also exclude them from bigpipe by default?
for views in blocks, this will result in placeholders within placeholders?
what about entity references, comments?
some entities might be not be cacheable, that could be costly. People do use views to render paragraphs for example more often than I'd like.
Comment #4
catchGenerally the entity view itself gets render cached, and that will get multiple loaded by CachedPlaceholderStrategy, so it ought to be OK.
But... not in this case and I hadn't thought about that.
Another way to approach this would to render the entities (or more likely views rows) within fibers but not placeholders.
A bit like #3496835: [PP-2] Render children in fibers although assuming we'll try to avoid doing that actual issue.
Comment #5
catchLooks like
StylePluginBase::renderGroupingSets()is the place to try using Fibers, which should produce the same result with path alias preloading but without actual placeholdering.Comment #6
catchComment #7
catchComment #8
catchComment #10
catchComment #11
catchPushed a branch with a couple of commits, needs cleaning up but it works in combination with #3496369: Multiple load path aliases without the preload cache to preload all the aliases rendered within views rows.
Comment #12
catchThe test failures are (mostly? all?) due to known issues - added to issue summary.
Comment #13
catchBlockers are in.
Comment #14
berdirI think there's a lot of potential here, it's not uncommon that views display 20, 50 or more results and often, they all contain a media, an alias and so on, so many entities and aliases that could be loaded in bulk.
I had a look at some test fails, but there are things that make this very tricky.
I looked into \Drupal\Tests\views\Kernel\Plugin\RowRenderCacheTest::doTestRenderedOutput(), which uses the counter field, which relies on $this->view->row_index in \Drupal\views\Plugin\views\field\Counter::getValue. this iindex is now incremented at the beginning of \Drupal\views\Plugin\views\style\StylePluginBase::renderFieldRow(), as they all then suspend, it means the counter is then always 6 for every row. The fix for this specific plugin we can switch to $values->index, and I guess deprecate that global counter then.
Going further, it only gets weirder from here. The comment admin test fail has a fatal error because \Drupal\comment\CommentInterface::getCommentedEntity() returns NULL, and also the following warning:
Exception: Warning: Undefined property: Drupal\Core\Field\EntityReferenceFieldItemList::$entity
The thing is, EntityReferenceFieldItemList has a __get() method, but it doesn't go in there, at least not the second time. I can reproduce manually, just create two comments an then visit the comment admin view. I tried to reproduce this in a minimal script, but that works: https://3v4l.org/089uP#v8.4.12
Comment #15
berdirThis is a known PHP bug: https://github.com/php/php-src/issues/14983, there is a workaround to not use the magic ->get as done in the latest commit here.
Comment #16
mably commented[Removed]
Comment #17
geek-merlinReally nice! Inspired me to create the related issue. I guess both of you @catch and @berdir have some thoughts on it.