Skip to content

support linting multiple files at once using php -l #10024

Closed

Description

@staabm

Description

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

grafik

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.

Activity

  1. cmb69 commented on Nov 30, 2022

    @cmb69
    Member

    See https://bugs.php.net/40077. Might be doable and useful.

  2. staabm commented on Nov 30, 2022

    @staabm
    ContributorAuthor

    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)

  3. cmb69 commented on Nov 30, 2022

    @cmb69
    Member

    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).

  4. staabm commented on Dec 1, 2022

    @staabm
    ContributorAuthor

    I assume this is due to file system performance

    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.

  5. cmb69 commented on Dec 1, 2022

    @cmb69
    Member

    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.

    But there is still the bad file system performance, see https://bugs.php.net/80695.

  6. staabm commented on Dec 1, 2022

    @staabm
    ContributorAuthor

    this issue would be solved for the linting case

    could you give me a few hints where I can start looking into it?

    I would start here

    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)?

  7. cmb69 commented on Dec 1, 2022

    @cmb69
    Member

    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

    case PHP_MODE_LINT:
    if (php_lint_script(&file_handle) == SUCCESS) {
    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.

  8. KapitanOczywisty commented on Dec 1, 2022

    @KapitanOczywisty

    @cmb69 Would be sensible to implement glob patterns here? I'd expect that in most cases you want to lint everything.

  9. cmb69 commented on Dec 1, 2022

    @cmb69
    Member

    Would be sensible to implement glob patterns here? I'd expect that in most cases you want to lint everything.

    I'd rather avoid that. Instead, users could do something like ls | xargs php -l (or build the command line by other means).

  10. ghnp5 commented on Feb 16, 2023

    @ghnp5

    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.

  11. added a commit that references this issue on Jun 29, 2023
  12. added a commit that references this issue on Jan 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions