OpenMage / magento-lts

Official OpenMage LTS codebase | Migrate easily from Magento Community Edition in minutes! Download the source code for free or contribute to OpenMage LTS | Security vulnerability patches, bug fixes, performance improvements and more.
https://www.openmage.org
Open Software License 3.0
870 stars 436 forks source link

Discovered another performance issue, this time in Mage_Adminhtml_Block_Sales_Order_Create_Customer_Grid #592

Closed pepijnblom closed 5 years ago

pepijnblom commented 5 years ago

A client was complaining about slow performance when re-ordering a previous order for their customers. I traced it to Mage_Adminhtml_Block_Sales_Order_Create_Customer_Grid which is added to the /admin/sales_order_create page even if you have already chosen a previous order to re-order. Normally you'd go to Sales/Orders and click Create New Order after which you must select a customer from the grid.

Considering the order & customer have already been chosen when re-ordering, this grid never needs to be added. It's currently running a query which is taking 29 seconds to run (11 or 12 left joins. Their customer_entity table holds 700,000+ customers and the customer_entity_varchar table holds 2.9 million records.

EDIT: I just did a bit more searching and you should see: app/design/adminhtml/default/default/template/sales/order/create/form.phtml and then \Mage_Adminhtml_Block_Sales_Order_Create_Form::getCustomerSelectorDisplay

Seriously??

I wonder what you thoughts are on fixing this. I'm not sure what the ideal solution is here. I've managed locally by retrieving the adminhtml/session_quote and then the customer from that in the __construct of Mage_Adminhtml_Block_Sales_Order_Create_Customer_Grid. If a customer Id can be gotten then it just returns. Works just fine but I'm not sure it's the best solution to this problem.

colinmollenhour commented 5 years ago

I'd encourage you to go ahead and submit a PR with your working fix so we can see how it works.

pepijnblom commented 5 years ago

I decided to implement it as a module in the shop I mentioned.

You can check it out here and implement the change if you see fit. It shaved off 30s of the load time for creating new orders from an existing order.

https://github.com/graciousagency/magento1-gridoptim

colinmollenhour commented 5 years ago

Hmm, what about also using wrapping the rendering of the block in a conditional in the form.phtml:

<?php if ($this->getCustomerId()): ?>
    <?php echo $this->getChildHtml('customer') ?>
<?php endif ?>
pepijnblom commented 5 years ago

Hmm that might work actually, but you'd have to flip the logic of the if. I'm going to have to test this some time and I'll make an MR if it works.

<?php if (!$this->getCustomerId()): ?>
    <?php echo $this->getChildHtml('customer') ?>
<?php endif ?>
colinmollenhour commented 5 years ago

Doh, thanks for the correction.. :)