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

  1. Store a component with nested slot sources in a ui_patterns_source field.
  2. 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)

Command icon 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

mogtofu33 created an issue. See original summary.

mogtofu33’s picture

Assigned: mogtofu33 » Unassigned
Status: Needs work » Needs review
Issue tags: +display builder
mogtofu33’s picture

Some performance benchmark for Display Builder in this comment

mogtofu33’s picture

just_like_good_vibes’s picture

Hello,
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.

mogtofu33’s picture

Understood, 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.