Updated: Comment #N
Problem/Motivation
Some cache tags are currently deleted over and over again, especially during test runs.
Additionally, we seem to delete cache tags for each backend, even if they only use the same. There's an issue to address this with a separate service.
See the attached screenshot, 300 calls to Cache::deleteTags() lead to 3000 DatabaseBackend::deleteTags(), each of those executing a merge query.
The biggest offenders are menu name cache tags, cached config storage, entity field definitions.
Proposed resolution
Added a static (drupal_static() just like the existing tags cache for now, real static might mess with tests). Can become a normal property of the cache tag service once we have that.
This cuts down the number of executed merge queries to 25, instead of 3000. Should be about the same for every single web test. Saves almost 2.5 seconds for me, on a fairly well optimized database.
See also the second screenshot how this changes the time per call, you can see that drupal_get_schema() moves to the top, probably because it also sets the cache again and therefore has to delete again. We might be able to optimize that by not adding the tag to the main schema cache item? But that's only the biggest part of the much much smaller total time now.
Remaining tasks
User interface changes
API changes
Comment | File | Size | Author |
---|---|---|---|
#11 | cache-tag-deletion-2194273-10.patch | 2.21 KB | Berdir |
#8 | cache-tag-deletion-2194273-8.patch | 2.9 KB | Berdir |
#3 | great_patch.jpg | 128.66 KB | amateescu |
cache_tag_delete_patch.png | 135.88 KB | Berdir | |
cache_tag_delete_now.png | 148.02 KB | Berdir |
Comments
Comment #1
BerdirComment #2
andypostYay, tested locally and it rally speeds-up!
d8t --class '\Drupal\field_ui\Tests\FieldUIRouteTest'
changed from 32-31 sec to 25!Needs to finish
Comment #3
amateescu CreditAttribution: amateescu commentedComment #4
BerdirFixing title.
Comment #6
BerdirNeed to look into the fails, but this took 1h30 on testbot #813, other recent core tests there were around 2h20. So the effect there seems to be considerably bigger, I guess that's related to concurrency and that mysql has to handle 8 parallel installations/cache rebuilds.
Comment #7
sunI think this is (at least) major. — Also, I'm super-happy that we're not implanting a test-specific workaround again, but actually attacking the root cause! :)
Thanks @Berdir!
Comment #8
BerdirAw, stupid web tests, were messing with the logic again :)
Fixed this exact problem already once, needs a reset in resetVariables(), just like the cache tags cache too.
Also includes the fix #2152825: rename FieldItemBase::getFieldSetting[s]() to getSetting[s]() as HEAD is currently broken, hoping that it will start to run before HEAD comes back :)
Comment #9
BerdirRelated, while debugging this I noticed that configFindByPrefix is deleted *a lot*. I just check with an actual installation that we have that has been running for a while, and the counter of that is now at ~45k. So, it might be very worth it to replace it with a CacheCollector, less cache tag deletion and cache writes, not sure how complicated it would be to implement it.
Comment #11
BerdirOk, HEAD was fixed before the test was started as it was already postponed, here's an updated one without the fix and also added the comment that I wanted to write above.
Comment #12
sunLooks good to me, thanks!
Do we already have an issue to replace those weird preexisting
drupal_static()
s with class properties or something?Comment #13
BerdirYes, I guess that will happen in #918538: Decouple cache tags from cache bins, which makes cache tags a separate service.
Comment #14
alexpottCommitted af7652d and pushed to 8.x. Thanks!
Comment #15
Wim LeersHOLY SHIT this is an awesome patch!
Berdir++++++++++++++++++++++++++++++++++++++++