Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
The JSON:API module is creating a lot of URLs for entities and their fields for different actions like GET, POST, PATCH, PUT. For example the JSON:API module is generated 219 routes after fresh installation using "standard" profile and the total count of non admin routes is 319.
Our project has difficult structure and we are using a lot of different entities and some content types have about 60 fields. As a result the JSON:API module is generating 3225 routes. All this routes are non admin routes and cached by the Drupal\Core\Routing\RoutePreloader::onFinishedRoutes() first and by the Drupal\Core\Routing\RouteProvider::preLoadRoutes() on kernel.request event (routes are cached as serialized objects and the size of cached item is about 50mb and about 80% of routes are generated by the JSON:API). This routes are not used during page render and there are no reasons to store them in the cache. As a result the memory dedicated for the cache is used in not effective way because it contains about 80% of unused data.
Proposed resolution
Previous resolution in #3
1. Provide an alter to allow manage preloadable routes
2. Implement alter to exclude the URLs generated by the JSON:API module from the preload cache
3. Make it configurable.
Current resolution
- Change logic in \Drupal\Core\Routing\RoutePreloader::onAlterRoutes() to only preload routes that are for HTML and support GET
Remaining tasks
User interface changes
API changes
Data model changes
Release notes snippet
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | 3120301-17.patch | 3.89 KB | alexpott |
| #17 | 14-17-interdiff.txt | 1.79 KB | alexpott |
| #14 | 3120301-13.patch | 3.8 KB | alexpott |
| #14 | 10-13-interdiff.txt | 4.9 KB | alexpott |
| #10 | 3120301-10.patch | 6.45 KB | alexpott |
Comments
Comment #2
zestagio CreditAttribution: zestagio at Smile commentedComment #3
zestagio CreditAttribution: zestagio at Smile commentedComment #4
zestagio CreditAttribution: zestagio at Smile commentedComment #5
zestagio CreditAttribution: zestagio at Smile commentedComment #6
zestagio CreditAttribution: zestagio at Smile commentedComment #7
charginghawk CreditAttribution: charginghawk as a volunteer commentedWow, perfect timing, we just ran into this issue, and it was adding 15000+ routes, which could mean a ~232 MB cache object that's not just expensive to generate, but could also crowd out valuable cache items and cause poor performance elsewhere. Thanks for the patch!
Comment #9
alexpottHere's a different approach which makes the RoutePreloader obey it's own documentation. It says:
So let's only pre load HTML routes. Because they are the ones that can appear as links... and then we can also use the _admin_route option that's added by \Drupal\system\EventSubscriber\AdminRouteSubscriber() automatically and also is in some routing definitions. Also we can check if GET is in the allowed methods because linking to a post route in a menu doesn't really work.
I think this is a bug.
Comment #10
alexpottHere's a test.
Comment #12
alexpottDone some additional testing with #10...
Standard with patch preloaded route count: 76
Standard preloaded route count: 98
Routes no longer preloaded on Standard:
Routes now preloaded that weren't before:
Therefore I think preloading block.admin_demo is wrong and potentially it is way more useful to pre node.add as many users can have that route so I'm going to revert the _admin_route logic checking and leave the GET and HTML format check in as that solves the most pressing problem at hand - namely all the JSON API routes ending up in this list.
Routes no longer preloaded on Standard with JSON API:
Comment #13
alexpottNow with the patch attached... on standard and JSON API installed we have 93 routes in state
No new routes have stored.
And the routes no longer stored are:
Comment #14
alexpottForgot to add the patch to #13 lol.
Comment #15
Wim Leers🤯
This seems very reasonable 👍😊
Übernit: s/Non HTML/Non-HTML/
Nit: the format that the JSON:API module uses is not called
json_apibutapi_json.(Yes, that is confusing.)
That being said, it doesn't matter for the test coverage here: the point is that the
htmlformat is absent.This could use an
@see \Drupal\Core\Routing\RequestFormatRouteFilter::getAvailableFormats()because it technically reimplements (duplicates) that logic.Comment #16
Wim LeersForgot some critical context to justify RTBC'ing this: this cannot be a performance regression for JSON:API requests because
\Drupal\Core\Routing\RoutePreloader::onRequest()does this:… if anything, this means JSON:API could do its own route preloading strategy!
Comment #17
alexpottThanks for the review @Wim Leers - I've addressed the nits. Since this is all comment and test changes leaving at rtbc.
Comment #20
catchVery nice find, good to see this RTBC.
Committed a6ef32e and pushed to 9.2.x. Thanks!
Also cherry-picked to 9.1.x
Comment #21
alexpottComment #22
alexpottI think given the amount memory being wasted here if you're using json api we should consider this a major bug and backport to 8.9.x.
Comment #24
catchYes agreed and the high memory usage definitely makes this major, could even lead to OOM errors. Cherry-picked to 8.9.x.
Comment #25
Wim Leers🥳