-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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.
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
Some optimizations #199
Some optimizations #199
Conversation
Have you tried benchmarking this change in real environment eg homepage load time? |
@@ -217,7 +217,7 @@ public function getOutput($clearPrevious = true) | |||
*/ | |||
public function pushSilent() | |||
{ | |||
array_push($this->_silentSaved, $this->_silent); | |||
$this->_silentSaved = $this->_silent; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the missing [] on purpose?
mage2-team not in this case. I did it in a single loop. However, all evidence found on google points to the fact that array_push is a lot slower and it's quite obvious, since it's a function reference. |
I can confirm that from a lot of other projects |
So should we replace all uses of function aliases as well ? Edit : what tools would you suggest to benchmark a Magento ? |
From the array_push documentation: Note: If you use array_push() to add one element to the array it's better to use $array[] = because in that way there is no overhead of calling a function. Even if the overhead is minimal, the improvement should not be overlooked, as it is an easy fix. Also, easier to read and the rest applies. |
* Moved general action-related functionality to \Magento\App\Action\Action in the library. Removed Magento\Core\Controller\Varien\Action and related logic from the Magento_Core module * Moved view-related methods from action interface to \Magento\App\ViewInterface with corresponding implementation * Moved redirect creation logic from the action interface to \Magento\App\Response\RedirectInterface * Moved Magento\Core common blocks to the library * Added reading of etc/integration/config.xml and etc/integration/api.xml files for API Integrations * Various improvements: * Email-related logic from the Core and Adminhtml modules consolidated in the new Email module * GitHub requests: * [#238](#238) -- Improve escaping HTML entities in URL * [#199](#199) -- Replaced function calls to array_push with adding the elements directly * [#182](#182) -- By default use collection _idFieldName for toOption* methods. * [#233](#233) -- Google Rich Snippet Code * [#339](#339) -- Correcting 'cahce' typo in documentation. * [#232](#232) -- Update app/code/core/Mage/Checkout/controllers/CartController.php (fix issue #27632) * Fixed bugs: * Fixed JavaScript error when printing orders from the frontend * Fixed Captcha problems on various forms when Captcha is enabled on the frontend * Fixed "Page not found" on category page if setting "Add Store Code to Urls" to "Yes" in the backend config * Fixed Fatal error when creating shipping label for returns
Hello and-ers, Thank you for your contribution! |
[Firedrakes] MTF Configuration code clean up
…bsite_to_default_stock magento#199 - Assign main website to default stock
Replaced function calls to array_push with adding the elements directly. This has 2 important advantages.