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

php-cs-fixer: utilize multiple processors #4674

Merged
merged 16 commits into from May 13, 2021
Merged

php-cs-fixer: utilize multiple processors #4674

merged 16 commits into from May 13, 2021

Conversation

staabm
Copy link
Member

@staabm staabm commented May 11, 2021

utilize multi core capabilities. Using 3 processes at a time, because github action VMs have 2 cpus, but cs-fixer is also doing some IO and therefore its likely processes are blocked for IO.

In CI php-cs-fixer cannot benefit from the cs-fixer-cache, therefore running things in parallel results in a great perf boost.

Refs PHP-CS-Fixer/PHP-CS-Fixer#5390

@staabm staabm marked this pull request as ready for review May 11, 2021 18:30
@@ -39,7 +39,7 @@ jobs:

- name: Fix code style
if: env.writable == 1
run: vendor/bin/php-cs-fixer fix --diff
run: vendor/bin/php-cs-fixer list-files --config=.php-cs-fixer.dist.php | xargs -n 150 -P 3 vendor/bin/php-cs-fixer fix --diff --config=.php-cs-fixer.dist.php
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ich habe den eindruck..

  • parallelisierung beschleunigt den schritt hier von ca 20-22 auf 12-14 sekunden
  • der cs-fixer hat vor dem PR hier einen cache aufgebaut und diesen als seiteneffekt in zeile 53 genutzt - jetzt scheinbar nicht mehr
  • cs-fixer parallel laufen zu lassen und anschließend die ausgabe durch cs2pr pipen, scheint aktuell nicht möglich

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • der cs-fixer hat vor dem PR hier einen cache aufgebaut und diesen als seiteneffekt in zeile 53 genutzt - jetzt scheinbar nicht mehr

Irgendwo hatte ich es schon mal angemerkt, dieser Zweit-Check ist aus meiner Sicht eigentlich überflüssig.
Denn wenn hier etwas gefixt und commitet wird, wird anschließend ja sowieso die ganze Action neu gestartet.
Wir brauchen den dry-mode mit cs2pr aber trotzdem für Fork/Dependabot-PRs. Aber er muss nicht ablaufen, wenn schon hier der fix-Schritt ablief.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok super.

@staabm staabm marked this pull request as draft May 11, 2021 19:41
@staabm staabm self-assigned this May 12, 2021
@staabm staabm marked this pull request as ready for review May 12, 2021 17:56
@staabm staabm changed the title php-cs-fixer: use multiple processors WIP: php-cs-fixer: use multiple processors May 12, 2021
@staabm staabm changed the title WIP: php-cs-fixer: use multiple processors php-cs-fixer: utilize multiple processors May 12, 2021
@staabm
Copy link
Member Author

staabm commented May 12, 2021

Ready to land

@gharlan gharlan added the automerge Automatisch PR rebasen und mergen label May 13, 2021
@kodiakhq kodiakhq bot merged commit b3385ac into main May 13, 2021
@kodiakhq kodiakhq bot deleted the staabm-patch-4 branch May 13, 2021 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Automatisch PR rebasen und mergen
Development

Successfully merging this pull request may close these issues.

None yet

2 participants