FieldInfo::getBundleInstances() caches Field & FieldInstance objects for a given [$entity_type, $bundle] in one single cached array (with separate 'fields' & 'instances' entries, but still in the same array).
That's a problem because:
- unserializing the array means unserializing all Field and FieldInstance structures
-- unserializing a FieldInstance currently triggers fetching the corresponding Field definition with FieldInfo::getField().
--- FieldInfo::getField() doesn't find the Field in its static cache, because we are in the process of unserializing them. So it fetches it from config()
So FieldInfo::getBundleInstances() triggers one extra config() read per instance for nothing.
Note:
#2073297: Remove cache of Field / FieldInstance objects has a broader proposal to reconsider Field/FieldInfo caching. But that would need to be benchmarked, and those benchmarks should happen against a state of HEAD that is not artificially slow because it's broken.
Comment | File | Size | Author |
---|---|---|---|
#7 | field_cache_serialization-2071997-7.patch | 3.62 KB | yched |
#7 | interdiff.txt | 731 bytes | yched |
#1 | field_cache_serialization-2071997-1.patch | 3.62 KB | yched |
Comments
Comment #1
yched CreditAttribution: yched commentedAttached patch fixes this by caching fields and instances in separate cache entries, that can be unserialized separately.
This might not be the final state for this "caching" code in D8, I'd like to try avoiding serialization of Field & FieldInstance config entities and cache only config ids, relying on the config() cache for the objects themselves instead. But that would need to be benchmarked, and those benchmark should happen against a state of HEAD that is not artificially slow because it's broken.
Comment #2
BerdirIn response to the question asked in #2015697: Convert field type to typed data plugin for file and image modules, nope, this issue does not fix #2025451: Drupal\entity_reference\Tests\EntityReferenceAdminTest fails on PHP 5.4.9 and libxml 2.9.0, still getting the same failures and (and a apache segfault).
Comment #3
yched CreditAttribution: yched commentedBench on a moderately complex example:
- two node types, each with: 5 shared fields + 5 fields of their own
- page listing 5 nodes of each type
HEAD: 238.923 ms
Patch: 230.992 ms -> -3.3%
Comment #4
Anonymous (not verified) CreditAttribution: Anonymous commentedlooks like a good improvement from a simple cache addition to me.
can you open a follow up for the "just cache config ids" bit?
Comment #5
yched CreditAttribution: yched commentedYup, created #2073297: Remove cache of Field / FieldInstance objects, I'll update the OP.
(note it's not really a followup, it's a task in itself, and this issue here is more like a hotfix for something that's currently broken)
Comment #6
yched CreditAttribution: yched commentedtagging
Comment #7
yched CreditAttribution: yched commentedTypo in code comment.
Comment #8
Anonymous (not verified) CreditAttribution: Anonymous commentedlooks good to me.
Comment #9
alexpottCommitted e2e8b97 and pushed to 8.x. Thanks!
Comment #10.0
(not verified) CreditAttribution: commentedLink to #2073297: Remove cache of Field / FieldInstance objects