-
Notifications
You must be signed in to change notification settings - Fork 9.4k
Open
Labels
Issue: ready for confirmationReported on 2.4.xIndicates original Magento version for the Issue report.Indicates original Magento version for the Issue report.Triage: Dev.ExperienceIssue related to Developer Experience and needs help with Triage to Confirm or Reject itIssue related to Developer Experience and needs help with Triage to Confirm or Reject it
Description
Preconditions and environment
Magento 2.4.x
Steps to reproduce
- Review https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Catalog/Model/AbstractModel.php#L180 and there is a nested for in https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Eav/Model/Entity/Collection/AbstractCollection.php#L1236 and it calls
_setItemAttributeValue
once per attribute and_setItemAttributeValue
iterates all products and set the value which is O(n*m) [50 products in cart & 50 attributes leads to 2500 iterations]
n = number of attributes
m= numbers of products
Profiler Output

- In addition
setData
function in https://github.com/magento/magento2/blob/2.4-develop/app/code/Magento/Catalog/Model/AbstractModel.php#L180 does the same validation for all products instead it should be done once per attribute per collection given it doesn't change per product or per entityValue
Expected result
Iterates the number of rows from select query and then set once per product
Actual result
Iterates per attribute per product which increases the time complexity to multifold
Additional information
Proposal is to avoid the nested loop with the below
In Magento\Eav\Model\Entity\Collection\AbstractCollection::_loadAttributes
Replace
foreach ($values as $value) {
$this->_setItemAttributeValue($value);
}
With
$attributeCode = $data = [];
$entityIdField = $this->getEntity()->getEntityIdField();
foreach ($values as $value) {
$entityId = $value[$entityIdField];
$attributeId = $value['attribute_id'];
if(!isset($attributeCode[$attributeId])) {
$attributeCode[$attributeId] = array_search($attributeId, $this->_selectAttributes);
if (!$attributeCode[$attributeId]) {
$attribute = $this->_eavConfig->getAttribute(
$this->getEntity()->getType(),
$attributeId
);
$attributeCode[$attributeId] = $attribute->getAttributeCode();
}
$data[$entityId][$attributeCode[$attributeId]] = $value['value'];
}
}
if($data)
$this->_setItemAttributeValue($data);
Replace Magento\Eav\Model\Entity\Collection\AbstractCollection::_setItemAttributeValue
with the below
protected function _setItemAttributeValue($valueInfo)
{
foreach ($valueInfo as $entityId => $value) {
if (!isset($this->_itemsById[$entityId])) {
throw new LocalizedException(
__('A header row is missing for an attribute. Verify the header row and try again.')
);
}
$object =$this->_itemsById[$entityId][0];
$value = array_replace($object->getData(), $value);
$object->setData($value);
}
return $this;
}
Thus avoid calling _setItemAttributeValue equivalent to the number of attributes and internally it calls all products per attribute
Additional Questions
Since array_replace is memory intensive for large array would it benefit over all or not ?
Triage and priority
- Severity: S0 - Affects critical data or functionality and leaves users without workaround.
- Severity: S1 - Affects critical data or functionality and forces users to employ a workaround.
- Severity: S2 - Affects non-critical data or functionality and forces users to employ a workaround.
- Severity: S3 - Affects non-critical data or functionality and does not force users to employ a workaround.
- Severity: S4 - Affects aesthetics, professional look and feel, “quality” or “usability”.
Metadata
Metadata
Assignees
Labels
Issue: ready for confirmationReported on 2.4.xIndicates original Magento version for the Issue report.Indicates original Magento version for the Issue report.Triage: Dev.ExperienceIssue related to Developer Experience and needs help with Triage to Confirm or Reject itIssue related to Developer Experience and needs help with Triage to Confirm or Reject it
Type
Projects
Status
Ready for Confirmation