There are only a couple of calls to EntityFieldManager::getFieldMap() on a cold cache.
FieldStorage::getBundles() and layout builder's FieldBlockDeriver.
#3537962: EntityFieldManager::getFieldDefinitions() per-bundle caching can be expensive makes the field map less special by building it from field definitions. However because it needs to get field definitions for every entity type, this builds bundle field info for an additional four entity types over and above the ones that would otherwise build it.
I think we can probably reduce those if we build views and layout builder field information in a different way.
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
catchComment #4
berdirNot sure what to think of this.
I mean, avoiding that loop is why the field map exists. Aren't we risking to improve cold cache performance at the cost of warm caches and other scenarios? Looking through getBundles() calls, it's mostly, views data, field UI and update/delete hooks around fields.
Should we then consider to deprecate the field map entirely? Maybe solve getBundles() differently? If it's just concerned about configurable fields, maybe we could add lookup keys to field_config config entities and do direct entity queries? I suppose that would be a lot of lookups as we're doing it per field and this is then statically cached.
Not sure yet, just thinking out loud.
Comment #5
catchI don't think it will make much difference in warm cache scenarios. Views data has its own cache a level above this one, so once the views data cache is built, it doesn't touch this. Field UI is fairly far outside the critical path, and when caches are warm it will end up getting a cache hit in EntityFieldManager::getFieldDefinitions() for each bundle.
So I found this due to #3045509: EntityFieldManager key/value field map gets out of sync, doesn't recognise bundle fields which has some more details, that issue stops the field map from using key/value to fix that bug, then I realised even though it does that, it was more or less performance neutral because we already mostly do all the things the field map was trying to avoid on cold cache requests anyway.
Comment #7
amateescu commentedI've always thought that we should make it more useful by adding a few more pieces of info (e.g. translatable) and use it more, not less :)
#3577321: [PP-1] Use a JSON column for config entity queries would help a lot with that. Not sure it requires multiple lookups though, because the latest JSON column implementation supports the IN operator, so we could have a single lookup for all known bundles.
Comment #8
catch#3045509: EntityFieldManager key/value field map gets out of sync, doesn't recognise bundle fields landed and the field map no-longer uses the key/value store.
This is green but it doesn't tackle FieldBlockDeriver yet. That would be much easier to tackle after #3573879: Mark layout_builder_expose_all_field_blocks obsolete - because then we could build the field blocks based on the view display settings additively, rather than getting all entities + bundles and then removing them based on view display settings.
Comment #9
smustgrave commentedSince this is an API change assuming we will need a CR?
Comment #10
catchYes probably but it could use code review first, the CR will be very short 'new method added to interface that's also on the base class'.