search

mage-os / mageos-magento2

Work in progress.
Open Software License 3.0
214 stars 45 forks source link

feat(catalog): faster category product count #25

Closed damienwebdev closed 1 year ago

damienwebdev commented 1 year ago

Previously, in the admin panel, computing the CategoryTree UI (the left-hand element of the page that loads when you try to view the categories of your store) was wildly slow for the root scope when there were many is_anchor categories. This was due to several bugs:

  1. Use of a missing index: catalog_category_product_index that is now defunct and always empty. It was replaced by catalog_category_product_index_store*. However, this table did not exist for the store0. This diff both adds that missing table and changes the collection to use the new tables.

  2. A bad computation of the rootCategoryIds for the root store, causing a bad join condition against the temporary category recursion table resulting in the resulting index table (catalog_category_product_index_store0) always being empty.

  3. An exceedingly slow private method (getProductsCountFromCategoryTable) which should not have been merged in the first place given how slow it is. I have removed this method. Removing this does cause a small UX bug when the index is empty: the category page will show 0 products before an index has run. It's either this or perform the computation on every page refresh. I see this an acceptable break, given how slow it is to attempt to compute these values manually.

@Nuranto I have extracted your commit from https://github.com/magento/magento2/pull/34111/ and I am personally sorry for the way that PR was handled.

Fixes: https://github.com/magento/magento2/issues/34109 Fixes: https://github.com/magento/magento2/pull/35216

Here is a patch for

v2.4.4-p3

damienwebdev commented 1 year ago

@Vinai I think the semver check is broken. Additionally, I have no idea how to write tests for this, I have no idea wtf to do with the temp table/indexer changes I made. Debugging this was a nightmare. @vitaliy-golomoziy

Vinai commented 1 year ago

Thanks @damienwebdev - I agree the semver check seems to be broken. Probably we should disable it until we have the resources.

It would be great if you could push an empty commit to this PR so it triggers the current set of checks. I can't force a run manually myself.

Vinai commented 1 year ago

I'll approve the changes and merge them despite my two small comments, as I've gone over the patch and understand what it does and tried it out locally.

However, if one of you could get back to me with an answer to the question when you have time I would appreciate it.