Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Ability to run Fixer with parallel runner 🎉 #7777

Merged
merged 77 commits into from
May 15, 2024

Commits on May 15, 2024

  1. Register hidden WorkerCommand

    Empty for now, because logic for running fixers must be moved here. Basically it's a PoC of command being hidden - works on PHP 7.4 and 8.3 with latest Composer packages on each version.
    
    Command's arguments/options are more or less what we need in the worker, but it may be changed later.
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    f48325b View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    186bec4 View commit details
    Browse the repository at this point in the history
  3. Extract factory method for linting aware file iterator

    The same collection of files has to be determined for both sequential and parallel run.
    
    See PR7777#discussion_r1526648174
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    eebe4ef View commit details
    Browse the repository at this point in the history
  4. Initial implementation of parallel analysis 🎉

    It's working, but needs polishing here and there. It speeds up analysis significantly (like 5x for this codebase on my computer).
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    b788803 View commit details
    Browse the repository at this point in the history
  5. Use sequential runner by default

    Parallel runner must be introduced slowly, and start as an opt-in. When it's stable enough, we can change it to be default one.
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    c12f95f View commit details
    Browse the repository at this point in the history
  6. Configuration menu
    Copy the full SHA
    9fb0766 View commit details
    Browse the repository at this point in the history
  7. Auto detection for parallel config

    Using `ParallelConfig::detect()` can be used for utilising all available resources. Tested by running analysis natively on the host and inside Docker (with core limit set on the OrbStack level). In both scenarios it detects available cores correctly.
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    22d6bf3 View commit details
    Browse the repository at this point in the history
  8. Configuration menu
    Copy the full SHA
    2afff3b View commit details
    Browse the repository at this point in the history
  9. Configuration menu
    Copy the full SHA
    845b4ab View commit details
    Browse the repository at this point in the history
  10. Configuration menu
    Copy the full SHA
    32355df View commit details
    Browse the repository at this point in the history
  11. Configuration menu
    Copy the full SHA
    d5c4283 View commit details
    Browse the repository at this point in the history
  12. Configuration menu
    Copy the full SHA
    ea93685 View commit details
    Browse the repository at this point in the history
  13. Configuration menu
    Copy the full SHA
    6f11b20 View commit details
    Browse the repository at this point in the history
  14. Configuration menu
    Copy the full SHA
    6bdfa0e View commit details
    Browse the repository at this point in the history
  15. Configuration menu
    Copy the full SHA
    039a1f7 View commit details
    Browse the repository at this point in the history
  16. Configuration menu
    Copy the full SHA
    e265e51 View commit details
    Browse the repository at this point in the history
  17. Configuration menu
    Copy the full SHA
    93935d9 View commit details
    Browse the repository at this point in the history
  18. Explicitly require react/stream

    Detected by Composer Require Checker
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    dd53801 View commit details
    Browse the repository at this point in the history
  19. Configuration menu
    Copy the full SHA
    1f892a6 View commit details
    Browse the repository at this point in the history
  20. Support cache in parallel analysis

    Cache check is done during file iteration - if file is in cache and wasn't changed, then it's skip by iterator and doesn't even go to the worker. That's why `ReadonlyCacheManager` used on worker's side is a bit superfluous, but I thought it's safer to use it instead of `NullCacheManager`.
    
    Anyway, each file is stored to cache when worker reports analysis result to the parallelisation operator, so it's basically similar to sequential analysis, because file write is done only in one process.
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    d0364b1 View commit details
    Browse the repository at this point in the history
  21. Configuration menu
    Copy the full SHA
    63529c2 View commit details
    Browse the repository at this point in the history
  22. Apply Julien's suggestions from code review

    Co-authored-by: Julien Falque <julien.falque@gmail.com>
    Wirone and julienfalque committed May 15, 2024
    Configuration menu
    Copy the full SHA
    39a8c0e View commit details
    Browse the repository at this point in the history
  23. Sync --using-cache description

    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    41ed875 View commit details
    Browse the repository at this point in the history
  24. Use InvalidArgumentException for parallelisation misconfiguration +…

    … PHPStan narrowed types
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    34f3de0 View commit details
    Browse the repository at this point in the history
  25. Fix BC-break in DirectoryInterface

    Use absolute file path for reporting analysis result and convert it to relative path when processing on parallelisation operator's side. It gives the same effect in terms of caching, but does not require `DirectoryInterface::getAbsolutePath()`.
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    4bee6dd View commit details
    Browse the repository at this point in the history
  26. Report file analysis result per file instead of per chunk

    It improves caching, because cache save can be triggered more frequently and errors or interrupting during processing large chunk won't cause analysis info being lost (which would lead to re-analysing file that was already processed).
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    e3726fb View commit details
    Browse the repository at this point in the history
  27. Clean up parallel actions handling

    - extract constants to dedicated class with more fitting name
    - use constants in every place where communication between runner and worker occurs
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    c9b75bf View commit details
    Browse the repository at this point in the history
  28. Dirty workaround for running sequential runner in tests

    There were 2 problems:
    - when child processes were spawned, `ArgvInput` was created to pass the arguments/options to the worker, but when fix/check command was executed via command tester, `ArgvInput` was using PHPUnit's argv from top-level command (and was failing with "The "--configuration" option does not exist.")
    - Passing `InputInterface` instance from command to process (did an experiment with static property set in command to skip passing input through the Runner instance) was not enough because output was invalid and tests were failing.
    
    I hope we can figure out something better after discussion on code review 😉.
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    4d1e7aa View commit details
    Browse the repository at this point in the history
  29. Add most of the tests

    Some of the tests are little hacky at this point, maybe it's possible to refactor a bit to achieve better testability. Good enough for now, though.
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    9d965ee View commit details
    Browse the repository at this point in the history
  30. Extract process creation to separate factory

    Passing root process' input to the runner and then to `ProcessFactory` allows switching `InputInterface` implementation and test process creation.
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    4d477ff View commit details
    Browse the repository at this point in the history
  31. Configuration menu
    Copy the full SHA
    2ec2ed8 View commit details
    Browse the repository at this point in the history
  32. Configuration menu
    Copy the full SHA
    19d5bf2 View commit details
    Browse the repository at this point in the history
  33. Configuration menu
    Copy the full SHA
    cffeb97 View commit details
    Browse the repository at this point in the history
  34. Allow wider range of React packages

    Tested on:
    - PHP 8.3
    - PHP 8.3 with --prefer-lowest
    - PHP 7.4 with --prefer-lowest
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    b92bb52 View commit details
    Browse the repository at this point in the history
  35. Introduce --sequential option to fix command

    - it can be helpful when using parallel config but there's need to disable it temporarily (it's easier to add option to the command than editing config file)
    - it solves the internal problem with parallel runner being used for running tests that operate on command's output (parallel run produces undeterministic output because files are not processed in the strict order)
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    df9184c View commit details
    Browse the repository at this point in the history
  36. More tests, more coverage

    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    24ac18b View commit details
    Browse the repository at this point in the history
  37. Fix smoke tests

    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    41cb3e0 View commit details
    Browse the repository at this point in the history
  38. Configuration menu
    Copy the full SHA
    3c9af57 View commit details
    Browse the repository at this point in the history
  39. Configuration menu
    Copy the full SHA
    b06ad0c View commit details
    Browse the repository at this point in the history
  40. Add missing tests

    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    a305038 View commit details
    Browse the repository at this point in the history
  41. Configuration menu
    Copy the full SHA
    53bd005 View commit details
    Browse the repository at this point in the history
  42. Configuration menu
    Copy the full SHA
    f0918dd View commit details
    Browse the repository at this point in the history
  43. Configuration menu
    Copy the full SHA
    f8798a5 View commit details
    Browse the repository at this point in the history
  44. Configuration menu
    Copy the full SHA
    def6275 View commit details
    Browse the repository at this point in the history
  45. Fix test file/class name

    Broken during rebase.
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    cd8eb2f View commit details
    Browse the repository at this point in the history
  46. Configuration menu
    Copy the full SHA
    a18b4e0 View commit details
    Browse the repository at this point in the history
  47. Remove superfluous assertion

    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    2cec9a2 View commit details
    Browse the repository at this point in the history
  48. Configuration menu
    Copy the full SHA
    21f4e28 View commit details
    Browse the repository at this point in the history
  49. Configuration menu
    Copy the full SHA
    2053e7f View commit details
    Browse the repository at this point in the history
  50. Test future mode in Config

    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    e9d5e73 View commit details
    Browse the repository at this point in the history
  51. Skip linting on file iteration in main process of parallel runner

    The linting is done on the workers' side before fixers are applied, so we will collect any syntax errors anyway.
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    d5f6f47 View commit details
    Browse the repository at this point in the history
  52. Add todos for 4.0

    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    4d3bf56 View commit details
    Browse the repository at this point in the history
  53. Configuration menu
    Copy the full SHA
    34308f9 View commit details
    Browse the repository at this point in the history
  54. Configuration menu
    Copy the full SHA
    a766504 View commit details
    Browse the repository at this point in the history
  55. Configuration menu
    Copy the full SHA
    eaa1354 View commit details
    Browse the repository at this point in the history
  56. Configuration menu
    Copy the full SHA
    a4cc010 View commit details
    Browse the repository at this point in the history
  57. Configuration menu
    Copy the full SHA
    cfbf0bb View commit details
    Browse the repository at this point in the history
  58. Configuration menu
    Copy the full SHA
    2d05341 View commit details
    Browse the repository at this point in the history
  59. Make RunnerConfig internal

    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    71b089e View commit details
    Browse the repository at this point in the history
  60. Configuration menu
    Copy the full SHA
    051b9ee View commit details
    Browse the repository at this point in the history
  61. Fix smoke tests

    - always expect CPU(s) usage info in the output
    - always use dist config for PHAR parallel test (because local config can use sequential analysis)
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    763cc8b View commit details
    Browse the repository at this point in the history
  62. Align with PHPStan level 7

    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    df35f4f View commit details
    Browse the repository at this point in the history
  63. Configuration menu
    Copy the full SHA
    ee9def7 View commit details
    Browse the repository at this point in the history
  64. Refactoring and cosmetic changes

    * syntax sugar
    * sequential option - no shortcut, as we do not use shortcut for other any other option
    * explicitly mark `ProcessFactory::getCommandArgs` as `@private`, to indicate it shall not be called from outside of this class, ref PHP-CS-Fixer#7777 (comment)
    * `WorkerCommand` options written as single-liners, so unified as all other commands and easier to copy-paste
    * add explicit comment for hardcoded option
    * remove unused var
    * rename `ParallelRunnerConfigInterface` to `ParallelAwareConfigInterface`
    * make `ParallelConfig` only DTO and externalize factory methods to dedicated class
    * `Runner` - don't reevaluate `maxFixerPerProcess` in each loop
    * explicitly call generator, avoid simple 'job' as too similar to job-id
    * rename variables related to file chunks
    keradus authored and Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    c6ea784 View commit details
    Browse the repository at this point in the history
  65. More strict approach in WorkerCommand

    * explicitly crash on unexpected parallel action
    * explicitly crash on unexpected events count, i.e. when someone wrongly refactors EventBus
    * no need to calculate relative path + validate if result has proper size
    * introduce `ErrorsManager::reportWorkerError()`: don't report `WorkerError` via `report(Error)` method
    keradus authored and Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    97a2764 View commit details
    Browse the repository at this point in the history
  66. Auto-detected parallel config by default for future mode or as an opt…

    …-in via env variable
    keradus authored and Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    ab01e66 View commit details
    Browse the repository at this point in the history
  67. Reuse existing cache hash to avoid IO read operation

    * use cache nicely, without recalculation of hash
    * no need for read-only cache, anymore
    keradus authored and Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    69b64be View commit details
    Browse the repository at this point in the history
  68. Configuration menu
    Copy the full SHA
    f347f13 View commit details
    Browse the repository at this point in the history
  69. Configuration menu
    Copy the full SHA
    077fd6b View commit details
    Browse the repository at this point in the history
  70. Clean up loop that re-reports errors from worker to main process' err…

    …or manager
    
    - Remove invalid comment (no idea why it was here)
    - rename loop's variable to not confuse it with `WorkerError` class that represents unrecoverable errors on worker side
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    b7ac50d View commit details
    Browse the repository at this point in the history
  71. Configuration menu
    Copy the full SHA
    5a0cc3e View commit details
    Browse the repository at this point in the history
  72. Flip naming convention for ParallelAction consts and improve them

    It's actually more natural to name these actions after initiating side - when we're writing to the output, we should use action related to the place where we're writing, and when we're reading the message, action should point to the callee.
    
    See: PHP-CS-Fixer#7777 (comment)
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    5069549 View commit details
    Browse the repository at this point in the history
  73. Configuration menu
    Copy the full SHA
    d613ade View commit details
    Browse the repository at this point in the history
  74. Configuration menu
    Copy the full SHA
    7aa838b View commit details
    Browse the repository at this point in the history
  75. Configuration menu
    Copy the full SHA
    f63b4d5 View commit details
    Browse the repository at this point in the history
  76. Rework error handling in workers

    - handled process errors (linting, failed fixes etc.) are now collected with ~original exception as source when possible. These don't break the main process, unless `--stop-on-violation` is used, it was like that before too, but `ParallelisationException` was used for recreating error's source, which was not correct.
    - other errors (socket in/out errors caught by React, or unhandled errors that kill the worker) are re-thrown in the main process that effectively stop further execution
    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    0ad99f6 View commit details
    Browse the repository at this point in the history
  77. Fix smoke tests

    Wirone committed May 15, 2024
    Configuration menu
    Copy the full SHA
    1bae68b View commit details
    Browse the repository at this point in the history