Skip to content
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

hasSales queries are taking a really long time to execute #758

Closed
echantigny opened this issue Feb 27, 2019 · 10 comments
Closed

hasSales queries are taking a really long time to execute #758

echantigny opened this issue Feb 27, 2019 · 10 comments
Assignees
Labels

Comments

@echantigny
Copy link
Contributor

echantigny commented Feb 27, 2019

Description

I'm trying to build a "On Sale" page and for some reason, the queries to get those products are running extremely slow, now matter how I try to run them.

It's so slow that the Debug Toolbar isn't even showing on those pages for some reason so I can't even give you proper numbers. From the google Network waterfall:

Exact same display page, just using a different query
All games (limit 24): 614ms
Has Sales (limit 24): 20s

This is all on a publicly accessible server, but I'll need to give you credentials to get in. I'll send those to you by email if you require them to replicate.

Worth noting, I have 4757 products, with 1 variant each. I'll never have more than 1 variant.

Steps to reproduce

These are the methods I'm trying to use:

craft.products.hasVariant({hasSales: true}).limit(24).all()
craft.products.hasVariant({hasSales: true}).one()
craft.variants.hasSales(true).one()

Additional info

  • Craft version: Craft Pro 3.1.14
  • Craft commerce: 2.1.0.2
  • PHP version: 7.2.8
  • Database driver & version: MySQL 5.7.22
  • Plugins & versions:
@echantigny
Copy link
Contributor Author

echantigny commented Feb 27, 2019

Documenting this a bit more:

{% set variants = craft.variants.limit(1000) %}

<ul>
    {% for variant in variants.all() %}
            <li>{{ variant.sku }}</li>
    {% endfor %}
</ul>

Results in 15 queries, 28ms
Time 455ms
Memory 11.9MB

{% set variants = craft.variants.limit(1000) %}

<ul>
    {% for variant in variants.all() %}
        {% if variant.onSale %}
            <li>{{ variant.sku }}</li>
        {% endif %}
    {% endfor %}
</ul>

Results in 3007 DB queries, 2,021ms
Time 11,435
Memory 43.9MB

Using variant.salePrice gives virtually identical results.

I really think this needs to be looked into. There's currently no efficient way to show a list of on sale products. This is the only area that the system is slowing down on. The rest works like a charm.

@lukeholder
Copy link
Member

Hi @echantigny

Thanks for bringing this to our attention. We discussed it on a call as a team last night.

Due to the current architecture of sales at the moment, there is not a simple way to optimize in a significant way the use of the hasSales variant query param.

Having said that I am wanting to get a copy of your DB for me to reproduce and take a look at the queries generated. I have already found one "low hanging fruit" code fix that should reduce your 3007 DB queries by about 500-1000 depending on the types of sales you have set up.

We hope to fully address the performance of hasSales when we tackle our new condition builder in 3.x

Let me know if you can send through your database?

@lukeholder
Copy link
Member

Using this code:

<h1>On Sale</h1>
{% set variants = craft.variants.hasSales(true).limit(50).all %}
{% for variant in variants %}
{{ variant.id }}:
{{ variant.salePrice }}
<br>
{% endfor %}

With Eric's DB (4000+ variants and a number of category based sales)

Which was timing out before, is now returning in 2-3 seconds with 127 queries

This is with

  1. sub-query cloning, so that the sub-query to match the purchasable to the sale has all the same conditions as the main query
  2. category-purchasable relation checking and query optimisations
  3. And the big one, memoizing the sale-purchasable match within the same request. Calling the hasSales query param AND calling salePrice on a variant in the same request was doing the match twice

@echantigny
Copy link
Contributor Author

echantigny commented Mar 6, 2019

@lukeholder For some reason, it looks like it is not working for me at all, with new errors too.

This is all in a new database that has all the same products (plus newer ones) and I created the Promotion (exact same as before) after I installed the dev-develop branch (107dd69).

Using this gives me an Undefined index: id
craft.products.hasVariant({hasSales: true}).limit(48).all

yii\base\ErrorException: Undefined index: id in C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\elements\db\ElementQuery.php:2233
Stack trace:
#0 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\elements\db\ElementQuery.php(2197): craft\commerce\elements\db\VariantQuery->_createElement()
#1 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\elements\db\ElementQuery.php(1171): craft\commerce\elements\db\VariantQuery->_createElements()
#2 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\db\Query.php(238): craft\commerce\elements\db\VariantQuery->populate()
#3 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\db\Query.php(161): craft\commerce\elements\db\VariantQuery->all()
#4 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\elements\db\ElementQuery.php(1200): craft\commerce\elements\db\VariantQuery->all()
#5 C:\wamp64\www\Great-Boardgames\vendor\craftcms\commerce\src\elements\db\VariantQuery.php(420): craft\commerce\elements\db\VariantQuery->all()
#6 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\elements\db\ElementQuery.php(1050): craft\commerce\elements\db\VariantQuery->beforePrepare()
#7 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\db\QueryBuilder.php(227): craft\commerce\elements\db\VariantQuery->prepare()
#8 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\db\Query.php(146): craft\db\mysql\QueryBuilder->build()
#9 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\db\Query.php(307): craft\commerce\elements\db\VariantQuery->createCommand()
#10 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\db\Query.php(211): craft\commerce\elements\db\VariantQuery->column()
#11 C:\wamp64\www\Great-Boardgames\vendor\craftcms\commerce\src\elements\db\ProductQuery.php(673): craft\commerce\elements\db\VariantQuery->column()
#12 C:\wamp64\www\Great-Boardgames\vendor\craftcms\commerce\src\elements\db\ProductQuery.php(579): craft\commerce\elements\db\ProductQuery->_applyHasVariantParam()
#13 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\elements\db\ElementQuery.php(1050): craft\commerce\elements\db\ProductQuery->beforePrepare()
#14 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\db\QueryBuilder.php(227): craft\commerce\elements\db\ProductQuery->prepare()
#15 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\db\Query.php(146): craft\db\mysql\QueryBuilder->build()
#16 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\db\Query.php(456): craft\commerce\elements\db\ProductQuery->createCommand()
#17 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\db\Query.php(268): craft\commerce\elements\db\ProductQuery->queryScalar()
#18 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\db\Query.php(347): craft\commerce\elements\db\ProductQuery->queryScalar()
#19 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\elements\db\ElementQuery.php(1184): craft\commerce\elements\db\ProductQuery->count()
#20 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\helpers\Template.php(95): craft\commerce\elements\db\ProductQuery->count()
#21 C:\wamp64\www\Great-Boardgames\storage\runtime\compiled_templates\25\25e1a44d9dd1ee9a534d5f31fd3b4df694d7850dd8bb94d943295ce09b937b08.php(88): craft\helpers\Template::paginateCriteria()
#22 C:\wamp64\www\Great-Boardgames\vendor\twig\twig\lib\Twig\Template.php(189): __TwigTemplate_a8a5bc7d28032354914bc2a6392e50d6ba067abf251093cc083b1587357c0bff->block_body()
#23 C:\wamp64\www\Great-Boardgames\storage\runtime\compiled_templates\29\29d56089492016f4fb35a7f8a2accbdeffe6e7dd4ab3194dbfaec5ad98b1e45c.php(158): __TwigTemplate_e27143a7b672e7e0c412b56d20ec46914c3a0ccd5fe163b7bc570a4d1a388439->displayBlock()
#24 C:\wamp64\www\Great-Boardgames\vendor\twig\twig\lib\Twig\Template.php(386): __TwigTemplate_e27143a7b672e7e0c412b56d20ec46914c3a0ccd5fe163b7bc570a4d1a388439->doDisplay()
#25 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\web\twig\Template.php(49): __TwigTemplate_e27143a7b672e7e0c412b56d20ec46914c3a0ccd5fe163b7bc570a4d1a388439->displayWithErrorHandling()
#26 C:\wamp64\www\Great-Boardgames\vendor\twig\twig\lib\Twig\Template.php(363): __TwigTemplate_e27143a7b672e7e0c412b56d20ec46914c3a0ccd5fe163b7bc570a4d1a388439->displayWithErrorHandling()
#27 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\web\twig\Template.php(31): __TwigTemplate_e27143a7b672e7e0c412b56d20ec46914c3a0ccd5fe163b7bc570a4d1a388439->display()
#28 C:\wamp64\www\Great-Boardgames\storage\runtime\compiled_templates\25\25e1a44d9dd1ee9a534d5f31fd3b4df694d7850dd8bb94d943295ce09b937b08.php(62): __TwigTemplate_e27143a7b672e7e0c412b56d20ec46914c3a0ccd5fe163b7bc570a4d1a388439->display()
#29 C:\wamp64\www\Great-Boardgames\vendor\twig\twig\lib\Twig\Template.php(386): __TwigTemplate_a8a5bc7d28032354914bc2a6392e50d6ba067abf251093cc083b1587357c0bff->doDisplay()
#30 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\web\twig\Template.php(49): __TwigTemplate_a8a5bc7d28032354914bc2a6392e50d6ba067abf251093cc083b1587357c0bff->displayWithErrorHandling()
#31 C:\wamp64\www\Great-Boardgames\vendor\twig\twig\lib\Twig\Template.php(363): __TwigTemplate_a8a5bc7d28032354914bc2a6392e50d6ba067abf251093cc083b1587357c0bff->displayWithErrorHandling()
#32 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\web\twig\Template.php(31): __TwigTemplate_a8a5bc7d28032354914bc2a6392e50d6ba067abf251093cc083b1587357c0bff->display()
#33 C:\wamp64\www\Great-Boardgames\storage\runtime\compiled_templates\65\6589f0bac5fdc4e8b655f16a4e5a508773dcc9948a6df48843f0829261a57baa.php(32): __TwigTemplate_a8a5bc7d28032354914bc2a6392e50d6ba067abf251093cc083b1587357c0bff->display()
#34 C:\wamp64\www\Great-Boardgames\vendor\twig\twig\lib\Twig\Template.php(386): __TwigTemplate_81a6a2bad5646563ee1819375e211501f169d82ab474c1986e435b20c3200a4c->doDisplay()
#35 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\web\twig\Template.php(49): __TwigTemplate_81a6a2bad5646563ee1819375e211501f169d82ab474c1986e435b20c3200a4c->displayWithErrorHandling()
#36 C:\wamp64\www\Great-Boardgames\vendor\twig\twig\lib\Twig\Template.php(363): __TwigTemplate_81a6a2bad5646563ee1819375e211501f169d82ab474c1986e435b20c3200a4c->displayWithErrorHandling()
#37 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\web\twig\Template.php(31): __TwigTemplate_81a6a2bad5646563ee1819375e211501f169d82ab474c1986e435b20c3200a4c->display()
#38 C:\wamp64\www\Great-Boardgames\vendor\twig\twig\lib\Twig\Template.php(371): __TwigTemplate_81a6a2bad5646563ee1819375e211501f169d82ab474c1986e435b20c3200a4c->display()
#39 C:\wamp64\www\Great-Boardgames\vendor\twig\twig\lib\Twig\Environment.php(289): __TwigTemplate_81a6a2bad5646563ee1819375e211501f169d82ab474c1986e435b20c3200a4c->render()
#40 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\web\View.php(337): craft\web\twig\Environment->render()
#41 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\web\View.php(384): craft\web\View->renderTemplate()
#42 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\web\Controller.php(161): craft\web\View->renderPageTemplate()
#43 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\controllers\TemplatesController.php(78): craft\controllers\TemplatesController->renderTemplate()
#44 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\base\InlineAction.php(57): craft\controllers\TemplatesController->actionRender()
#45 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\base\InlineAction.php(57): ::call_user_func_array:{C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\base\InlineAction.php:57}()
#46 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\base\Controller.php(157): yii\base\InlineAction->runWithParams()
#47 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\web\Controller.php(109): craft\controllers\TemplatesController->runAction()
#48 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\base\Module.php(528): craft\controllers\TemplatesController->runAction()
#49 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\web\Application.php(297): craft\web\Application->runAction()
#50 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\web\Application.php(103): craft\web\Application->runAction()
#51 C:\wamp64\www\Great-Boardgames\vendor\craftcms\cms\src\web\Application.php(286): craft\web\Application->handleRequest()
#52 C:\wamp64\www\Great-Boardgames\vendor\yiisoft\yii2\base\Application.php(386): craft\web\Application->handleRequest()
#53 C:\wamp64\www\Great-Boardgames\public_html\index.php(21): craft\web\Application->run()
#54 {main}

@echantigny
Copy link
Contributor Author

Because this solution is doing the limit on the product query, and then looking at each to see if they are on sale or not, it is never returning the true set of onSale products.

My temporary solution consists of looping through all the sales and getting the product ids associated with them. I have not looked into validating that the sales are still active yet, but this returns the correct set of products within an acceptable timeframe.

This solution does not take the User Group sales into account.

{% set sales = craft.commerce.sales.getAllSales() %}
{% set categories = [] %}
{% set variantIds = [] %}

{% for sale in sales %}
    {% set categories = categories|merge(sale.categoryIds) %}
    {% set variantIds = variantIds|merge(sale.purchasableIds) %}
{% endfor %}

{% set productIds = [] %}
{% for product in craft.products.relatedTo(categories).all() %}
    {% set productIds = productIds|merge([product.id]) %}
{% endfor %}

{% set variants = craft.variants.id(variantIds).all() %}
{% for variant in variants %}
    {% set productIds = productIds|merge([variant.productId]) %}
{% endfor %}


{% set products = craft.products.id(productIds).with("variants").all() %}
<ol>
    {% for product in products %}
        {% set variant = product.defaultVariant %}
        <li>{{ variant.title }}:{{ variant.price }} - {{ variant.salePrice }}</li>
    {% endfor %}
</ol>

@lukeholder lukeholder reopened this Mar 22, 2019
@lukeholder
Copy link
Member

Re-opening as the issue is not solved. It is a performance issue with a large number of variants.

@echantigny
Copy link
Contributor Author

@lukeholder Please keep in mind that the solution you currently implemented in the system is wrong. Because the hasSales is applied to the products after the query limit is applied, it never returns the correct amount of products. That part should be reverted back so people don't run into that issue.

@lukeholder
Copy link
Member

@echantigny should already be fixed in 9311cb2 are you on the latest dev-develop?

@echantigny
Copy link
Contributor Author

@lukeholder I'm not. I'm finalizing my site for production so I'm staying on the Released product for now. I can't really test that anyway as it crashes my site with the 5000+ products.

@lukeholder lukeholder added the bug label Mar 27, 2019
@lukeholder
Copy link
Member

Closing this, and adding a comment to the condition builder ticket. #80

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants