Optimize render caching

Small update:

Now that we have VariationCache ✨ Abstract RenderCache into a separate service that is capable of cache redirects in a non-render array-specific way Fixed and made sure that nothing in core still (ab)uses the render cache to store things other than render arrays, it could be easier to remove the render cache. Having said that, I still see value in keeping it. The comments also seem to be in favor of keeping it for some parts, so perhaps we could update the IS to strike out the part that suggests outright removing the render cache?

Having said that, I think it's hard to determine beforehand what we may or may not want to exclude. For instance the IS suggests disabling it for entities and blocks but keeping it for views. But what if I have a block in my footer that is really expensive to calculate and only ever varies by what day it is, I'd rather that block be render cached and every page request from thereon out gets the cached copy than have to recalculate said block on every page request for it to then be cached by DPC.

I really like @Berdir's suggestion in #11 but I'd turn that upside down. What if we do not cache anything in render cache unless we specify that it's expensive to calculate. This is a lot safer when you consider outside alters: If I flag something as CACHE_NOT_WORTH_IT and then someone alters my render array and adds in something expensive, we have a problem. But if my render array is by default considered as not worth caching and then someone alters in something expensive, adding RENDER_CACHE_REQUIRED, then that would make more sense.

Figuring out what we want cacheable or not by default is still possible that way, we'd only need to add RENDER_CACHE_REQUIRED to the things we know are often expensive.

#59 sounds promising, also seems like it would fit quite well with lazy builders/autoplaceholdering where you have to explicitly define something in a way it can be placeholdered.

Patch for Drupal 10.3

I really like @Berdir's suggestion in #11 but I'd turn that upside down. What if we do not cache anything in render cache unless we specify that it's expensive to calculate. This is a lot safer when you consider outside alters: If I flag something as CACHE_NOT_WORTH_IT and then someone alters my render array and adds in something expensive, we have a problem. But if my render array is by default considered as not worth caching and then someone alters in something expensive, adding RENDER_CACHE_REQUIRED, then that would make more sense.

I think render cache is by default still more valuable than not. Entity types can already easily opt out, even per bundle, and we do that for paragraphs for example, where we can safely assume that they are not reused outside of of a single node and by default, if one is invalidated by a save, then all of them are. But nodes very commonly still are absolutely useful to cache. Rendering entities is surprisingly expensive with view display config, many formatters, often multiple child entities like medias, paragraphs, layout builder and so on. It's quite easy to end up with something like a view in a block on all pages that is going to be frequently invalidate your complete dynamic page cache and then you have to rebuild all your node pages again which are most likely still cached.

Media entities I'm on the fence, I can see those being not really worth it and enough to inherit in the parent, I've also had multiple cases with per parent access and customization, where you have to consider the parent anyway. But we don't need any new API's for that, it's just a flag we could set on the entity type. But even for media, don't forget about things the media library, where we render a lot of media entities.

Also for blocks, IMHO the majority are still worth to cache, menu blocks can be very expensive with many links for example, it's IMHO also much easier for BC to add an opt out rather than an opt-in.

We now have rendering time built-into the render debug output, it would be fairly easy to automatically highlight all elements that take less than a configurable/estimated/measured render cache get. We could even build that into the performance tests?

for example, the default powered by drupal block for me reports as "RENDERING TIME: 0.003705978". The default frontpage node from the paragraphs_demo module with 7 paragraphs is "RENDERING TIME: 0.229816914". You can search for RENDERING TIME: 0.00 to find things that are propably not worth to cache:

on olivero, for me, additionaaly:
site branding: RENDERING TIME: 0.005095959 (interesting example, because if you do some more complex things with the logo, e.g. we often have per-language logos, or inline SVG logos, that could easily get slower)
search form: RENDERING TIME: 0.004688025
account menu is RENDERING TIME: 0.009571075
messages block: RENDERING TIME: 0.002686024
syndicate block: RENDERING TIME: 0.004869223

so, doesn't get much faster than ~0.03 on my system. didn't compare yet how fast a render cache get is.

The render time avenue is worth exploring, but how would that work? The debug info is only available after rendering, so we can choose not to set it when we had to render it, but we'd still be trying to get it from the cache every time it is to be built. Ideally, we don't go to the cache at all, which would be possible using a tag like CACHE_NOT_WORTH_IT or RENDER_CACHE_REQUIRED.

So either we depend on render time and eat unnecessary cache lookups, but have the benefit of a "smart' render cache. Or we go with the aforementioned special flags/tags, but require manual intervention.

If we do choose to go with a smart system based on render time, we'd need a way for people to opt out of it. Let's say you have a block that renders fast but pings an external API, you might wanna cache that block regardless for a given while as to not run into a rate limit. (I know there's ways around that by using a service with an internal cache, but just giving a feasible example)

A downside of adding the render time checks to StandardPerformanceTest is that the time is machine-dependent, so we'd be introducing another test that can randomly fail on GitLab CI.

Either way, not a bad idea. Definitely worth exploring.

A downside of adding the render time checks to StandardPerformanceTest is that the time is machine-dependent, so we'd be introducing another test that can randomly fail on GitLab CI.

Yeah I've been specifically avoiding any assertions based on times, but we could add something to the OpenTelemetry spans so that render elements show up and/or to webprofiler module.