You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
support linting multiple files at once using php -l #10024
would it be possible to support passing multiple files to php -l, e.g. php -l file1.php file2.php file3.php?
my request is motivated by the fact the current linting tools are slow because of the overhead involved to lint every single file using a single process, so e.g. proc_open dominates the whole process.
see this example blackfire profile, where I lint a project with 2890 files which takes 1min 14secs:
tested with on windows11 - but we see also unnecessary slowdown within github action jobs (not that big as in windows)
PHP 8.1.2 (cli) (built: Jan 19 2022 10:13:52) (NTS Visual C++ 2019 x64)
Copyright (c) The PHP Group
Zend Engine v4.1.2, Copyright (c) Zend Technologies
if we could e.g. lint 2 or 3 files at a time we could easily cut this processing time in half or even more.
Btw: I find it pretty suprising that it takes over 1 minute on a 10 core cpu windows laptop to lint the same project, which takes 15 seconds on a 2 core github action job (with 10 processes in parallel)
I find it pretty suprising that it takes over 1 minute on a 10 core cpu windows laptop to lint the same project, which takes 15 seconds on a 2 core github action job (with 10 processes in parallel)
I assume this is due to file system performance (which is slow on Windows, and multiple cores won't help with that).
I am not that much into the details regarding windows internals.
maybe its worth to profile how expensive process creation on windows is and whether there are some low hanging fruits to get.
maybe its worth to profile how expensive process creation on windows is
Oh, right, creating new PHP processes is also very slow, especially if there are many DLLs to load (part of that problem is ASLR). We have been able to considerably speed up our test runs in CI by only loading the extensions which are really required for a single test case. Anyhow, this issue would be solved for the linting case, if we support multiple files to be linted at once.
do you agree we would just allow to support a fixed number of additional arguments, or do you think we should support "n" arguments (no matter how much given)?
zend_printf("No syntax errors detected in %s\n", php_self);
EG(exit_status) =0;
} else {
zend_printf("Errors parsing %s\n", php_self);
EG(exit_status) =255;
}
I think supporting an arbitrary amount of files makes the most sense, but that might get somewhat ugly, since currently reading from STDIN is supported, and we should not allow multiple files for other operations.
Activity
cmb69 commentedon Nov 30, 2022
See https://bugs.php.net/40077. Might be doable and useful.
staabm commentedon Nov 30, 2022
Btw: I find it pretty suprising that it takes over 1 minute on a 10 core cpu windows laptop to lint the same project, which takes 15 seconds on a 2 core github action job (with 10 processes in parallel)
cmb69 commentedon Nov 30, 2022
I assume this is due to file system performance (which is slow on Windows, and multiple cores won't help with that).
staabm commentedon Dec 1, 2022
I am not that much into the details regarding windows internals.
maybe its worth to profile how expensive process creation on windows is and whether there are some low hanging fruits to get.
I am not at all a C expert though.
thanks for all the feedback until now.
cmb69 commentedon Dec 1, 2022
Oh, right, creating new PHP processes is also very slow, especially if there are many DLLs to load (part of that problem is ASLR). We have been able to considerably speed up our test runs in CI by only loading the extensions which are really required for a single test case. Anyhow, this issue would be solved for the linting case, if we support multiple files to be linted at once.
But there is still the bad file system performance, see https://bugs.php.net/80695.
staabm commentedon Dec 1, 2022
could you give me a few hints where I can start looking into it?
I would start here
php-src/sapi/cli/php_cli.c
Line 601 in d6ac533
php-src/main/main.c
Line 2604 in d6ac533
do you agree we would just allow to support a fixed number of additional arguments, or do you think we should support "n" arguments (no matter how much given)?
cmb69 commentedon Dec 1, 2022
Yes, these are the correct places (note, though, that the cgi and litespeed SAPIs also support linting). And see:
php-src/sapi/cli/php_cli.c
Lines 967 to 974 in d6ac533
I think supporting an arbitrary amount of files makes the most sense, but that might get somewhat ugly, since currently reading from STDIN is supported, and we should not allow multiple files for other operations.
KapitanOczywisty commentedon Dec 1, 2022
@cmb69 Would be sensible to implement glob patterns here? I'd expect that in most cases you want to lint everything.
cmb69 commentedon Dec 1, 2022
I'd rather avoid that. Instead, users could do something like
ls | xargs php -l(or build the command line by other means).ghnp5 commentedon Feb 16, 2023
Hey
Is it just me, or is it muuuuch slower to lint since PHP 8.2?
I have a 128GB RAM server with NVMe, and a very good CPU, and until PHP 8.1, it used to be fast enough (~1 minute) to run the lint.
Since I upgraded to PHP 8.2, it takes ~2 or 3 minutes to run the same lint across all files.
Anyway - regarding running multiple lints, I know this isn't the answer to this Feature Request, but I've just found this:
https://packagist.org/packages/overtrue/phplint
https://github.com/overtrue/phplint
which can hopefully speed up the releases.
Implement phpGH-10024: support linting multiple files at once using p…
Implement phpGH-10024: support linting multiple files at once using p…