Avoid wasting Opcache memory with Paginator queries

[mpdude]

This PR prevents the Paginator from causing OpCache "wasted memory" to increase on every request when used with Symfony's PhpFilesAdapter as the cache implementation for the query cache.

Depending on configured thresholds, wasted memory this will either cause periodic opcache restarts or running out of memory and not being able to cache additional scripts (Details).

Fixes #9917, closes #10095.

Problem analysis

There is a long story (#7820, #7821, #7837, #7865) behind how the Paginator can take care of DBAL type conversions when creating the pagination query. This conversion has to transform identifier values before they will be used as a query parameter, so it has to happen every time the Paginator is used.

For reasons, this conversion happens inside WhereInWalker. Tree walkers like this are used only during the DQL parsing/AST processing steps. Having a DQL query in the query cache short-cuts this step by fetching the parsing/processing result from the cache.

So, to make sure the conversion happens also with the query cache being enabled, this line

orm/lib/Doctrine/ORM/Tools/Pagination/Paginator.php

Line 165 in 1753d03

$whereInQuery->expireQueryCache();

was added in #7837. It causes \Doctrine\ORM\Query::parse() to re-parse the query every time, but will also put the result into the query cache afterwards.

At this point, the setup described in #9917 – which, to my knowledge, is the default in Symfony + DoctrineBundle projects – will ultimately bring us to this code:

https://github.com/symfony/symfony/blob/4b3391725f2fc4a072e776974f00a992cbc70515/src/Symfony/Component/Cache/Adapter/PhpFilesAdapter.php#L248-L249

When writing a cache item with an already existing key, the driver has to make sure the opcache will honor the changed PHP file. This is what causes wasted memory to increase.

Suggested solution

Instead of using \Doctrine\ORM\Query::expireQueryCache(), which will force \Doctrine\ORM\Query::parse() to parse the query again before putting it into the cache, use \Doctrine\ORM\Query::useQueryCache(false). The subtle difference is the latter will not place the processed query in the cache in the first place.

A test case is added to check that repeated use of the paginator does not call the cache to update existing keys. That should suffice to make sure we're not running into the issue, while at the same time not complicating tests by using the PhpFilesAdapter directly.

Note that in order to observe the described issue in tests, you will need to use the PhpFilesDriver and also make sure that OpCache is enabled and also activated for php-cli (which is running the unit tests).

Performance remark

This particular subquery generated/used by the Paginator is not put into the query cache. The DQL parsing/to-SQL conversion has to happen every time the Paginator is used.

This, however, was already the case before this PR. In other words, this PR only changes that we do not store/update the cached result every time, but instead completely omit caching the query.

[mpdude]

@icedevelopment since you reported #9917, I'd like to ask you to try this one-line change and report if it fixes the issue for you.

[derrabus]

Nice catch @mpdude!

[mpdude]

Thank you @derrabus for the swift review and merge!

[icedevelopment]

@icedevelopment since you reported #9917, I'd like to ask you to try this one-line change and report if it fixes the issue for you.

Just tested this and it does fix it! 🎉
No more wasted OPCache memory increasing on each request when using a paginator.
Thank you very much for your work @mpdude!

@derrabus any idea of the approximate release date of 2.14.2?

[derrabus]

No. Use "doctrine/orm": "~2.14.2@dev" in your composer.json if you don't want to wait.