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

Optimize breadcrumb computation performances #8896

Merged
merged 9 commits into from Apr 13, 2018

Conversation

Projects
None yet
6 participants
@jocel1
Contributor

jocel1 commented Mar 29, 2018

Questions Answers
Branch? develop
Description? Greatly improve performances with a lot of categories
Type? improvement
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? http://forge.prestashop.com/browse/BOOM-3666
How to test? load a product page in BO with a lot of categories in the database

It actually improves both frontoffice & backoffice performances when a lot of categories are used


This change is Reviewable

jocel1 added some commits Mar 28, 2018

speedup category creations during install
speedup get parent category by using two queries overall instead of one query by parent.
@jocel1

This comment has been minimized.

Contributor

jocel1 commented Mar 29, 2018

@jocel1 jocel1 requested a review from mickaelandrieu Mar 29, 2018

*
* @param int $idLang Language ID
*
* @return array|false|mysqli_result|null|PDOStatement|resource

This comment has been minimized.

@mickaelandrieu

mickaelandrieu Mar 29, 2018

Contributor

are you sure it is all types you may return? :trollface:

This comment has been minimized.

@jocel1

jocel1 Mar 29, 2018

Contributor

mmm perhaps mysql_result is missing here :trollface:

if (count(Category::getCategoriesWithoutParent()) > 1 && Configuration::get('PS_MULTISHOP_FEATURE_ACTIVE') && count(Shop::getShops(true, null, true)) != 1) {
if (count(Category::getCategoriesWithoutParent()) > 1
&& Configuration::get('PS_MULTISHOP_FEATURE_ACTIVE')
&& count(Shop::getShops(true, null, true)) != 1) {

This comment has been minimized.

@mickaelandrieu

mickaelandrieu Mar 29, 2018

Contributor
- != 1
+ !== 1
$sql .= ' AND c.`id_parent` != 0';
}
AND `id_lang` = '.(int) $idLang.Shop::addSqlRestrictionOnLang('cl').')';
if (Shop::isFeatureActive() && Shop::getContext() == Shop::CONTEXT_SHOP) {

This comment has been minimized.

@mickaelandrieu

mickaelandrieu Mar 29, 2018

Contributor

can you use "===" instead of "==" when possible?

@jocel1

This comment has been minimized.

Contributor

jocel1 commented Mar 29, 2018

With 12000 categories: https://blackfire.io/profiles/compare/4da5f0af-3bdd-4898-9233-4521842db8ed/graph
The number of sql queries is a bit lower now :trollface:

@jocel1 jocel1 added this to the 1.7.4.0 milestone Mar 30, 2018

@mickaelandrieu mickaelandrieu requested a review from eternoendless Mar 30, 2018

@jocel1 jocel1 changed the title from Optimize category to Optimize breadcrumb computation performances Apr 2, 2018

$rootTreeInfo = Db::getInstance(_PS_USE_SQL_SLAVE_)->getRow(
'SELECT c.`nleft`, c.`nright` FROM `'._DB_PREFIX_.'category` c
WHERE c.`id_category` = '.(int) $context->shop->id_category
);

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Apr 4, 2018

Member

It seems this request is not mandatory if (!empty($treeInfo)), and can be moved in the if block?

{
$categories = [];
while (isset($this->categoryList[$categoryId])) {

This comment has been minimized.

@Quetzacoalt91

Quetzacoalt91 Apr 4, 2018

Member

I wonder how you get out of the while statement? I don't see any unset or = null in this method.

This comment has been minimized.

@jocel1

jocel1 Apr 4, 2018

Contributor

the $categoryId is updated at each loop to get the parent id value :)

@marionf

This comment has been minimized.

Contributor

marionf commented Apr 5, 2018

@jocel1

I have a lot of errors at the installation, maybe it's my local configuration

capture du 2018-04-05 08-52-39

@marionf marionf added the QA ✔️ label Apr 5, 2018

@wdimd

This comment has been minimized.

wdimd commented Apr 11, 2018

Thank you for the work, I appreciate, as I have over 14k categories and am unable to load products at all..

Now, I've merged this to my PrestaShop install, though it just throws me error 500s loading both categories and products, I'm unable to get any logs to the error_log as well. Any idea?

Edit, error editing the product (was only able to load product list in developer mode no idea why), attaches as productedit.txt
productload.txt

I can now load categories, but I cannot add or edit both categories and products.

Another issue editing products.
editingproducts.txt


Renaming to the old CategoryDataProvider.php allows me to edit products. Although, I've cleared my store, and am reimporting stuff again, and for now, it works, but we'll see once it allows me to fully import categories and products. I was also getting an undisclosed error while importing categories.

@mickaelandrieu

This comment has been minimized.

Contributor

mickaelandrieu commented Apr 11, 2018

@wdimd you may need to clear the cache?

@wdimd

This comment has been minimized.

wdimd commented Apr 11, 2018

I've done that many times, even entirely disabled it @mickaelandrieu . This got me the install completely messed up and will need to start from scratch.

Edit: Reimporting categories and products again, then will use the new files to see if it helps. I have been completely unable to add or edit both products and categories. I don't think this is an issue with the server, its load is barely 1, running php-fpm stack, on 8 core, 32GB RAM, RAID 10 NVMe.

Though I see it PrestaShop is having issues with 14k categories, and 7.4k products. Investigating, hopefully this fix will help me. Until now, it got nothing but issues but I could be doing something wrong.

@wdimd

This comment has been minimized.

wdimd commented Apr 11, 2018

Reinstalled it from scratch, imported 14k categories, didn't import the products yet, and I'm unable to create any products. Running the last PrestaShop from 9th April:

[11-Apr-2018 19:24:16 Europe/Lisbon] PHP Fatal error: Out of memory (allocated 548220928) (tried to allocate 20480 bytes) in /home/dir/public_html/vendor/symfony/symfony/src/Symfony/Component/Form/Extension/Core/Type/FormType.php on line 147

--
I've set no limit for memory usage, though this amount seems to be very exagerated, no?

Edit: So yeah, I've imported both categories and products now. Over 14k categories, roughly 7440 products. I am able to edit and add categories. But I am unable to add or edit products, it throws the memory error above, even though I did set the limit to -1, which is virtually no limit set. It's the first time we're getting Prestashop to production, as ink resellers, and this is damn crazy. Love prestashop and bought many plugins already, but I can't get it why it's unable to handle these products/categories. Maybe something in my end? I run php 7.1 btw.

@mickaelandrieu

This comment has been minimized.

Contributor

mickaelandrieu commented Apr 11, 2018

ping @alegout wdyt? do you reproduce the bug?

@jocel1

This comment has been minimized.

Contributor

jocel1 commented Apr 11, 2018

@wdimd I've just pushed a fix to avoid in your case to store the result of the new big category query into the prestashop query cache.
As for the memory usage, try to raise it, with such a high number of categories, it will have to create a lot of forms in the product page

@wdimd

This comment has been minimized.

wdimd commented Apr 11, 2018

@jocel1 I'll download the new code.
BTW, attached new information. Do note that I've set no limits for memory (it's -1), server has 32GB RAM. I even tried with 1024M, and it won't work to edit/add products.

Logs attached, I'll be trying your new code in a few.
prodlogs.txt

Edit, @jocel1 really weird stuff. I didn't notice any different, though, issue remains adding or editing a product. There must be a value, somewhere, dropping the memory limit, or defining it. I've loaded a php info to debug the ram and I've assigned 2048M, it tries to use:
[2018-04-11 22:09:10] request.CRITICAL: Uncaught PHP Exception Symfony\Component\Debug\Exception\OutOfMemoryException: "Error: Out of memory (allocated 548220928) (tried to allocate 20480 bytes)

Doesn't make sense. I've 2GB assigned.

@wdimd

This comment has been minimized.

wdimd commented Apr 11, 2018

@jocel1,

Update: I'm able to add a product, or edit an existing one. Issue was Apache was limited to 1024M, and it wasn't enough, I've bumped it to 4096M, which is a lot.
Is still takes forever.
I'd say this is a damn good challenge, getting Prestashop to run properly when you have 7k+ products and 14k+ categories, where the products are assigned to multiple categories. There must be a way to get this much faster than it is. Takes forever to edit/add product, at least 1 minute for the webpage to load itself. This is on a NVMe server with plenty of IOPs...

I just tested editing a product, or adding, if I load the categories to assign to (and they're split properly), it just blocks completely.

@jocel1

This comment has been minimized.

Contributor

jocel1 commented Apr 11, 2018

Avoid using -1, I saw several bugs, especially inside modules which was not correctly handling this value.
It's weird it takes forever in your case (because I have tested with 12k categories and a great amount of products), could you try to disable the prestashop cache ?

@wdimd

This comment has been minimized.

wdimd commented Apr 11, 2018

@jocel1
-1 has been disabled long time ago as soon as I seen the issue with Apache, I've set a fair limit, 4GB for PHP and 4GB for Apache, it takes forever to load. It won't let me do anything in the product edit/add, even though it stops loading, like if it was loaded, but it's not, as soon as I click in Prices, or expand categories, it won't work, browser will block, basically.
I've just disabled cache, it won't allow me to do it anyway, changing to quantity / price tab blocks it entirely.

Edit: Measured with Chrome dev tools, 45s to load, 16MB transferred, 76 requests. Seems too much? I got only a few plugins installed, the frontend is superfast, backend too, the issue seems to rely in the products. I'm more than willing to share the environment as a test, should it be, and I appreciate the super help. We've spent several months organizing both the categories / products database to import to Prestashop, and now this is getting us completely stuck. It's not that categories have 1000 thousand in a single category, they're all split in sub categories, you can preview the categories at https://www.goprint.pt/content/6-tinteiros-e-acessorios , the plugin in the top loads those, using ajax, it's super fast. I can't get it why it's so slow in the backend adding products.

@jocel1

This comment has been minimized.

Contributor

jocel1 commented Apr 11, 2018

are you in dev mode or in prod mode ? in defines.inc.php you should set
define('PS_MODE_DEV', false);
otherwise you'll get crap performances as well

@wdimd

This comment has been minimized.

wdimd commented Apr 11, 2018

@jocel1 , switched to dev mode, took 2 minutes to load adding a product, and still won't allow me to do anything once it loads.

I noticed a couple 404s as well, in the folder admin/_wdt. No dice, I'm hopeless.

@jocel1

This comment has been minimized.

Contributor

jocel1 commented Apr 11, 2018

which Prestashop version you are using BTW, the develop branch?

@wdimd

This comment has been minimized.

wdimd commented Apr 11, 2018

@jocel1 I'm using 1.7.3.1, released in 9th April, with your modifications.

@jocel1

This comment has been minimized.

Contributor

jocel1 commented Apr 11, 2018

could you give it a try directly with the develop branch? (and this patch of course)

@wdimd

This comment has been minimized.

wdimd commented Apr 11, 2018

@jocel1 will need to investigate if I can simply replace the files, never done it that way for Prestashop, though this was released 2 days ago, not sure if that'll make any difference.

@jocel1

This comment has been minimized.

Contributor

jocel1 commented Apr 11, 2018

there are a few other performances fixes in the upcoming 1.7.4.0 (that's why I suggest to test the develop branch) which should be interesting to test in your case, as well as #8904. Anyway, your use case is interesting, you can contact me directly at jocelyn.fournier@softizy.com by mail or hangout, perhaps I could take a look directly at your datas to verify whether it's a new performance issue.

@wdimd

This comment has been minimized.

wdimd commented Apr 11, 2018

@jocel1 I believe I've already emailed you! :)

Will email right now.

Yep indeed, in my case, it seems weird why it won't work properly, it's not like the database is huge, it's not, at all.

@wdimd

This comment has been minimized.

wdimd commented Apr 11, 2018

FYI, got Prestashop there, dev branch won't even import categories with this fix. I'll email you the info.
Accessing catalog, gives me an error as well.

- fix category import when force Id is not used
- make sure we only regenerate the NTree / clear the smarty cache at the end of the import
- disable the garbage collector on the product detail page because there could be a lot of objects on them and keeping the GC is counter-productive
@jocel1

This comment has been minimized.

Contributor

jocel1 commented Apr 12, 2018

impact of the gc_disable() on the product detail page:
https://blackfire.io/profiles/compare/4ee56e6f-9105-419b-acf1-c97e0b537a6a/graph

@jocel1 jocel1 merged commit b59fc42 into PrestaShop:develop Apr 13, 2018

2 of 3 checks passed

code-review/reviewable 3 files, 5 discussions left (alegout, mickaelandrieu, Quetzacoalt91)
Details
Codacy/PR Quality Review Good work! A positive pull request.
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@wdimd

This comment has been minimized.

wdimd commented Apr 13, 2018

I would like to thank @jocel1 for all the effort here, definitely, a contribution that was very much needed for PrestaShop. Will keep following up pulls that relate to high number of categories/products as it seems there's plenty of room for improvement here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment