[1.x][2.0.x] Better caching for cookie performance popup

Created on 22 November 2018, over 6 years ago
Updated 24 February 2025, about 1 month ago

Based on profiling, I saw that the popup rendering is quite expensive because it has a lot of different templates, all those messages and so on that all need to be rendered and prepared in case they will be used.

However, once we defined whether or not popup should be shown, I think the output of all that processing is then the same for everyone?

We can't use render cache, but we might be able to just put everything into a single regular cache entry and fetch from that instead of having to build it over and over again on every page?

✨ Feature request
Status

Fixed

Version

1.0

Component

Code

Created by

πŸ‡¨πŸ‡­Switzerland berdir Switzerland

Live updates comments and jobs are added and updated live.
  • Needs reroll

    The patch will have to be re-rolled with new suggestions/changes described in the comments in the issue.

Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡ΊπŸ‡ΈUnited States pianomansam

    We can't use render cache, but we might be able to just put everything into a single regular cache entry and fetch from that instead of having to build it over and over again on every page?

    Why can't we use render cache? The three existing parts of the $cid (language ID, domain ID, theme name) are already covered by the render cache. In fact, having to add info from another module, the domain module, is a code smell that perhaps this isn't the best approach. The domain module's documentation suggests users add the url.site cache context so that the cache can vary based on the domain. So the EU Cookie Compliance module shouldn't need to worry about the domain. However, because it micromanages this part of the renderable content, it needs to account for all the ways it may vary.

    In working on the πŸ› DNT (Do Not Track) header detection not working when caching enabled Active bug, I traced the issue to the custom cache entry's $cid not varying based on the DNT header. If a custom cache entry was not deployed, that bug would easily be fixed with one line of code adding the DNT header to the cache context. Instead, we need yet another variance of the $cid.

    So to summarize, why can't we let the render cache do its job? Micromanaging a separate cache entry causes additional complexity, makes it easier to cause bugs (DNT header issue), and causes this module to care about unrelated modules (domain module).

  • πŸ‡³πŸ‡΄Norway svenryen

    Reopening this issue after comment from @pianomansam.

Production build 0.71.5 2024