Skip to content

Performance: Getting rid of array_merge in loop #26355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Feb 8, 2020
Merged

Performance: Getting rid of array_merge in loop #26355

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
0228d4b
Refactor + `array_merge` in loop replace + PHPCS fixes
lbajsarowicz Jan 12, 2020
882fa18
Code Review changes
lbajsarowicz Jan 13, 2020
c3b8a56
Fix PHPUnit fatal error related with class methods not implemented
lbajsarowicz Jan 14, 2020
cc38aed
Merge remote-tracking branch 'origin/2.4-develop' into refactor/perfo…
lbajsarowicz Jan 16, 2020
d3dbcee
Module Catalog: Blocks
lbajsarowicz Jan 16, 2020
f37c228
Added Unit Tests for 2 cases and introduce fallback ( [[]] )
lbajsarowicz Jan 18, 2020
390b919
Add description
lbajsarowicz Jan 18, 2020
4e42ad5
Merge remote-tracking branch 'origin/2.4-develop' into refactor/perfo…
lbajsarowicz Jan 18, 2020
24a3447
Update Upsell.php
engcom-Golf Jan 24, 2020
2e346af
Semantic Version Checker fix
lbajsarowicz Jan 28, 2020
e8f5632
Merge remote-tracking branch 'origin/2.4-develop' into refactor/perfo…
lbajsarowicz Jan 30, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 28 additions & 25 deletions app/code/Magento/Catalog/Block/Product/ProductList/Related.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,15 @@

namespace Magento\Catalog\Block\Product\ProductList;

use Magento\Catalog\Block\Product\AbstractProduct;
use Magento\Catalog\Block\Product\Context;
use Magento\Catalog\Model\Product;
use Magento\Catalog\Model\Product\Visibility as ProductVisibility;
use Magento\Catalog\Model\ResourceModel\Product\Collection;
use Magento\Checkout\Model\ResourceModel\Cart as CartResourceModel;
use Magento\Checkout\Model\Session as CheckoutSession;
use Magento\Framework\DataObject\IdentityInterface;
use Magento\Framework\Module\Manager;
use Magento\Framework\View\Element\AbstractBlock;

/**
Expand All @@ -16,8 +24,7 @@
* @SuppressWarnings(PHPMD.LongVariable)
* @since 100.0.2
*/
class Related extends \Magento\Catalog\Block\Product\AbstractProduct implements
\Magento\Framework\DataObject\IdentityInterface
class Related extends AbstractProduct implements IdentityInterface
{
/**
* @var Collection
Expand All @@ -27,53 +34,50 @@ class Related extends \Magento\Catalog\Block\Product\AbstractProduct implements
/**
* Checkout session
*
* @var \Magento\Checkout\Model\Session
* @var CheckoutSession
*/
protected $_checkoutSession;

/**
* Catalog product visibility
*
* @var \Magento\Catalog\Model\Product\Visibility
* @var ProductVisibility
*/
protected $_catalogProductVisibility;

/**
* Checkout cart
*
* @var \Magento\Checkout\Model\ResourceModel\Cart
* @var CartResourceModel
*/
protected $_checkoutCart;

/**
* @var \Magento\Framework\Module\Manager
* @var Manager
*/
protected $moduleManager;

/**
* @param \Magento\Catalog\Block\Product\Context $context
* @param \Magento\Checkout\Model\ResourceModel\Cart $checkoutCart
* @param \Magento\Catalog\Model\Product\Visibility $catalogProductVisibility
* @param \Magento\Checkout\Model\Session $checkoutSession
* @param \Magento\Framework\Module\Manager $moduleManager
* @param Context $context
* @param CartResourceModel $checkoutCart
* @param ProductVisibility $catalogProductVisibility
* @param CheckoutSession $checkoutSession
* @param Manager $moduleManager
* @param array $data
*/
public function __construct(
\Magento\Catalog\Block\Product\Context $context,
\Magento\Checkout\Model\ResourceModel\Cart $checkoutCart,
\Magento\Catalog\Model\Product\Visibility $catalogProductVisibility,
\Magento\Checkout\Model\Session $checkoutSession,
\Magento\Framework\Module\Manager $moduleManager,
Context $context,
CartResourceModel $checkoutCart,
ProductVisibility $catalogProductVisibility,
CheckoutSession $checkoutSession,
Manager $moduleManager,
array $data = []
) {
$this->_checkoutCart = $checkoutCart;
$this->_catalogProductVisibility = $catalogProductVisibility;
$this->_checkoutSession = $checkoutSession;
$this->moduleManager = $moduleManager;
parent::__construct(
$context,
$data
);
parent::__construct($context, $data);
}

/**
Expand All @@ -84,7 +88,7 @@ public function __construct(
protected function _prepareData()
{
$product = $this->getProduct();
/* @var $product \Magento\Catalog\Model\Product */
/* @var $product Product */

$this->_itemCollection = $product->getRelatedProductCollection()->addAttributeToSelect(
'required_options'
Expand Down Expand Up @@ -139,12 +143,11 @@ public function getItems()
*/
public function getIdentities()
{
$identities = [];
$identities = [[]];
foreach ($this->getItems() as $item) {
// phpcs:ignore Magento2.Performance.ForeachArrayMerge
$identities = array_merge($identities, $item->getIdentities());
$identities[] = $item->getIdentities();
}
return $identities;
return array_merge(...$identities);
}

/**
Expand Down
72 changes: 38 additions & 34 deletions app/code/Magento/Catalog/Block/Product/ProductList/Upsell.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,16 @@

namespace Magento\Catalog\Block\Product\ProductList;

use Magento\Catalog\Block\Product\AbstractProduct;
use Magento\Catalog\Block\Product\Context;
use Magento\Catalog\Model\Product;
use Magento\Catalog\Model\Product\Visibility as ProductVisibility;
use Magento\Catalog\Model\ResourceModel\Product\Collection;
use Magento\Checkout\Model\ResourceModel\Cart as CartResourceModel;
use Magento\Checkout\Model\Session as CheckoutSession;
use Magento\Framework\DataObject;
use Magento\Framework\DataObject\IdentityInterface;
use Magento\Framework\Module\Manager;

/**
* Catalog product upsell items block
Expand All @@ -15,16 +24,15 @@
* @SuppressWarnings(PHPMD.LongVariable)
* @since 100.0.2
*/
class Upsell extends \Magento\Catalog\Block\Product\AbstractProduct implements
\Magento\Framework\DataObject\IdentityInterface
class Upsell extends AbstractProduct implements IdentityInterface
{
/**
* @var int
*/
protected $_columnCount = 4;

/**
* @var \Magento\Framework\DataObject[]
* @var DataObject[]
*/
protected $_items;

Expand All @@ -41,53 +49,50 @@ class Upsell extends \Magento\Catalog\Block\Product\AbstractProduct implements
/**
* Checkout session
*
* @var \Magento\Checkout\Model\Session
* @var CheckoutSession
*/
protected $_checkoutSession;

/**
* Catalog product visibility
*
* @var \Magento\Catalog\Model\Product\Visibility
* @var ProductVisibility
*/
protected $_catalogProductVisibility;

/**
* Checkout cart
*
* @var \Magento\Checkout\Model\ResourceModel\Cart
* @var CartResourceModel
*/
protected $_checkoutCart;

/**
* @var \Magento\Framework\Module\Manager
* @var Manager
*/
protected $moduleManager;

/**
* @param \Magento\Catalog\Block\Product\Context $context
* @param \Magento\Checkout\Model\ResourceModel\Cart $checkoutCart
* @param \Magento\Catalog\Model\Product\Visibility $catalogProductVisibility
* @param \Magento\Checkout\Model\Session $checkoutSession
* @param \Magento\Framework\Module\Manager $moduleManager
* @param Context $context
* @param CartResourceModel $checkoutCart
* @param ProductVisibility $catalogProductVisibility
* @param CheckoutSession $checkoutSession
* @param Manager $moduleManager
* @param array $data
*/
public function __construct(
\Magento\Catalog\Block\Product\Context $context,
\Magento\Checkout\Model\ResourceModel\Cart $checkoutCart,
\Magento\Catalog\Model\Product\Visibility $catalogProductVisibility,
\Magento\Checkout\Model\Session $checkoutSession,
\Magento\Framework\Module\Manager $moduleManager,
Context $context,
CartResourceModel $checkoutCart,
ProductVisibility $catalogProductVisibility,
CheckoutSession $checkoutSession,
Manager $moduleManager,
array $data = []
) {
$this->_checkoutCart = $checkoutCart;
$this->_catalogProductVisibility = $catalogProductVisibility;
$this->_checkoutSession = $checkoutSession;
$this->moduleManager = $moduleManager;
parent::__construct(
$context,
$data
);
parent::__construct($context, $data);
}

/**
Expand All @@ -98,7 +103,7 @@ public function __construct(
protected function _prepareData()
{
$product = $this->getProduct();
/* @var $product \Magento\Catalog\Model\Product */
/* @var $product Product */
$this->_itemCollection = $product->getUpSellProductCollection()->setPositionOrder()->addStoreFilter();
if ($this->moduleManager->isEnabled('Magento_Checkout')) {
$this->_addProductAttributesAndPrices($this->_itemCollection);
Expand Down Expand Up @@ -181,8 +186,8 @@ public function getRowCount()
*/
public function setColumnCount($columns)
{
if ((int) $columns > 0) {
$this->_columnCount = (int) $columns;
if ((int)$columns > 0) {
$this->_columnCount = (int)$columns;
}
return $this;
}
Expand Down Expand Up @@ -231,8 +236,8 @@ public function getIterableItem()
*/
public function setItemLimit($type, $limit)
{
if ((int) $limit > 0) {
$this->_itemLimits[$type] = (int) $limit;
if ((int)$limit > 0) {
$this->_itemLimits[$type] = (int)$limit;
}
return $this;
}
Expand All @@ -250,9 +255,9 @@ public function getItemLimit($type = '')
}
if (isset($this->_itemLimits[$type])) {
return $this->_itemLimits[$type];
} else {
return 0;
}

return 0;
}

/**
Expand All @@ -262,11 +267,10 @@ public function getItemLimit($type = '')
*/
public function getIdentities()
{
$identities = [];
foreach ($this->getItems() as $item) {
// phpcs:ignore Magento2.Performance.ForeachArrayMerge
$identities = array_merge($identities, $item->getIdentities());
}
return $identities;
$identities = array_map(function (DataObject $item) {
return $item->getIdentities();
}, $this->getItems()) ?: [[]];

return array_merge(...$identities);
}
}
40 changes: 36 additions & 4 deletions app/code/Magento/Catalog/Test/Unit/Block/Product/ProductList/UpsellTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,21 @@
*/
namespace Magento\Catalog\Test\Unit\Block\Product\ProductList;

use Magento\Catalog\Block\Product\ProductList\Upsell as UpsellBlock;
use Magento\Catalog\Model\Product;

class UpsellTest extends \PHPUnit\Framework\TestCase
{
const STUB_EMPTY_ARRAY = [];
/**
* @var \Magento\Catalog\Block\Product\ProductList\Upsell
* @var UpsellBlock
*/
protected $block;

protected function setUp()
{
$objectManager = new \Magento\Framework\TestFramework\Unit\Helper\ObjectManager($this);
$this->block = $objectManager->getObject(\Magento\Catalog\Block\Product\ProductList\Upsell::class);
$this->block = $objectManager->getObject(UpsellBlock::class);
}

protected function tearDown()
Expand All @@ -26,10 +30,10 @@ protected function tearDown()
public function testGetIdentities()
{
$productTag = ['compare_item_1'];
$product = $this->createMock(\Magento\Catalog\Model\Product::class);
$product = $this->createMock(Product::class);
$product->expects($this->once())->method('getIdentities')->will($this->returnValue($productTag));

$itemsCollection = new \ReflectionProperty(\Magento\Catalog\Block\Product\ProductList\Upsell::class, '_items');
$itemsCollection = new \ReflectionProperty(UpsellBlock::class, '_items');
$itemsCollection->setAccessible(true);
$itemsCollection->setValue($this->block, [$product]);

Expand All @@ -38,4 +42,32 @@ public function testGetIdentities()
$this->block->getIdentities()
);
}

public function testGetIdentitiesWhenItemGetIdentitiesReturnEmptyArray()
{
$product = $this->createMock(Product::class);
$product->expects($this->once())->method('getIdentities')
->willReturn(self::STUB_EMPTY_ARRAY);

$itemsCollection = new \ReflectionProperty(UpsellBlock::class, '_items');
$itemsCollection->setAccessible(true);
$itemsCollection->setValue($this->block, [$product]);

$this->assertEquals(
self::STUB_EMPTY_ARRAY,
$this->block->getIdentities()
);
}

public function testGetIdentitiesWhenGetItemsReturnEmptyArray()
{
$itemsCollection = new \ReflectionProperty(UpsellBlock::class, '_items');
$itemsCollection->setAccessible(true);
$itemsCollection->setValue($this->block, self::STUB_EMPTY_ARRAY);

$this->assertEquals(
self::STUB_EMPTY_ARRAY,
$this->block->getIdentities()
);
}
}
Loading