[6.x] Small PHP CP performance improvements and UTF-8 fix - #15254
Merged
Conversation
The base64-encoded fieldtype config was always run through mb_convert_encoding() with the full mb_list_encodings() detection list. That's ~9.6us per request, and it can misdetect perfectly valid UTF-8 as another encoding and corrupt it — a config containing only emoji came back as Cyrillic mojibake. Checking mb_check_encoding() first skips the detection entirely for valid UTF-8, which is the normal case, and leaves the existing conversion in place as the fallback for genuinely non-UTF-8 input.
setItems() resolves every field into a Field object, and the very next call in the chain, setFields(), immediately threw that work away. Assigning the items directly skips the wasted resolution.
getItemData() looked each id up twice — once in authorizeItemData() to check the user can view it, and again in toItemArray() to build the response. Caching the found item on the instance halves the lookups. The cache is reset in __clone(), since the fieldtype repository hands out clones of a single instance per handle. Without it the first consumer's lookups get baked into the shared instance and inherited by every unrelated field of the same type. Authorization is unaffected — the cache holds the item, not a decision, so authorizeViewable() still runs on every call.
Assigning the collection directly left both instances sharing one collection object, where setItems() would have given the new instance its own. Nothing in core mutates it in place, but addons can subclass Fields and reach it through items(), and a clone costs one allocation against the resolveFields() call we're skipping. Note this is shallow, matching what setItems() did — items can contain validation rule objects, and those stay shared either way.
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
1 participant
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.
Skip encoding detection when the relationship config is already UTF-8
The base64-encoded fieldtype config was always run through
mb_convert_encoding()with the fullmb_list_encodings()detection list. Checkingmb_check_encoding()first skips it in the normal case.It's also a bug fix. That detection list could misdetect valid UTF-8 as some other encoding and mangle it, so a config whose only multibyte content was emoji came back as Cyrillic mojibake —
{"display":"😀👍"}became{"display":"ЁЯШАЁЯСН"}. Taking the fast path leaves it alone. The original conversion stays as the fallback for genuinely non-UTF-8 input (#566).Avoid resolving fields twice in
Fields::newInstance()setItems()resolves every item into aFieldobject, and the very next call in the chain,setFields(), immediately discarded that work. Assigning the items directly skips it.Memoize relationship item lookups
getItemData()looked each id up twice, once inauthorizeItemData()and again intoItemArray(). Caching the found item on the instance halves that. Roughly 24% offgetItemData()for a 50-item relationship field.The cache is reset in
__clone(), because the fieldtype repository hands out clones of a single instance per handle. Without that, the first consumer's lookups get baked into the shared instance and inherited by every unrelated field of the same type. Authorization is unaffected: the cache holds the item, not a decision, soauthorizeViewable()still runs on every call.