Problem/Motivation
Since #3548884: SourceValueItem field synchronized translations, SourceValueItem::setValue() calls SourceTree::ensureNodeIds(), which builds typed config data for the whole tree to find nodes without a node_id.
setValue() runs on every entity load, and a stored tree already has its ids, so the walk is pure cost.
In Display Builder, one settings request set the field 80 times, and the walks took about 23% of its profiled time.
On a 2690-node page, they added about 0.4 s to the Canvas load.
Steps to reproduce
- Store a component with nested slot sources in a
ui_patterns_sourcefield. - Profile a load of the entity:
Traverser::traverse()builds typed data for every element of the tree, each time.
Proposed resolution
Return early from ensureNodeIds() when no node the traversal reaches lacks an id.
The check walks the plain array where the traverser assigns ids: the root, then the variant, props and slot sources of each component.
It does not look into a source whose settings schema is not a mapping or a sequence, such as entity_field, since the traverser does not either.
It may walk a tree that needs nothing, never skip one that needs ids.
On 426 stored Display Builder revisions with their nested ids removed, it flagged all 411 trees the traversal changed.
Remaining tasks
- Review.
User interface changes
None.
API changes
None.
Data model changes
None.
AI Generated: Yes (problem found and fix)
Issue fork ui_patterns-3630456
Show commands
Start 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:
Comments
Comment #3
mogtofu33 commentedComment #4
mogtofu33 commentedSome performance benchmark for Display Builder in this comment
Comment #5
mogtofu33 commentedComment #6
just_like_good_vibesHello,
thank you for reporting, but the solution seems a bit "too complex" for now.
i understand you are trying to push the limits and see where are the bottlenecks.
for the moment is subject is not all a blocker and may be addressed later.
Comment #7
mogtofu33 commentedUnderstood, the check has to follow where the traverser assigns ids, which makes it heavier than the problem looks.
Display Builder is the main payer: an instance keeps its whole layout in a source field, so each read of its sources walks the tree, and in our benchmark this MR took a page layout's builder page from 983 to 274 ms.
Until something lands here, Display Builder ships this MR as a patch in its composer.json, with UI Patterns pinned to the version it applies on.
If you see a simpler approach later, I will rework the MR.