PHPCompatibility / PHPCompatibilityWP

PHPCompatibility ruleset for WordPress projects
GNU Lesser General Public License v3.0
179 stars 11 forks source link

Fatal error: Allowed memory size #5

Closed JakeHenshall closed 6 years ago

JakeHenshall commented 6 years ago

Hi,

I've used Composer to add this into my WordPress Plugin.

When I run ./vendor/bin/phpcs -p . --standard=PHPCompatibilityWP this is the error that I get.

./vendor/bin/phpcs -p . --standard=PHPCompatibilityWP
.........W..........PHP Fatal error:  Allowed memory size of 134217728 bytes exhausted (tried to allocate 8388616 bytes) in /Users/jake/code/WP-Plugin/vendor/squizlabs/php_codesniffer/src/Tokenizers/CSS.php on line 247

Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 8388616 bytes) in /Users/jake/code/WP-Plugin/vendor/squizlabs/php_codesniffer/src/Tokenizers/CSS.php on line 247

Any help would be great.

Thanks.

jrfnl commented 6 years ago

Hi @JakeHenshall Thank you for reporting this.

PHP_CodeSniffer can not analyze minified CSS and JS files well. The error you mention above, is typically caused by these.

Now there are a couple of options for you:

  1. If you only use the PHPCompatibilityWP ruleset, like in your example command line command above - i.e. not combined with a code style ruleset, such as WordPress -, you can use --extensions=php to limit PHPCS to only analyze PHP files.

    See: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#specifying-valid-file-extensions

  2. If you do use several rulesets to analyze your code, you can exclude the minified files by:

    • either using a <exclude-pattern> directive in a custom ruleset;
    • or by passing the names of the files to exclude via the command-line using --ignore=....;
    • or by adding a // phpcs:ignoreFile annotation at the top of minified files you don't want analyzed.

    See: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Advanced-Usage#ignoring-files-and-folders

Does this help ?

jrfnl commented 6 years ago

@JakeHenshall Just checking in - did my previous reply help ?

jrfnl commented 6 years ago

Closing for lack of response.

Also, the Readme file for version 2.0.0 will contain a section mentioning the --extensions=php suggestion. See PR #6.

jeffmagill commented 6 years ago

@jrfnl I was having the same problem and your first option, --extensions=php, worked for me.

jrfnl commented 6 years ago

@jeffmagill Glad to hear it!

I've been trying to see if I could improve the minified file detection in PHPCS, but need test cases where this error still occurs while using PHPCS 3.x.

If the files on which the issue happened for you are public (and you are using PHPCS 3.x), could you point me to them, so I can have a look ?

jeffmagill commented 6 years ago

@jrfnl The files are not public but I would be willing to send it over to you if I can. The only problem is that this project has a large legacy codebase and I am not even familiar with half of it. I dont know where the error occurred. Can you recommend any way to identify the problematic file(s)? Perhaps putting a try/catch somewhere in PHPCompatibility and outputting an error with the file path?

Error message:

Fatal error: Allowed memory size of 134217728 bytes exhausted (tried to allocate 20480 bytes) in <path>/vendor/squizlabs/php_codesniffer/src/Tokenizers/Tokenizer.php on line 1516

Its interesting that @JakeHenshall's error was in Tokenizers/CSS.php whereas mine is in Tokenizers/Tokenizer.php.

jrfnl commented 6 years ago

@jeffmagill You can determine in which file(s) the issue occurs by adding the -v option to the PHPCS command. That should show the files in the order PHPCS is processing them.

Could you DM me on Twitter (@jrf_nl), so I can give you my email address to send the file(s) to ?

jeffmagill commented 6 years ago

In order to keep this conversation public, I'll repeat here that I determined that it was something in TinyMCE 3.2.5 that was throwing the error.

jrfnl commented 6 years ago

I've just spend quite some of hours analysing this and trying to find some sort of solution. I'm documenting here what I've done for future reference.

Additional information @jeffmagill provided me with: the memory_limit of the install used was 128M.

Now, PHPCS 3.x already contains quite a good detection of CSS/JS minified files and skips them automatically, so that is no longer the problem.

With my own install set to the same memory_limit, I was able to reproduce the issue with the tiny_mce_src.js file - which is not a minified file. I went on to exclude the fie and see if the run would finish in that case and ran into a significant number of other - PHP - problem files. All these files were large files from a PHP perspective, i.e 100K or more, though later in the run, even some 50K sized files were giving problems.

Next I did an analysis of the memory usage during a run. In short, with each analysed file, the memory used by PHPCS increased. In practice, this means that a large file might be analysed without problem if it's early in the queue, while if it is at the end of the queue, this may no longer be possible.

Based on that, I did an analysis of the garbage collection using xdebug.gc_stats_enable and found that garbage collection was never triggered, but that there were also no cycles collected. A test with turning garbage collection on/off and manually triggering the cleaning also had no effect whatsover, so there's no solution in that direction.

An analysis with BlackFire shows that most memory is used by the tokenization process (no surprise there) which is largely based on PHP native token_get_all() function with enhancements, so there is not much room for optimization there.

All in all, this really is a memory issue. The files I've found were are not minified, just huge and the memory needed to Tokenize those causes the memory overflow.

I initially thought I may be able to add some sort of short-circuit to the PHPCS tokenizer by doing a rough guess based on memory_limit and the file size to see if we're likely to hit this issue and if so, skip the file, however as the memory available depends on where in the run we are, this will be a lot more complicated to "guess" right that I anticipated.

Further analysis with BlackFire to see if there are micro-optimizations which can be made aimed at lowering the memory usage of PHPCS is recommended. A first PR as a result of that has already been submitted to PHPCS: https://github.com/squizlabs/PHP_CodeSniffer/pull/2273, but an analysis of the sniffs in PHPCompatibility itself could also be beneficial.

To sum up: for now, there isn't really a easy solution in the form of a change to PHPCS itself.

For anyone who comes across this issue and is looking for a way around issues with running out of memory when running PHPCS with PHPCompatibility, I would recommend the following solutions:

  1. Increase the memory for PHPCS.
    • This can be done for individual runs from the command-line using -d memory_limit=256M (set to whatever new memory limit you want)
    • You can also set this in a custom ruleset using <ini name="memory_limit" value="256M"/>.
    • Or if you are happy to increase the memory for PHP in general, you can change it in the php.ini file.
  2. Limit the files for PHPCompatibility to your PHP files.
    • If you don't combine the PHPCS run for PHPCompatibility with any other standard, you can use --extensions=php on the command line or <arg name="extensions" value="php"/> in your custom ruleset. If you use other extensions for PHP files, you can pass those too, like so: <arg name="extensions" value="php,inc/php"/>
    • If you do combine the PHPCS run with another standard and use PHPCS 3.x, my current recommendation would be to always use a custom ruleset and to add PHPCompatibility to it like so:
      <rule ref="PHPCompatibility">
          <include-pattern>*\.php$</include-pattern><!-- Repeat for any other extensions you use for PHP files. -->
      </rule>

      This will limit PHPCompatibility to PHP files only, while any other sniffs will still be able to analyse JS/CSS files as well.