[6.x] Bard server-side performance improvements - #15258
Merged
Merged
Conversation
The interface declares a $this return. Removes the corresponding PHPStan baseline entry.
preload() called meta() twice on the same throwaway asset field, generating the whole Assets preload payload — container lookup, permission checks and blueprint columns — twice over.
Relationship and Assets both build their preload 'data' from getItemData(), so link data can be resolved directly from it. The rest of the payload — selection URLs, columns, creatables, container permissions — was being built and thrown away once per link in the content. Also fixes a 500 when a Bard field contains an asset link but has no container configured: the old path went through Assets::preload(), which throws UndefinedContainerException.
Every Bard field rebuilt the toolbar's link types from scratch, generating a full preload payload per link type. A page with several Bard fields — or a Replicator whose sets each contain one — repeated that work per field. Keyed on the current site and user as well as the field config, because the payload embeds both: an entry link type with no configured collections falls back to the current site's routable collections, and the asset link type's meta carries the current user's container permissions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split out of #15158.
Generate the asset field's meta once
preload()calledmeta()twice on the same throwaway asset field, once forcontainerand once forcolumns. Each call builds the entire Assets preload payload — container lookup, five permission checks, blueprint columns — so half of that work was thrown away.Resolve link data without a full preload payload
linkDataForType()calledpreload()on a nested field just to read['data'][0].RelationshipandAssetsboth build thatdatafromgetItemData(), so it can come straight from there, skipping the selection URLs, columns, creatables and container permissions that were being built and discarded once per link in the content.It's also a bug fix. When a Bard field has an asset link in its content but no configured container, the old path went through
Assets::preload()→container(), which throwsUndefinedContainerExceptionand 500s the publish form. The new path returns the usual invalid-item marker instead.The fast path is guarded with
instanceofrather than amethod_exists('getItemData')check.Dictionary::getItemData()is private and returns a plain array, so duck-typing would fatal on a link type using it, where today it harmlessly resolves tonull.Cache the toolbar's link types for the request
Every Bard field rebuilt the toolbar's link types from scratch, generating a full preload payload per link type. A page with several Bard fields — or a Replicator whose sets each contain one — repeated that per field. Blink is request-scoped, so there's no cross-request staleness.
The key includes the current site and user as well as the field config, because the payload embeds both. An entry link type with no configured collections falls back to the collections routable in
Site::current(), and the asset link type's meta carries the current user's container permissions. Keying on the config alone serves one context's toolbar to another — withblogroutable inenonly andactusinfronly, the second site got the first site's collections.Return
$thisfrom theDataAwareRule'ssetData()The interface declares a
$thisreturn. Removes the corresponding PHPStan baseline entry.