Problem/Motivation
Follow-up for #2429617: Make D8 2x as fast: Dynamic Page Cache: context-dependent page caching (for *all* users!).
See #219 & #276.4 + replies.
Proposed resolution
- Don't vary by
route, but byurl, this allows us to avoid routing altogether. - Somehow make sure we don't need to run the authentication subscriber anymore, so that we don't need to boot Drupal in order to know the values for the
user.*cache contexts. (See #7 for a potential idea.)
Remaining tasks
TBD.
User interface changes
None.
API changes
None.
Data model changes
None.
Comments
Comment #1
fabianx commentedI am all for making things work with url instead of [route] and rely on the cacheable metadata added during access handling on the request object and we can cache the user related cache contexts per 'user session'.
That would then be the same strategy as my ESI v2 battleplan takes, just inside of core instead of in Varnish.
One caveat to all of that is:
- I had problems rendering placeholders without http kernel having run first, because something broke.
So likely this pre-routing middleware is only useful if there are no placeholders that are not cached / cacheable.
Comment #2
wim leersComment #3
almaudoh commentedComment #4
effulgentsia commentedRaising priority because I suspect the Performance gain from this will be quite large.
Comment #5
effulgentsia commentedMoving to 8.1 though.
If this is still the case, then perhaps this issue could be done in two steps? First step to move it to a much higher priority REQUEST listener to at least factor out dependencies on routing and route-based authentication. Then a second step to move it to a middleware to factor out dependencies on http kernel.
Comment #6
fabianx commentedYes, agree with #5 to do this in two steps.
Comment #7
wim leersAlso, as noted in http://wimleers.com/article/drupal-8-dynamic-page-cache#authcache — Authcache uses an interesting trick to sidestep the vast majority of the cost. I'm not sure how safe it is nor if it works with every authentication mechanism. I'm certain we can't use it for Dynamic Page Cache directly, because it relies on every response of the site varying by the same cache contexts (which is grossly inefficient).
But I think we should consider learning from it.
user.*cache contexts in a cache entry: roles, permissions hash, UID, etc.(IS updated.)
Comment #8
fabianx commented#7: Yes, that is the same authentication step per session cookie that I am doing in ESI.
In essence caching the cache contexts.
Comment #9
wim leersYep, and important to note that we cache that per session.
Comment #14
wim leersI just noticed this is in the wrong component. Probably the right one didn't exist yet at the time.
Comment #26
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #27
catchPer route significantly improves things for 404s because the page rendering itself all happens on the same route (e.g. the custom 404 page) (see #3516169: Adding url.path to breadcrumb cache context results in every 404 page getting a different dynamic page cache entry for how that improvement can then be undone) so I'm not sure this would necessarily be an improvement across the lifecycle of a site.
Comment #28
berdirIf anything, it should only move to before routing. We do a lot of post processing on dynamic page hits, such as building uncacheable or cache miss elements like blocks. This can not and will never work in a middleware without loaded modules and so on.
Whether that's feasible at all or not I don't know, didn't think through the 404 case at all. Also BC and implications on existing code that needs to run before/after seems very complicated. I'd say won't fix.