OFFLINE-GmbH / oc-mall-plugin

:convenience_store: E-commerce solution for October CMS
https://offline-gmbh.github.io/oc-mall-plugin
MIT License
169 stars 114 forks source link

[Optimization] Reduce queries to display a category #45

Closed pvullioud closed 6 years ago

pvullioud commented 6 years ago

I have a category with 180 main products with variants. mall is running 326 queries in 9s Number of queries should be reduce to improve the speed

tobias-kuendig commented 6 years ago

Is your repo up to date? We did some optimizations in this commit:

https://github.com/OFFLINE-GmbH/oc-mall-plugin/commit/b9849d8d13900c40cc1e844fe5f19d7bbb1988c8

We'll definitely have to optimize it further. A caching layer for the whole shop is planned as well once the basic functionality is done.

pvullioud commented 6 years ago

It's with the last branch. caching will help but we should find a way to reduce the number of queries. a category could have 1000 articles and add some index

tobias-kuendig commented 6 years ago

Yes, that's for sure. I guess it's a N+1 problem, I'll see if I can find the relevant query.

pvullioud commented 6 years ago

The variant is calling the parent each time https://github.com/OFFLINE-GmbH/oc-mall-plugin/blob/develop/models/Variant.php#L191

tobias-kuendig commented 6 years ago

Yes, this is not optimal. And IIRC all products of the category are currently loaded to generate the possible filter values.

I'll definitely have to refactor this so only the X products that are displayed on the current page are loaded. The filters have to be generated by other means.

Fl0Cri commented 6 years ago

Would it be possible to solve the problem by removing the $parent property of the Variant model and use the product relation instead?

Theorically it should not impact performance because the query to the relation should be cached by eloquent. In addition it would be possible to eager load all products for a collection of variants, like here https://github.com/OFFLINE-GmbH/oc-mall-plugin/blob/develop/models/Category.php#L377 by adding variants.product.

I've tried it but I get the following strange error when trying to query the product relation from the getAttribute() method of a Variant (at the line quoted by @pvullioud ):

Undefined property: OFFLINE\Mall\Models\Variant::$product_id

I'm probably missing something but if we manage to make it work (and don't fall in a potential infinite getAttribute loop), there's a chance to solve the problem without having to refactor the filters.

tobias-kuendig commented 6 years ago

This is a massive oversight on my part, thanks for bringing this up. Of course it's not necessary to load the parent's data if we have the relation set up.

What's the query count like after applying this fix?

https://github.com/OFFLINE-GmbH/oc-mall-plugin/commit/f909d840c5ef0329088e443fa8043a840ea7171f

pvullioud commented 6 years ago

Well done for queries! before fix and cache: 326 queries

Now

123 queries without cache (first load) 31 queries with cache (2nd and next page load) : category cache and your fix

loading speed is quite slow 4.2s for category and filter (6s without cache, first load) 1.4s for category only I'll open a other issue for the speed