In #2482295: Rebuilding field map with many bundles/fields is very slow, we noticed that we're very likely doing too much cache clearing in post/pre save/delete.
The render cache clear in FieldConfigBase should no longer be needed, we should cover that through view display cache tags already.
We could also look into moving the manual clearCachedFieldDefinitions() methods into the new EntityManager::onFieldDefinition*() methods.
And optimize them to no longer require to rebuild fieldMap and possibly even change it to write-through to the field map cache.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | clean-up-cache-clear-field-config-2487287-3.patch | 6.79 KB | berdir |
| #3 | clean-up-cache-clear-field-config-2487287-3-interdiff.txt | 2.95 KB | berdir |
| #1 | clean-up-cache-clear-field-config-2487287-1.patch | 6.34 KB | berdir |
Comments
Comment #1
berdirNice, looks like the test fails from over there are easier to fix than I thought, I think I just forgot to explicitly unset the fieldMapByType property, as that needs to be updated.
Comment #3
berdirThis should work better ;)
Comment #4
yched commented@Berdir : forgot the patch ?
Comment #5
berdirYes, thanks.
Keep in mind that this is still work in progress, not sure how much of this we want to do, just experimenting...
Comment #7
wim leersFrom the IS:
Well, that's only half the truth. The truth is that
FieldConfigBase::postSave()was actually the one that invalidated the view display cache tags.The failures in
BlockContentCacheTagsTesthappen during the phase, which with this patch now only invalidates these cache tags:As you can see, no view display cache tags.
There are two ways to work around this:
FieldConfigBase::postSave()Comment #8
wim leersComment #9
Lucasljj commentedadded related issue
Comment #23
smustgrave commentedThank you for creating this issue to improve Drupal.
We are working to decide if this task is still relevant to a currently supported version of Drupal. There hasn't been any discussion here for over 8 years which suggests that this has either been implemented or is no longer relevant. Your thoughts on this will allow a decision to be made.
Since we need more information to move forward with this issue, the status is now Postponed (maintainer needs more info). If we don't receive additional information to help with the issue, it may be closed after three months.
Thanks!
Comment #24
smustgrave commentedWanted to bump 1 more time.
Comment #25
berdirHm. The code moved around, but mostly is unchanged, so I think it's there are still some useful bits here. And the split of EntityManager resulted in some weird splits and improper separation of concerns. For example \Drupal\Core\Field\FieldDefinitionListener::onFieldDefinitionCreate clears the field map and updates the underlying key value store, and calls \Drupal\Core\Entity\EntityFieldManager::setFieldMap() which doesn't actually persist the field map, that seems pretty weird.