Add gc_mem_caches() call for PHP7 after HttpKernel::stripComments() #17384
Conversation
…anager will not release small buckets to OS automatically in cases exposed by token_get_all()
|
Just asking: is it really needed to optimize this? If I'm not wrong, this method is only called once during the application life, right? |
|
We're running on cloud-based hosting platforms under memory limitations (Platform.sh). When memory is exceeded we're into swap and the cache clearing process goes from seconds to minutes for the initial deployment, which really slows our development workflow and also causes holding page delays. So yes it will definitely help our use-case, I should think there are others in a similar position. |
|
Understood. It makes sense. Thanks! |
| // reclaim memory for php7, as memory manager will not release after token_get_all() | ||
| if (function_exists('gc_mem_caches')) { | ||
| gc_mem_caches(); | ||
| } |
There was a problem hiding this comment.
If we call gc_mem_caches() because of the usage of token_get_all()I'd we should move these lines to stripComments().
|
absolutely, thank you! |
|
I'm closing this PR then, thank you for your detailed investigation and fix! |
… token_get_all (nicolas-grekas, peteward) This PR was merged into the 2.3 branch. Discussion ---------- Fix performance (PHP5) and memory (PHP7) issues when using token_get_all | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16868 | License | MIT | Doc PR | - Although it's not the case anymore on PHP 7, on PHP 5, `is_array` checks are much slower than `isset` checks. Also from @peteward in #17384: > New PHP7 memory manager will not release small buckets to OS automatically in cases exposed by `token_get_all()`. This function call addition specifically for PHP7 will reclaim this memory to keep the footprint down of long processe > See above ticket and suggested actions by PHP internals team for long-running tasks (https://bugs.php.net/70098) - I think `cache:clear/warmup` on a heavy app justifies this. > We're running on cloud-based hosting platforms under memory limitations (Platform.sh). When memory is exceeded we're into swap and the cache clearing process goes from seconds to minutes for the initial deployment, which really slows our development workflow and also causes holding page delays. Commits ------- e555aad Add gc_mem_caches() call for PHP7 after itoken_get_all() as new memory manager will not release small buckets to OS automatically d1f72d8 Fix perf and mem issue when using token_get_all
New PHP7 memory manager will not release small buckets to OS automatically in cases exposed by
token_get_all(). This function call addition specifically for PHP7 will reclaim this memory to keep the footprint down of long processeSee above ticket and suggested actions by PHP internals team for long-running tasks (https://bugs.php.net/bug.php?id=70098) - I think
cache:clear/warmupon a heavy app justifies this.