As identified in #2241377: [meta] Profile/rationalise cache tags, the extension:views cache tag is invalidated a lot, and it is not really clear why it exists. Identify similar cache tags and other code that can be improved in/for views data/plugin caches.
Changes done in this issue:
- Remove the extension:views cache tag
- Remove usage of the config:core.extension cache tag in views plugin managers, they rely on the plugin cache clearer now.
- Replace the deleteAll() calls and invalidations of that cache tag with an explicit call to invalidate the views data cache. We currently can't think of any example why a field config change would require changes in cached views, so that is removed. If a specific field requires that, it can add a cache tag for the relevant field.
- Remove the config:core.extension cache tag invalidation in system_list_reset(), that is called repeatedly during dfac() calls and is not needed anymore.
- Removal of manual views data cache clears in a lot of tests, that no longer need it due to the updated hooks.
- Some test fixes that we don't really know why they were passing before.
| Issue category | Bug because too many invalidations is a bug. |
|---|---|
| Issue priority | Normal, because the impact was not measured. |
| Prioritized changes | The main goal of this issue is performance as cache invalidations need unnecessary SQL queries and even might remove too many cache items. |
| Disruption | Not disruptive as its unlikely someone would have used those cache tags and need them invalidated. |
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 2443485-11.patch | 12.19 KB | berdir |
| #8 | 2443485-8-interdiff.txt | 662 bytes | berdir |
| #8 | 2443485-8.patch | 8.64 KB | berdir |
| #5 | 2443485-5.patch | 12.22 KB | dawehner |
| #5 | interdiff.txt | 3.55 KB | dawehner |
Comments
Comment #1
berdirThis is likely too bold, but let's see what is failing...
Comment #3
berdirOk, so the field_config hooks are important, but I think we can make them nicer. I'm not sure about the render part, but instead of clearing that directly, we should maybe add the views data tag to views caches?
Comment #5
dawehnerI'm speechless.
Comment #6
berdirThe patch removes the deleteAll() from render bin when fields are changed.
We don't know why it is there, but we said that if someone depends on field settings, then it should just add the relevant cache tags (Do we need a method on the view so that something like a filter plugin can do that?)
Comment #7
wim leersGreat clean-up so far!
Comment #8
berdirUpdated the issue summary.
Also removed the field_config_list cache tag again, that was just a test.
I don't think there's something left to do here.
drush cr before this shows one invalidation of config:core.extension and 19 invalidations of config:core.extension and extension:views in my more or less clean installation.
They're all gone with this patch.
Comment #10
dawehner@berdir
Did you removed the changes in #5 on purpose?
Comment #11
berdirNo, I did it because I'm stupid :)
Comment #12
fabianx commentedThats ready for RTBC now :), but needs a beta eval.
Comment #13
fabianx commentedRe-classifying as a bug, because too many invalidation is a bug.
Comment #14
fabianx commentedAdded beta evaluation.
Comment #15
fabianx commentedComment #17
catchCommitted/pushed to 8.0.x, thanks!
Comment #18
catchCommitted/pushed to 8.0.x, thanks!