[5.x]: Using the Twig default filter on an element triggers a lot of queries

[mmikkel]

What happened?

Description

After upgrading an older site to Craft 5, I noticed that a lot of pages were generating significantly more queries, and had a much higher TTFB, than on Craft 4.

The culprit turned out to be Twig statements like this:

{% if entry|default %}
    ...
{% endif %}

As it turns out, using the |default filter on an element in Craft 5 generates a lot of queries! Importantly, this is a change in behaviour from prior Craft versions, where doing the same thing would generate no extra queries (I tested this on Craft 4.17.13.1).

Something like entry|default is not really code I would write today, but it's certainly something we did a lot back in the day, before the null coalescing operator was a thing in Twig. A fair amount of those older sites are still alive and kicking and now on Craft 5, so this is a bit of an unfortunate performance gotcha.

The fix (in most situations) will be to use the null coalescing operator instead of |default - but actually identifying and fixing all cases where the default filter has been used on element variables won't be fun, or particularly easy 😓 – so it'd be great if it was possible to prevent those queries from being generated on Craft's end.

Steps to reproduce

Expected behavior

Actual behavior

Craft CMS version

5.9.20

PHP version

No response

Operating system and version

No response

Database type and version

No response

Image driver and version

No response

Installed plugins and versions

[linear]

CMS-1966

[i-just]

Hi @mmikkel, thanks for getting in touch!

I did some digging around this. You mentioned that using the default filter in v4 wouldn’t generate any extra queries - do you mean compared to rendering a page with an entry without using the default filter or something else?

Here’s the summary of what I found so far:

v4 (4.17.3.1) uses twig ~3.19.0
v5 (5.9.20) uses twig ~3.21.1
There’s no notable difference between those versions in terms of the default filter

Twig’s default filter is using this method: https://github.com/twigphp/Twig/blob/v3.21.1/src/Extension/CoreExtension.php#L900-L907.
It relies on the `testEmpty() method: https://github.com/twigphp/Twig/blob/v3.21.1/src/Extension/CoreExtension.php#L1414-L1429, which also hasn’t changed recently.

testEmpty is called on an Element which is a Traversable instance, which means Element’s getIterator() is called. That method also hasn’t really changed between versions 4 and 5.

v4 (latest, 4.17.3.1):

• using |default on a result of a query for a non-existent element (on null) => 24
• using ?? 'default' on a result of a query for an existing element or just querying for a single entry => 24
• using |default on a result of a query for existing element => 31

v5 (latest, 5.9.20):

• using |default on a result of a query for a non-existent element (on null) => 35
• using ?? 'default' on a result of a query for an existing element or just querying for a single entry => 35
• using |default on a result of a query for existing element => 42

(testEmpty() method is also used by the empty test (is empty), and calling it on a result of a query for an existing element also produces 31 (v4) vs 42 (v5) queries in the setup that I have.)

We’ll discuss this further internally, but it doesn’t quite look like the default filter behaves differently in v4 vs v5.

[mmikkel]

Hi @i-just – thanks a lot for looking into this!

We’ll discuss this further internally, but it doesn’t quite look like the default filter behaves differently in v4 vs v5.

It appears you're right, and my initial claim that the |default filter doesn't generate any additional queries on Craft 4, is wrong.

I spun up a fresh Craft 4 project (v. 4.17.13.1), created a single entry with just a title field, and tested again with this code:

{% set entry = craft.entries.section('news').one() %}
{% if entry|default %}
    {{ entry.title }}
{% endif %}

The query count for that page load was 33. If the {% if entry|default %} conditional is removed, the query count drops to 26.

Then, I upgraded the site to Craft 5. Now, the code above generates 44 queries with the conditional, and 36 queries without it.

So yeah, on both Craft versions entry|default does generate additional queries. And, at least for a very simple entry (no custom fields), the number of queries generated is very similar (7 on Craft 4 vs. 8 on Craft 5).

It's weird that on the "real" project where I encountered this, the difference was much bigger (0 extra queries on Craft 4, 100+ extra queries on Craft 5). But I guess that could be due to any number of reasons.

So... I guess this issue boils down to "it'd be nice if doing entry|default didn't generate any queries" – because IMO ideally that operation should ideally work similarly to entry ?? null, and not traverse anything. I realise this would probably be difficult to achieve without Craft overriding the core |default Twig filter, though.

[brandonkelly]

I just PR’d a change to Twig that would help with this: twigphp/Twig#4793

[GromNaN]

The fix (in most situations) will be to use the null coalescing operator instead of |default - but actually identifying and fixing all cases where the default filter has been used on element variables won't be fun, or particularly easy 😓 – so it'd be great if it was possible to prevent those queries from being generated on Craft's end.

You can override the default filter in CraftCMS to trigger de message when you detect such situation where it should be replace by ??.

twigcs is a tool you can investigate to apply changes automatically. You may have to create a custom rule.

[mmikkel]

You can override the default filter in CraftCMS to trigger de message when you detect such situation where it should be replace by ??.

twigcs is a tool you can investigate to apply changes automatically. You may have to create a custom rule.

Both decent options! But, using and maintaining a custom |default core filter override in a module or private plugin, across something like 100+ projects seems to me like an unnecessary risk long-term 😌

As for Twigcs, I guess it could help identify cases where |default is used on an element variable, but again; running that on 100+ projects would be a lot of work (we don't use Twigcs currently on any of them). Also... the element variable could theoretically be called anything (like newsArticle|default or the like), which I'm guessing would make it harder to make a generic rule? Haven't looked at how Twigcs works though.

Anyways, Brandon's PR would be a great way to solve this upstream, so here's hoping that ends up being merged 😊

[brandonkelly]

Solved this for 4.18 and 5.10 by overriding the |default filter and is empty test, to treat all yii\base\Model instances (including elements) as not empty. (b0e26b9)