Shopify / liquid

Liquid markup language. Safe, customer facing template language for flexible web apps.
https://shopify.github.io/liquid/
MIT License
11.11k stars 1.39k forks source link

Shopify Theme Performance #1074

Open sdn90 opened 5 years ago

sdn90 commented 5 years ago

Based on my experiences as a partner, stores with high TTFB times (above 2 seconds) are becoming increasingly common and I think how Liquid is being used has to do a big part in it. Compared to years ago, themes are getting much more complex and more apps are being used.

Site speed became a factor in Google search results in July 2018 and is also used grade the quality score of an ad. This is an issue that directly impacts sales and non-technical stakeholders can see it is a problem.

I know this is very hard to solve in practice since these problems are due to things out of Shopify's control and that there is huge variance in network quality.

App and theme developers tend to not put a huge emphasis on performance and in the end, all these things add up. To their defense, there are absolutely no resources on how to deal with this or measure it.

Some ideas:

Add performance section to Shopify Help Center

I have a feeling this may be tough for you guys to have documentation on this since things are probably changing on a day to day basis but I think even the smallest amount of information would be a lot of help.

Add performance measurement to Liquid

If a client asks me to solve high TTFB times, I basically have to review their entire theme by measuring the impact of removing and adding different parts of their theme which can potentially be time consuming to the point where a rewrite of the theme is actually feasible. This is an extremely bad look for all sides.

bakura10 commented 5 years ago

I am the developer of Maestrooo themes (like Prestige) and indeed, the expectations from merchants on themes lead us to have increasingly complex Liquid (however, contrary to your statement, I am personally taking performance as my main concern for all our themes, by making sure all the Liquid is as optimized as possible ; we also recently get rid of jQuery in all our themes).

I don't think this is particularly related to Liquid (but more to Shopify), but let me answer to some of your questions from what I have figured out. Once again, those are based on my personal findings and I agree that it would be awesome to have more technical details.

Caching

From what I can see, the cache is completely invalidated as soon as anything changes (eg. adding a new product to the cart, having a metafield changing...). I'm really not sure that Shopify has "smarter" caching where only parts of the page (some sections only for instance) are invalidated, while others are left untouched. But I may be wroong.

Metafields

I had to investigate this for a friend who used metafields extensively and I found some interesting things. He was using heavily variant metafields, on collection pages:

{% for product in collection.products %}
  {% for variant in product.variants %}
    {{ variant.metafields.foo.bar }}
  {% endfor %}
{% endfor %}

Each product having a large number of metafields, this constantly leaded to 2-3 seconds rendering. After a lot of tests, I figured out that if you group each metafields under the same namespace, this is way faster. Said otherwise:

{{ variant.metafields.foo.bar }}
{{ variant.metafields.foo.baz }}

is much faster than:

{{ variant.metafields.foo.bar }}
{{ variant.metafields.bim.baz }}

Once you've accessed a namespace, getting different values from same namespace is cheap, but getting a value from another is not.

Especially when the number of variants adds up. I'm pretty sure that internally when you get a value from a given namespace, Shopify does a kind of JOIN that get all the keys from the given namespace. So put it simply: put all your metafields in same namespace for best performance.

Lack of some features

Finally, other common issue is the lack of some variables that force us to do very complex Liquid, which involve a lot of looping that quickly adds up (especially on collection pages). For instance, what if you want to get all the variants associated with the option "Red"?

Ideally, we should be able to do that:

{% for option in product.options_with_values %}
  {% if option.name == 'Color' %}
    {% for option_value in option.values %}
       {% if option_value == 'Red' %}
          {% for variant in option_value.variants %}
          {% endfor %}
       {% endif %}
    {% endfor %}
  {% endif %}
{% endfor %}

This is often needed to build full-featured swatches on collection pages, but unfortunately there is no such thing as option_value.variants, so we have to do a lot of complex matching by iterating through every variants and do a lot of string comparaison.

There are other areas where new variables could be exposed to drastically increase performance.

shopmike commented 5 years ago

Hey guys,

Thanks for all the detailed information here. I can't comment on specifics for now but be assured some major work is being done to improve a lot of the exact concerns you have raised here.

Also, I'm personally working on a little update on the issue below that I should be able to share very shortly.

{% for option in product.options_with_values %}
  {% if option.name == 'Color' %}
    {% for option_value in option.values %}
shopmike commented 5 years ago

This isn't a full solution be we have just released a new options_by_name method for products so the example you have above can now be simplified. More to come on this front

Before

{% for option in product.options_with_values %}
  {% if option.name == 'Color' %}
    {% for option_value in option.values %}
       {% if option_value == 'Red' %}
          {% for variant in option_value.variants %}
          {% endfor %}
       {% endif %}
    {% endfor %}
  {% endif %}
{% endfor %}

After

{% for option_value in product.options_by_name.Color.values %}
    {% if option_value == 'Red' %}
        {% for variant in option_value.variants %}
        {% endfor %}
    {% endif %}
{% endfor %}
bakura10 commented 5 years ago

OH. This is wonderful !!! Thanks a lot!!!

jonathanmoore commented 5 years ago

@shopmike With options_by_name is the name case sensitive? We'll often see merchants that run into side-effects of not realizing some products name the option "color" and others are "Color". This new method will be extremely helpful, especially if the value could be normalized. In the old nested loop method, we will usually run the option name through handlize to prevent matching issues.

shopmike commented 5 years ago

@jonathanmoore thanks for the feedback. We are actually going to look to modify this a bit so that it will do a case-insensitive lookup. So the color and Color won't be an issue. Is there any other reason you handlize and don't just downcase

shopmike commented 5 years ago

Happy to announce that options_by_name is now case-insensitive. So you no longer need to check for color when in fact it is Color as the method will do it for you.

shopmike commented 5 years ago

https://help.shopify.com/en/themes/liquid/objects/product#product-options_by_name

jonathanmoore commented 5 years ago

@shopmike We started using handlize a while back as it seemed to work more reliably with non-Latin characters—Cyrillic, Kanji, etc. Although this was around the time when some newer shops had support for non-Latin handles and older stores didn't.

shopmike commented 5 years ago

Thanks for the details @jonathanmoore will see how this implementation goes. If we see a common use case it's not covering there is an option to improve it further.

bakura10 commented 5 years ago

@shopmike : it does not seem to be possible to get the variants from a product_option. Is it expected?

shopmike commented 5 years ago

@bakura10 there will be more updates rolling out and I expect that to be one coming soon.

bakura10 commented 4 years ago

Any news on when this will be rolled out @shopmike ? Would still love to be able to get the variants from product_option. I've epxerimented with the options_by_name but unfortunately in our use case this does not provide any improvement (neither in reducing code lines or improving performance).