Skip to content

Conversation

@kjac
Copy link
Contributor

@kjac kjac commented Nov 30, 2025

Prerequisites

  • I have added steps to test this contribution in the description below

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 Snapshot cache level for published property values was obsoleted. In this obsoletion, the Snapshot cache level was treated as the Element cache level, which is inherently wrong; the Element level is the most aggressive level of caching, where the Snapshot level was the least aggressive (not counting the None level).

#20681 fixed the Element level, so Element level properties are actually cached as they should be. But this fix then affects all Snapshot cached properties, because they are now cached at Element level. While this is certainly more performant than the alternative (which is None), it is unfortunately also wrong. We need to apply the None cache level to Snapshot cached 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 None cache level looks like quite the performance degradation. In effect, though, it really isn't:

  • Snapshot before the hybrid cache meant a re-rendering for each request; the cache lasted for the duration of the snapshot, which was effectively per request.
  • Snapshot after the hybrid cache should have (wrongfully) been behaving like Element, but due to the above-mentioned error, all Element level caching was effectively discarded.

All in all, this change only has a negative impact on Snapshot level 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 Snapshot level 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

  1. Attach a debugger to a Snapshot level property value converter and verify that ConvertSourceToIntermediate is executed for each request.
  2. Verify that property expansion and field limiting works for the Delivery API. This must be verified with multiple, different expansion instructions for the same (expandable) property, to ensure that the PR works as intended.

Copy link
Member

@Zeegaan Zeegaan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works like a charm 💪

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants