Rendering: Don't use element cache level on snapshot cache level properties #21006
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Prerequisites
Description
This is somewhat a continuation of the work done specifically for the RTE in #20880
The original hybrid cache implementation removed snapshot caching as a concept. As part of this, the
Snapshotcache level for published property values was obsoleted. In this obsoletion, theSnapshotcache level was treated as theElementcache level, which is inherently wrong; theElementlevel is the most aggressive level of caching, where theSnapshotlevel was the least aggressive (not counting theNonelevel).#20681 fixed the
Elementlevel, soElementlevel properties are actually cached as they should be. But this fix then affects allSnapshotcached properties, because they are now cached atElementlevel. While this is certainly more performant than the alternative (which isNone), it is unfortunately also wrong. We need to apply theNonecache level toSnapshotcached properties, for them to render as dynamically as they used to before the hybrid cache.This PR fixes it. Incidentally, this also aligns with the published element property implementation.
Immediate performance considerations
At first glance, the
Nonecache level looks like quite the performance degradation. In effect, though, it really isn't:Snapshotbefore the hybrid cache meant a re-rendering for each request; the cache lasted for the duration of the snapshot, which was effectively per request.Snapshotafter the hybrid cache should have (wrongfully) been behaving likeElement, but due to the above-mentioned error, allElementlevel caching was effectively discarded.All in all, this change only has a negative impact on
Snapshotlevel properties being requested (accessed) multiple times during a page rendering.Future performance considerations
We have a backlog task to consider if we can do something about
Snapshotlevel properties, to avoid them being re-rendered if requested multiple times during a page rendering. This might mean introducing a replacement cache level, which would be explicitly targeting a request scoped cache lifetime.Testing this PR
Snapshotlevel property value converter and verify thatConvertSourceToIntermediateis executed for each request.