-
Notifications
You must be signed in to change notification settings - Fork 684
Description
What happened?
Description
A bit difficult to describe the chain reaction but many sites + Craft Commerce + a matrix field with propagation method custom makes a project of us nearly unusable for customers.
Here is what happens:
- we have a Craft CMS Project with many sites (10+ for many languages)
- we have Craft Commerce (important for later)
- we have an entry with a matrix field with propagation method custom
- no other relevant plugins
- the entry has 20 simple nested entries with only text - nothing else, no relations
Rendering the entry edit form takes a really long time (sometimes 20s) with sometimes 3000 duplicate queries. The relevant stack trace is kinda this
#1 /app/vendor/craftcms/cms/src/elements/NestedElementManager.php(347): craft\web\View->renderObjectTemplate()
#2 /app/vendor/craftcms/cms/src/fields/Matrix.php(549): craft\elements\NestedElementManager->getSupportedSiteIds()
#3 /app/vendor/craftcms/cms/src/elements/Entry.php(1097): craft\fields\Matrix->getSupportedSitesForElement()
#4 /app/vendor/craftcms/cms/src/helpers/ElementHelper.php(317): craft\elements\Entry->getSupportedSites()
#5 /app/vendor/craftcms/cms/src/helpers/ElementHelper.php(358): craft\helpers\ElementHelper::supportedSitesForElement()
#6 /app/vendor/craftcms/cms/src/base/Element.php(5782): craft\helpers\ElementHelper::siteStatusesForElement()
#7 /app/vendor/craftcms/cms/src/fieldlayoutelements/BaseField.php(384): craft\base\Element->getIsCrossSiteCopyable()
#8 /app/vendor/craftcms/cms/src/fieldlayoutelements/CustomField.php(591): craft\fieldlayoutelements\BaseField->formHtml()
craft\fields\Matrix::formHtml()is calledgetIsCrossSiteCopyableneeds to check for available sitescraft\elements\NestedElementManager->getSupportedSiteIds()requests the siteIds- first Issue https://github.com/craftcms/cms/blob/5.8.20/src/elements/NestedElementManager.php#L344 will request the very same Owner of each block for each site again and again
$siteOwner = $elementsService->getElementById($owner->id, get_class($owner), $siteId);- second issue:
\craft\web\View::renderObjectTemplateis called with it's behavior to include all object variables https://github.com/craftcms/cms/blob/5.8.20/src/web/View.php#L725-L745
if ($object instanceof Model) {
foreach ($object->attributes() as $name) {
if (!isset($variables[$name]) && str_contains($template, $name)) {
$variables[$name] = $object->$name;
}
}
}
if ($object instanceof Arrayable) {
// See if we should be including any of the extra fields
$extra = [];
foreach ($object->extraFields() as $field => $definition) {
if (is_int($field)) {
$field = $definition;
}
if (preg_match('/\b' . preg_quote($field, '/') . '\b/', $template)) {
$extra[] = $field;
}
}
$variables += $object->toArray([], $extra, false);
}- third issue: this will lead to
Entry::getAuthors()
#0 /app/vendor/craftcms/cms/src/elements/Entry.php(1675): craft\elements\Entry->getAuthors()
#1 /app/vendor/craftcms/cms/src/elements/Entry.php(1651): craft\elements\Entry->getAuthorIds()
#2 /app/vendor/yiisoft/yii2/base/Component.php(139): craft\elements\Entry->getAuthorId()
#3 /app/vendor/craftcms/cms/src/base/Element.php(2649): yii\base\Component->__get()
#4 /app/vendor/craftcms/cms/src/base/NestedElementTrait.php(136): craft\base\Element->__get()
#5 /app/vendor/yiisoft/yii2/base/ArrayableTrait.php(126): craft\elements\Entry->__get()
#6 /app/vendor/craftcms/cms/src/base/Element.php(2809): craft\base\Element->traitToArray()
#7 /app/vendor/craftcms/cms/src/web/View.php(751): craft\base\Element->toArray()
fetching each author for the very same entry again and again
7) 4th issue: Commerce joins and fetches addressIds
https://github.com/craftcms/commerce/blob/a46c16d17bd0c107a60a2f6e268915dd7c30f211/src/Plugin.php#L786-L799
Event::on(UserQuery::class, UserQuery::EVENT_AFTER_POPULATE_ELEMENTS, function(PopulateElementsEvent $event) {
$users = $event->elements;
$customerIds = ArrayHelper::getColumn($users, 'id');
if (empty($customerIds)) {
return;
}
$customers = (new Query())
->select(['customerId', 'primaryBillingAddressId', 'primaryShippingAddressId'])
->from([Table::CUSTOMERS])
->where(['customerId' => $customerIds])
->all();In combination, we have a massive amount of queries.
Solution
For the time being we solved it and reduced the amount of time back to 2s with 2 simple (quick and dirty) vendor changes. It would be awesome if you could do something similar in a way or another.
cms/src/elements/NestedElementManager.php
Line 344 in cec2d4f
$siteOwner = $elementsService->getElementById($owner->id, get_class($owner), $siteId);
Could easily remember previously fetched elements without any issues or downsides in this case. You could implement a simple variable, that remembers already fetched owners for other sites by key "$id:$elementClass:$siteId"
craft\web\Viewcould include an additional parameter if the Element/Model/Object should really include all it's attributes into the$variablesarray (->toArray()) or if it might be enough to just pass the object itself. Of course this will be a breaking change, so set it to true by default and include a setting in\craft\fields\Matrixif it's required or not.
If the developer turns it off on purpose there is no need to serialize everything which will lead to a massive performance boost. Speaking of which maybe future Craft CMS versions could default to "do not serialize everything" as it caused many issues in the past for me.
Craft CMS version
5.8.x
PHP version
8.4
Operating system and version
No response
Database type and version
No response
Image driver and version
No response
Installed plugins and versions
- Craft CMS Commerce 5.5.0