Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Problem/Motivation
This was added when we were using the file storage for the active store, so it was worth it.
However, the cache tag is invalidated a lot, so we probably spend more time on recalculating those lists and saving it back into the cache than we benefit, especially with the database backend, that can do a findByPrefix() in a pretty fast way.
Proposed resolution
Remove the persistent cache and the invalidations, keep the static cache, so that repeated requests for the same list are cached (useful for e.g. many repeated entity queries on the same entity type).
Remaining tasks
User interface changes
API changes
Comments
Comment #1
BerdirComment #3
BerdirComment #4
BerdirGrml.
Comment #5
BerdirHm. I just check the test execution time, and while those used to vary a lot, they actually don't do that anymore, and this is one minute slower:
Patch:
16 min 52 sec
4 different other tests on the same server:
15 min 54 sec
15 min 51 sec
15 min 50 sec
15 min 53 sec
So it would probably be good to figure out why this happens.
Comment #8
olli CreditAttribution: olli commentedHow about keeping the static cache (findByPrefixCache) but drop persistent caching?
Comment #9
BerdirYeah maybe, here's a patch for that, no interdiff I just started fresh.
Comment #11
BerdirWill fix the unit test asap.
Performance looks better with this, test run was 16:40, while other runs were 16:57, 17:00 and similar, so slightly faster.
Comment #12
BerdirRemoving the tests that we no longer need.
Comment #13
Wim LeersSo basically, this issue is: .
… because we do exactly that, except for removing
StorageCacheInterface::FIND_BY_PREFIX_CACHE_TAGitself. Why not?If the performance numbers in #11 are correct, then +1. :)
Comment #14
BerdirRight, removed that.
The caching isn't *completely* useless, but we're caching a query on an indexed column with a cache get. And it is a cache that is expensive to maintain and often invalidated.
It was a lot more useful when it was caching a glob() on a file system that might have hundreds of files in a single folder.
Comment #15
Wim Leers+1, especially the latter.
Indeed.
Question: does this make it impossible for somebody to keep config in the file system and still apply similar caching?
Comment #16
BerdirIt doesn't make it impossible, you can still write your own implementation and replace it, the new patch keeps the interface so that it can be reset, but I think that is mostly needed for tests anyway.
Comment #17
Wim LeersThanks, that addresses all concerns.
Comment #18
alexpottThis issue is a normal task that will improve performance and the disruption it introduces is zero. Per https://www.drupal.org/core/beta-changes, this is a good change to complete during the Drupal 8 beta phase. Committed 566bc8b and pushed to 8.0.x. Thanks!