I'm seldom surprised by some of the horrors under the Magento hood, but today's little gem takes some beating.

On a setup I administer, there are over 200,000 address records. When you view an order in the backend, and click 'edit address', the server grinds away and eventually dies either because it hits the max_execution_time limit or runs out of RAM.

You might see an otherwise meaningless error like this:

Fatal error: Allowed memory size of 268435456 bytes exhausted (tried to allocate 79 bytes) in /home/somewhere/public_html/lib/Zend/Db/Statement/Pdo.php on line 290

The cause for this, is the strange manner in which Magento searches for addresses in app/code/core/Mage/Adminhtml/controllers/Sales/OrderController.

 * Edit order address form
public function addressAction()
    $addressId = $this->getRequest()->getParam('address_id');
    $address = Mage::getModel('sales/order_address')
    if ($address) {
        Mage::register('order_address', $address);
    } else {

The problem is in the $address->getCollection()->getItemById() chain. Magento creates and fully loads a collection of Address objects (which when you have 200k of them, takes a while). The final call in the chain, getItemById() takes the collection, iterates over it assigning each address to an array keyed by its entityId. It then returns any value which matches $addressId.

Now, there's another, really simple way to do the same thing. It doesn't involve instantiating 200,000 address objects, or iterating over them, or even using associative arrays. It's very familiar.

$address = Mage::getModel('sales/order_address')->load($addressId);

This one line does the same thing far more efficiently. Now, the thing that worries me, is I can't see any reason why they aren't doing this already? The change in code works, nothing appears to break and the speed boost is (obviously) immense.

So why is it not done this way?