mirror of
https://github.com/doctrine/orm.git
synced 2026-03-24 06:52:09 +01:00
[PR #10434] Avoid wasting Opcache memory with Paginator queries #12359
Reference in New Issue
Block a user
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Original Pull Request: https://github.com/doctrine/orm/pull/10434
State: closed
Merged: Yes
This PR prevents the Paginator from causing OpCache "wasted memory" to increase on every request when used with Symfony's
PhpFilesAdapteras 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
1753d03500/lib/Doctrine/ORM/Tools/Pagination/Paginator.php (L165)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:
4b3391725f/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
PhpFilesAdapterdirectly.Note that in order to observe the described issue in tests, you will need to use the
PhpFilesDriverand also make sure that OpCache is enabled and also activated forphp-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.