Meetings index in a component optimization

[entantoencuanto]

🎩 What? Why?

This PR adds fragment caching to meetings index of a component performance by fragment caching the _meetings partial render

• The cache key used is defined in a similar way as in cells with a cache_hash method. This method takes into account:
  • The list of meetings
  • The filter params of the request
  • The current page
  • The cache version of the meetings query (cache_key_with_version does not work because it adds a hash related to the sql query that varies with each request because it uses the current date and time)
  • Cache key with version of component and current_user if present
  • The locale is included by default so it's not included there
• The cache expires at 10 minutes
• The PR also adds a touch:true to Follow association with followable. This allows the cache to be expired when changes in meetings followers occur.

Metrics

Data obtained from appsignal data of staging applicaction, with a component with 48 meetings and pagination with 20 items per page. Time in ms.

The main improvement is observed in the times of action_view

Before optimization

	Mean	Sample 1	Sample 2	Sample 3	Sample 4	Sample 5
action_view	1231,4	978	943	1400	1920	916
active_record	182	126	125	138	388	133
action_controller	69,2	59	57	77	96	57
active_support	113	90	55	49	282	89

After optimization

	Mean	Sample 1	Sample 2	Sample 3	Sample 4	Sample 5
action_view	335,6	348	367	272	333	358
active_record	115	112	127	106	106	124
action_controller	118,6	285	105	85	58	60
active_support	33,4	4	16	38	108	1

📌 Related Issues

• Related to Optimize First Contentful Paint #8471

Testing

Verify the page loads as before, and maybe in the logs check the cache hit message.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

• ❓ CONSIDER adding a unit test if your PR resolves an issue.
• ✔️ DO check open PR's to avoid duplicates.
• ✔️ DO keep pull requests small so they can be easily reviewed.
• ✔️ DO build locally before pushing.
• ✔️ DO make sure tests pass.
• ✔️ DO make sure any new changes are documented in docs/.
• ✔️ DO add and modify seeds if necessary.
• ✔️ DO add CHANGELOG upgrade notes if required.
• ✔️ DO add to GraphQL API if there are new public fields.
• ✔️ DO add link to MetaDecidim if it's a new feature.
• ❌AVOID breaking the continuous integration build.
• ❌AVOID making significant changes to the overall architecture.

📷 Screenshots

Please add screenshots of the changes you're proposing

♥️ Thank you!

[ferblape]

@decidim/product Andrés, we have created this PR to improve the server response time in some key pages and improve the First Contentful Paint. It's ready to be reviewed.

[ahukkanen]

I noticed few issues with the cache hash, could you please take a look?

Generally I wouldn't support this kind of changes. I can understand the desire to make the pages to load quicker but these kinds of changes just feel like adding a band aid on top of another band aid.

Another thing is that these kinds of things are very context specific such as:

• Specific to the particular logged in user
• Specific to the selected filters
• Specific to the selected language
• Specific to the selected order
• etc. etc.

This causes millions of potential different combinations in to be added to the cache for each content paint. If each paint accounts for 1kB (for instance, just a number from my head), this cache alone would potentially cause 1GB of accumulated cache size with a million different combinations.

Pushing stuff to the cache is not free, it has a cost. Many configurations use memory-based cache, so we just increased the Decidim RAM need (which is already very high) by 1GB adding this cache.

OK, so you can choose to store the cache on disk. Disk space is basically free right?

That's not completely free either. E.g. in multi-node setups this would cause the cache to accumulate separately on separate machines which itself can cause problems if the same user is served from multiple nodes during different loads. Another issue is that this makes the cache implementation itself run slower as it needs to go through more records and loading millions of records from the disk becomes slow at some point.

This seemingly improves performance as per the metrics if you isolate the tests only for a single user, single node, specific setup, etc. But it may cause issues or worsen the performance under other configurations.

---

@andreslucena @decidim/product Have you agreed on this approach beforehand?

If you feel this is necessary, we can keep on building the band aid pile until we allocate actual resources to re-think the caching strategy & necessity to show some user-specific stuff in the UI which would greatly reduce the amount of cache hash parameters. Or re-think how that user-specific stuff would be loaded separately in the front-end after the cards are loaded with these sections initially empty.

[ahukkanen]

Please add:

• Current locale
• Current order of items

[ferblape]

@ahukkanen you are totally right, and I think some of this user customizations are removed in the new design, where meetings are more compact.

That said I'm happy to discard this PR if you feel to until the new design is in place to avoid adding more logic layers that can be simplified later.

[ahukkanen]

@ferblape If you asked me, I would discard this PR. And exactly for that reason, we hopefully wouldn't need this kind of stuff in the future, i.e. there needs to be a better way to optimize the performance.

But I'm not the one pulling the cords here, so let's wait for response from @decidim/product .

[andreslucena]

I agree with what you've said, let's discard this. Caching with filtering doesn't work well.

@andreslucena https://github.com/orgs/decidim/teams/product Have you agreed on this approach beforehand?

The task that we have pending is just to improve general performance (see #8495). It's tricky because there are some things that probably doesn't make sense to solve as they could be directly related to the UI and the current design, and they will change with the redesign.