search

magento-ecg / coding-standard

Magento PHP_CodeSniffer Coding Standard
MIT License
308 stars 99 forks source link

Magento 1.x - Performance Loop Sniff #44

Closed mladenilic closed 7 years ago

mladenilic commented 7 years ago

Performance Loop Sniff detects and reports when load/save/delete model methods are called in a loop. It's obvious why this is a bad practice, however it's quite common to run into this when dealing with Magento models.

Example: Magento Admin grids allow implementing "mass actions". In case of product grid, it allows changing product status, updating attributes and deleting products. Deleting n products will require you to loop through array of ids, load and delete products.

I'm generally wondering if there's a recommended way how to deal this cases? If you consider catalog products, with all their related models (inventory, gallery) using LSD methods in a loop seems unavoidable in some cases.

Best regards,

zlik commented 7 years ago

@mladenilic, there may be multiple problems if you call Magento entity model's load() method in a loop:

  1. Every call with produce one or more SQL queries - generates an overhead on MySQL side.
  2. There may be one or multiple observers listening to when the load() is called and triggering extra PHP methods, SQL queries or web services calls - consumes CPU, generates overhead on MySQL or web service side.
  3. The load() method will fetch many entity's attributes, however, usually only a few attributes are actually used - consumes memory to accommodate all the data.

This altogether may generate a huge overhead for MySQL, PHP, or web services if the number of iterations in the loop is large.

Usually, loading models in a loop can be replaced with using Magento collection. Magento collections allow loading the data for multiple entities with only one SQL query and controlling which attributes should be fetched from the database.

Note: when using Magento collections you should keep an eye on the memory usage and do not load more data than can be put into memory. Sometimes loading data in a loop with limit cannot be avoided. Also, Magento collections will instantiate a PHP object model for every entity. There is an overhead associated with that. So instead of Magento model or collections, you can use raw SQL queries for the best performance, for example for data import/export where thousands of entities must be processed really fast.

While calling load() in a loop should be generally avoided, calling save or delete in a loop is usually OK. Calling many lightweight SQL queries instead or one heavy long-running query affecting thousands of rows helps offload MySQL slaves and avoid a replication lag for a master-slave database configuration.