Entity lazy multiple front loading

Ten years later...

The main barrier to pursuing this, apart from ::__get() or anything else is that when you call Entity::load()/EntityStorage::load() you either get an entity or FALSE back, you never get an entity object that might or might not exist in the database. That makes the original approach here very hard to reconcile with entity caching - we never want to run a query just to check if we have an entity or not, when we could just get it from cache anyway.

📌 Try to replace path alias preloading with lazy generation Active has a proof of concept for changing how path alias preloading works, it relies on Fibers which didn't exist until PHP 8.1. By doing so it doesn't rely on any changes to entity classes, magic methods etc. and I think the same thing can work here.

Let's say we have three independent entity loading calls in different placeholders (or any code that can run inside a Fiber, which will be most of it after 🌱 Adopt the Revolt event loop for async task orchestration Active .

// Placeholder 1.
Node::load(4);
// Placeholder 2.
Node::load(3);
// Placeholder 3.
Node::loadMultiple(1, 2, 3);

With the Fibers approach, when each of these calls happen, we can check the static cache, and return as normal in those cases.

When we get a cache miss, we can add the entity ID to a class property that holds 'a list of entities that need to be loaded' (i.e. haven't been returned from static cache), and suspend the Fiber.

So during the page request, node 4 gets added to the class property first, the fiber is suspended, we go to the next placeholder, same thing happens for node 3, go to the third placeholder, 1 and 2 are added (3 is already in there).

Then we run out of placeholders to loop through and end up back at the first one. We now have nodes 1, 2, 3, and 4 all in the 'to be loaded' property, then we can run the rest of multiple loading - first check the persistent cache, then the database. When we reach the later placeholders, they check the static cache again, find that the entity was already loaded, and go ahead.

What would have been three separate database loading operations now gets collapsed into one.

What this won't affect is the following code in a single placeholder:

Node::load(1);
Node::load(2);

e.g. when we Fiber::suspend(), we can only get back to Node::load(1) and complete that before going to Node::load(2).

But if this happens in two placeholders:

// Placeholder 1.
Node::load(4);
Node::load(3);
// Placeholder 2.
Node::load(2);
Node::load(1);

In this case, the approach would mean that node 4 and 2 are multiple loaded together, and then node 3 and 1 are multiple loaded together, so two multiple loads instead of four single loads.

And the worst case is that we Fiber::suspend(), there are no other entities of the same type to load, and we end up back where we were - then it's effectively a no-op and will have the overhead of only a couple of cheap function calls.

Pushed a branch. It's going to need 📌 Try to replace path alias preloading with lazy generation Active to be able to show any profiling numbers and for manual testing.

I think kernel test coverage of this should be pretty straightforward. We need a hook_entity_load() test implementation that logs how many times it's called (might already exist), create three entities, then load the three entities inside a fiber each, and see how many times the hook implementation is called.

📌 Create placeholders for more things Active isn't enough for manual testing at least with stock Umami. I think for it to work, we'd need to be rendering entities in placeholders, then it would help with entity references - that currently doesn't happen. Will open another issue. Can still add test coverage here to demonstrate the concept.

new RFC happening https://wiki.php.net/rfc/true_async

Updated the issue summary.

I combined this locally with the branch on 📌 Try to replace path alias preloading with lazy generation Active and together, they're knocking 40 database queries and 17 cache sets off the front page with a cold cache.

1) Drupal\Tests\demo_umami\FunctionalJavascript\OpenTelemetryFrontPagePerformanceTest::testFrontPagePerformance
Failed asserting that two arrays are identical.
--- Expected
+++ Actual
@@ @@
 Array &0 [
-    'QueryCount' => 376,
+    'QueryCount' => 336,
     'CacheGetCount' => 471,
-    'CacheSetCount' => 467,
+    'CacheSetCount' => 440,
     'CacheDeleteCount' => 0,
     'CacheTagLookupQueryCount' => 49,
     'CacheTagInvalidationCount' => 0,

Added a kernel test, which in turn uncovered a bug - pushed a commit for both.

The database query reduction now looks like 263 -> 247 when this issue is combined with the others.

I also found 🐛 Config overrides are loaded for English even when translate_english is false Active while looking at what queries were left, not really related to here except that it's a lot of config entity loads for no reason. 📌 ContentTranslationHooks::viewsDataAlter() could multiple load config entities Active is also over a dozen queries we can potentially consolidate.

Rebased to get a fresh test failure baseline. Removing needs tests tag because these were added.

A couple comments on the MR that look like bugs, not sure if related to the Umami test failure.

Applied those suggestions which look good, but there are still bizarre bugs with the MR. It's easy to reproduce with the combined MR, you just have to clear caches and load the umami front page to see broken rendering.

I went down a whole rabbit hole of the fibers logic, but after exhausting that, I'm back to an issue with the entity load logic again, it is potentially something like the wrong entity getting returned sometimes, which causes Umami to render different blocks to the ones requested, or something.

The good news is that 📌 Try to replace path alias preloading with lazy generation Active appears to be working without the same problems, although it needs 📌 Views entity rendering should use a lazy builder/placeholder Active to be more effective.

It's easy to reproduce with the combined MR, you just have to clear caches and load the umami front page to see broken rendering.

Tested this out locally really quick and I can see that drupalSettings.bigPipePlacheolderIds has 3 unreplaced entries.

{
    "callback=Drupal%5Cblock%5CBlockViewBuilder%3A%3AlazyBuilder&args%5B0%5D=umami_views_block__recipe_collections_block&args%5B1%5D=full&args%5B2%5D&token=LSXAUcXQcqsNxZk01EJ-DceQbM_P3ovAc_wniqiM2X0": true,
    "callback=Drupal%5Cblock%5CBlockViewBuilder%3A%3AlazyBuilder&args%5B0%5D=umami_views_block__promoted_items_block_1&args%5B1%5D=full&args%5B2%5D&token=XKowAdwoi2xYmp7GHvIqQteOfgS9deS_4OJckMkYGHQ": true,
    "callback=Drupal%5Cblock%5CBlockViewBuilder%3A%3AlazyBuilder&args%5B0%5D=umami_banner_home&args%5B1%5D=full&args%5B2%5D&token=j1t-pelbFkxnp6XgWmQ0AG2woBVFHKrU7calg2QLe8M": true
}

There's something very, very weird going on with the bugs here. I pushed a new branch - at https://git.drupalcode.org/project/drupal/-/merge_requests/12469 which has the absolute minimum changes to cause the bug - no entity preloading, just the fiber suspend in the same place. That I guess shows it's not the preloading causing the problem as such.

Update on #59:

1. The entity cache preloading itself isn't the problem, just suspending a fiber from entity load is enough to trigger some very weird placeholder replacement bugs with Umami.

2. After realising that, I wondered if there was a particular entity type causing the problem, and it turns out there is - it's file entities. If I skip the fiber suspend only when loading file entities, I don't get the bug.

3. This means that fiber suspending during file entity loading is sufficient to trigger the bug. That might be something specific to file entities, e.g. they're being rendered via a reference widget so maybe something in there, or it might be that on the umami front page this happens to be the only entity type that triggers the problematic code path a bit further up the call stack.

Still no idea what the actual bug is, but at least have managed to narrow it down from 'every possible situation that loads an entity' which is where I started and led me into several dead ends.

The blocker is in so this is technically unblocked, although it is definitely still stuck for now.

FWIW, since 📌 Pass RenderContext around in the Renderer Active is in, I applied the changes to core/lib/Drupal/Core/Entity/EntityStorageBase.php from MR 12444 to 11.x and still see unreplaced big pipe placeholders on the Umami home page, even with the exclusion of file entities. Of note is that one of the unreplaced placeholders is the recipe collection view block, which does not contain any images.

It could be that I didn't cherry-pick the changes correctly, but I don't think that's the case.