Remove automatic preloading of all "public" routes, cache routes in fast chained bin

When disabling RoutePreloader and all render cache bins, I get 77 calls in total to preloadRoutes() for an anonymous user on my frontpage only 6 of them actually fetch additional routes (including the two from redirect), many of them already deliberately preload their specific routes. and mostly happening within cacheable bits. LocalTask does all the node local tasks at once, a few menus, some weird bits like .

Worth noting: Our site building approach almost exclusively uses nodes with paragraphs, we basically have no other linked routes such as views or something.

In total, less than 100kb memory reported through that method by Blackfire.

With render caches (but page and dynamic page cache disabled), I'm getting exactly 2 as anon user, those from redirect. As an admin, on the user edit form with navigation module I get 800 calls to preloadRoutes(), 13 cache lookups. On the frontpage, it's 11 with render caches on, 17 with them off. 500-600kb memory usage for admin.

(I'm optimistic that a lot of these will actually go away if we can fully cache the navigation)

Is there a third option? Can we just remove the RoutePreloader, do we really need this?

Somehow missed #3503843: Remove automatic preloading of all "public" routes, cache routes in fast chained bin. Will keep this separate for now to do some more testing as the approach is quite different.

Doing some tests with a different implementation.

Basically, I disabled the route preloader for now completely. On a production system that's been running for a bit, I see around 110 variations of route preloader caches with the current approach. I assume that without preloading a ton of routes on every request, there will be a lot more of those variations. Right now, the cache is always for a bunch of routes, keyed by a hash of those.

I'm changing that to using getMultiple() so we don't get many overlapping variations of different sets of routes. I'm not sure what the performance difference between a multi-cache load and a single one is. It's multiple rows, multiple cache items to expand and check. On its own, possibly slower, but I plan to combine it with the second idea:

As mentioned, I'm playing with the idea of using a ChainedFast backend for either some (either hardcoded on not /admin or by still using the Preloader, but only to feed the list of "frontend" routes) or all. For the mix, it would split the routes, and load them from the fast or regular bin. At this point, getMultiple() a bunch of routes should be a lot faster than the regular query.

Right now we cache the serialized strings because we know we don't need all of them. Without the big preload, that's no longer true and we will use those routes quite reliable. So we could consider to stop doing that and benefit from improvements such as #3014514: Make igbinary the default serializer if available, it saves 50% time on unserialize and memory footprint by caching the route objects and letting the cache backend/serializer optimize it. would also

For size comparison as to how big the router table is, on our install profile:

select count(*), sum(length(route)) from router;
+----------+--------------------+
| count(*) | sum(length(route)) |
+----------+--------------------+
|      876 |            1244156 |
+----------+--------------------+

select count(*), sum(length(route)) from router where path not like '/admin%';
+----------+--------------------+
| count(*) | sum(length(route)) |
+----------+--------------------+
|      175 |             231266 |
+----------+--------------------+

MariaDB [db]> select count(*), sum(length(data)) from config;
+----------+-------------------+
| count(*) | sum(length(data)) |
+----------+-------------------+
|     1078 |           2124343 |
+----------+-------------------+

MariaDB [db]> select count(*), sum(length(data)) from cache_discovery;
+----------+-------------------+
| count(*) | sum(length(data)) |
+----------+-------------------+
|       40 |           1454662 |
+----------+-------------------+

Full router table is around 1.2MB, similar to discovery cache and around half the size of the config table. Non-admin routes is only 200kb.

Sure, router could get a lot bigger, but every view that would be added there will also add a much bigger chunk to config.

Unsure how fancy we want to get with this, happy for some opinions.

I think I tested with disabled render caches, my concerns around the caching atm are how the impact is cold caches, agreed that warm caches should have very few lookups.

A variant of the caching idea above is that we could also only use the fast cache and just don't cache others. It should only happen in the backend and for admins, and it's essentially a 1:1 of a query lookup and cache lookup.

I pushed a proof of concept for that, still using the route preloader and a new method, didn't add to interface yet, route preloader is now a misleading name.

RouteProvider already implements events too. I wonder if we should just inline that cacheable routes stuff and make it an implementation detail, unsure.

Testing this a bit on our project, this removes all 5 or so preload cache lookups against the data bin without doing any queries on a dynamic page cache hit (2 from redirect, the other from toolbar).

On a dynamic page cache miss but render cache hits, I'm getting no query lookups either.

On pages with no/partial render cache, I'm getting a bunch of stuff from toolbar, not sure why that seems to be cached with different contexts like that, but different issue.

On admin/config, it results in around 26 queries against the router table, many due to the children access check stuff. But according to blackfire, that page executes around 2000 queries (1500 key value lookups), so I don't think that hurts too much. On other admin pages such as admin/content, local tasks and help and so on are getting cached and there are 2 or so non-cached router lookups.

One downside of this is that using bootstrap for this means that we it takes longer for the bootstrap cache to stabilize, every time a new route is discovered, a new bootstrap cache entry is written, last written updated and then the fast backend needs to be updated again. But I don't think it's worse than reading a new config or plugin discovery for a less frequently used plugin type or something like that.

Setting to needs review. there are test fails, but I'm looking for feedback on how to handle the RoutePreloader, if we should add a new interface on RouteProvider or just inline the whole thing. Depends on whether or not we want others to add more cacheable routes I suppose.

One thing is that we currently exclude for example json api routes, but we possibly might want to reconsider and maybe store routes per format so that we can inject other routes for the same format. not sure. This is where a json api performance test would come in handy, because I expect we'd see some regressions there as the routes aren't cached anymore.

I didn't verify, that's why a test would be useful, but I think so? JSON:API contains lots of relationships and references to other pages, and I think they are created as routes?

Did a very basic test and I counted 10 or so router queries on a jsonapi route for a single article, they are now all uncached, on HEAD they are single data cache lookups.

Merging in the json api performance test, as expected, getting 7 or so extra uncached queries atm.

Testing my idea of storing routes per format, instead of only html. on umami, that results in this route count after enabling rest and jsonapi:

so, 140 HTML, 225 for jsonapi. And that's not a huge amount of bundles and field. jsonapi creates a 2 routes for every bundle plus 2 for every reference field/relationship. Seems like that should be possible to do using parameters, but that's a different topic. I didn't enable any resources for rest.

I'm a bit surprised that I didn't see any ajax/js formats, I guess those routes are using html, so I also assume that currently they are not preloaded as the request format then afaik doesn't use HTML. Maybe we want to unify them to HTML? So many scenarios where performance test could be interesting (views ajax, form ajax?).

Anyway, implemented that, and expected the number of queries to go down on the second cached request then. Except they didn't. The reason is that jsonapi is faking support for the request format. See \Drupal\jsonapi\Routing\EarlyFormatSetter. But with my request event priority change in this issue (to fix route caching for lookups coming early from redirect module), it doesn't yet see that. Reverting the weight change for now gives me the expected result. All the data cache lookups now shift to bootstrap lookups. Whether that's really worth I'm not quite sure.

Changing the subscriber to be after routing/dynamic page cache exposes the problem again for things that are not part of dynamic page cache.

This already happens on HEAD, but there it still hits the data cache. OpenTelemetryAuthenticatedPerformanceTest exposes this perfectly. Before the last change, I was able to move 2 cache lookups from data to bootstrap because it has the cacheaable list there (This is the isFrontPage() check in LanguageBlock, so very similar to redirect module)

I think this is doing too much at once, but I'm not quite sure yet how to cut it yet. Will try to work on an updated issue summary when I have more time, but essentially, I guess we should keep the data cache for non-optimized cases, then we should mostly be able to keep the status quo. And we can either optimize regular requests and ignore json:api for now, or optimize that. And considering that we now have a lot more blocks being rendered outside of dynamic page cache, I think it makes more sense to focus on that. A separate issue/request subscriber could still add jsonapi routes on jsonapi requests, bonus points for figuring out how to not add the regular ones.

Remaining is also BC (added a new method to an interface for now for the unit test) and naming (RoutePreloader no longer preloads routes) things to figure out.

This is primarily about avoiding excessive memory usage, PHP dumper would have exactly that problem and worse, as it would need load all routes into memory on every request.

Rebased.

Rebased with performance tests conflict.

> this should be a lot more efficient overall.

It's worth pointing out that this might not be obvious from the performance test, I explained why those numbers (seem to) increase in the updated issue summary.

Minor performance test conflict, verified the updates locally.

Replied there. Not sure about merging/deprecating the preload interface/method completely. But looking again, I thought the local task/menu tree services did call that method but they don't. They call getRouteByNames() and ignore the return value. So I guess why not, we can deprecate and make it call getRouteByNames().

I'm not sure we can drop the array_intersect_key() though? We still need to get all the routes we requested, and preLoadRoutes() only really deals with the possible subset of those that aren't loaded yet?

I thought a bit more about the deprecation of \Drupal\Core\Routing\PreloadableRouteProviderInterface::preLoadRoutes() and I'd suggest we push the discussion on deprecating or making other changes to how it works to a follow-up issue. I had the idea that we could change preload to work more like \Drupal\Core\Cache\CacheTagsChecksumPreloadInterface::registerCacheTagsForPreload(), so that multiple callers can register their routes and then we load them in one go.

On the other hand, this might be overkill, since on all but the first few requests, these routes should almost exclusively hit the APCu cache. I did some quick exploration on that and didn't get any significant performance test improvements out of it (there was significant reshuffling of executed queries on cold caches, but at least one test even had a slight increase in number of queries)

Still, this is already pretty complex and deprecating that requires some changes to other components such as \Drupal\Core\Menu\MenuLinkTree::load(). I can create a follow-up issue if that's OK.

I did the property deprecations and a post update, and created a CR to explain the behavior change and also mention that sites might want to review their APCu configuration. Depending on what happens with other issues such as the views data and the ttl expiration one, this might be worth putting into the 11.4 release notes.

> Feels like this ought to theoretically work for menus so we load all the routes from all the menus instead of menu by menu. Maybe we should open a follow-up to explore that more, and if we get stuck, then repurpose for deprecating it?

Yes, that is the idea, I did implement that locally, but it didn't really have a big impact. I opened #3589340: Re-evaluate the concept of preloding routes.

That said, routes are fast to load, even when not cached and their size is quite finite and soon warm. in most cases, large menus only use few different routes. I think it would be more beneficial to implement the same logic for entities and aliases: #3559353: Find a way to bulk load entities and aliases in menus. Right now if you have 3 menus with 20 links to nods each, then what happens is with the fiber stuff is that you load them in 20 groups of 3. But they aren't typically of equal size.

Rebased, StandardPerformanceTest conflicts due to the removed content types.

Another performance test rebase.

There is always a performance test merge conflict.

Yay!

Cherry-picked the commit on 11.x, only conflicts where on the performance test and the removed RoutePreloaderTest.

Good find, I previous had the second bin like that in an earlier MR, but we lost it when switching to only using a fast chained bin for route loading.