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

Enable parallel processing for PHPCS sniffs #61700

Merged
merged 2 commits into from
May 16, 2024

Conversation

anton-vlasenko
Copy link
Contributor

@anton-vlasenko anton-vlasenko commented May 15, 2024

What?

This PR adds parallel processing for PHPCS sniffs.
Fixes #61699.

Why?

Parallel processing significantly improves speed by using multiple processes to run the PHPCS linters (300% speed increase in my tests).

How?

It's a matter of adding a single line to the phpcs.xml.dist file.
That's it.
I'm not able to find any documentation on that setting, but it seems to work great.

Testing Instructions

Make sure that GitHub CI jobs pass.

Testing Instructions for Keyboard

Screenshots or screencast

@anton-vlasenko anton-vlasenko self-assigned this May 15, 2024
Copy link

github-actions bot commented May 15, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: anton-vlasenko <[email protected]>
Co-authored-by: desrosj <[email protected]>
Co-authored-by: ironprogrammer <[email protected]>
Co-authored-by: jrfnl <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@anton-vlasenko anton-vlasenko requested review from gziolo and removed request for ironprogrammer May 15, 2024 21:58
@anton-vlasenko anton-vlasenko added [Type] Performance Related to performance efforts [Type] Build Tooling Issues or PRs related to build tooling and removed [Type] Performance Related to performance efforts labels May 15, 2024
@anton-vlasenko anton-vlasenko requested a review from desrosj May 15, 2024 22:05
@ironprogrammer
Copy link
Contributor

Regarding prior art, WordPress Core uses this same setting here: https://github.com/WordPress/wordpress-develop/blob/13d5244c43d65da5fd3ff7abd390fa36bb4a87b5/phpcs.xml.dist#L27-L28.

I'm not 100% clear on how the value of 20 processes was reached (in WordPress/wordpress-develop@3581d34), but there was some past discussion in Slack around performance and system config, as well as some measurements in the original wiki documentation issue (there is also a new open documentation issue). Can we presume this setting would also be fine for Gutenberg?

@jrfnl
Copy link
Member

jrfnl commented May 16, 2024

@anton-vlasenko I see no reason why this shouldn't be included. Not even sure why you're asking me for a review for something as trivial as this.

Parallel processing significantly improves speed by using multiple processes to run the PHPCS linters (300% speed increase in my tests).

Note: only when available via the PCTNL extension and depending on the number of Cores available, so results will vary (and it has no effect when run on Windows as the PCTNL extension isn't available).

I'm not able to find any documentation on that setting, but it seems to work great.

At this moment, this is the closest you can get to docs: PHPCSStandards/PHP_CodeSniffer#10

Copy link
Contributor

@desrosj desrosj left a comment

Choose a reason for hiding this comment

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

Thanks @anton-vlasenko.

@anton-vlasenko anton-vlasenko changed the title Enable parallel processing for PHPCS sniffs. Enable parallel processing for PHPCS sniffs May 16, 2024
@anton-vlasenko
Copy link
Contributor Author

anton-vlasenko commented May 16, 2024

@ironprogrammer Yes, I saw these documentation issues on GH as well. Thanks for posting.

Regarding the setting, PHPCS spawns 20 processes with approximately 18 files to check in each process. I propose starting with 20 and changing it later if there are any complaints.

@anton-vlasenko
Copy link
Contributor Author

I see no reason why this shouldn't be included. Not even sure why you're asking me for a review for something as trivial as this.

@jrfnl I requested your review to ensure all angles were covered, given your expertise.
It's reassuring to know that this change is indeed as straightforward as it seems.
Thanks for confirming.

@anton-vlasenko anton-vlasenko merged commit 52c2827 into trunk May 16, 2024
66 of 69 checks passed
@anton-vlasenko anton-vlasenko deleted the add/parallel-processing-for-sniffs branch May 16, 2024 15:24
@github-actions github-actions bot added this to the Gutenberg 18.5 milestone May 16, 2024
@anton-vlasenko
Copy link
Contributor Author

Thank you, everyone, for your input/reviews!

SantosGuillamot pushed a commit that referenced this pull request May 17, 2024
Enable parallel processing for PHPCS sniffs. Documentation: PHPCSStandards/PHP_CodeSniffer#10

Co-authored-by: anton-vlasenko <[email protected]>
Co-authored-by: desrosj <[email protected]>
Co-authored-by: ironprogrammer <[email protected]>
Co-authored-by: jrfnl <[email protected]>
SantosGuillamot added a commit that referenced this pull request May 17, 2024
SantosGuillamot pushed a commit that referenced this pull request May 17, 2024
Enable parallel processing for PHPCS sniffs. Documentation: PHPCSStandards/PHP_CodeSniffer#10

Co-authored-by: anton-vlasenko <[email protected]>
Co-authored-by: desrosj <[email protected]>
Co-authored-by: ironprogrammer <[email protected]>
Co-authored-by: jrfnl <[email protected]>
SantosGuillamot added a commit that referenced this pull request May 17, 2024
SantosGuillamot added a commit that referenced this pull request May 17, 2024
SantosGuillamot added a commit that referenced this pull request May 17, 2024
SantosGuillamot added a commit that referenced this pull request May 21, 2024
SantosGuillamot added a commit that referenced this pull request May 21, 2024
SantosGuillamot added a commit that referenced this pull request May 24, 2024
SantosGuillamot added a commit that referenced this pull request May 24, 2024
SantosGuillamot added a commit that referenced this pull request May 24, 2024
SantosGuillamot added a commit that referenced this pull request May 24, 2024
bph pushed a commit to bph/gutenberg that referenced this pull request May 27, 2024
Enable parallel processing for PHPCS sniffs. Documentation: PHPCSStandards/PHP_CodeSniffer#10

Co-authored-by: anton-vlasenko <[email protected]>
Co-authored-by: desrosj <[email protected]>
Co-authored-by: ironprogrammer <[email protected]>
Co-authored-by: jrfnl <[email protected]>
SantosGuillamot added a commit that referenced this pull request May 28, 2024
SantosGuillamot added a commit that referenced this pull request May 28, 2024
SantosGuillamot added a commit that referenced this pull request May 28, 2024
SantosGuillamot added a commit that referenced this pull request May 28, 2024
SantosGuillamot added a commit that referenced this pull request May 29, 2024
SantosGuillamot added a commit that referenced this pull request May 29, 2024
SantosGuillamot added a commit that referenced this pull request May 29, 2024
SantosGuillamot added a commit that referenced this pull request May 29, 2024
SantosGuillamot added a commit that referenced this pull request May 30, 2024
SantosGuillamot added a commit that referenced this pull request May 30, 2024
SantosGuillamot added a commit that referenced this pull request May 31, 2024
* Create bindings util for transforming block attributes

* Get attributes and name from the store

* Change function to return only the bound attributes

* Add action, selector, and reducer for block context

* Sync store in edit component

* Revert "Sync store in edit component"

This reverts commit 988c4b6.

* Move logic to BlockContextProvider

* Change parent context logic

* Use useLayoutEffect

* Go back to syncing store in edit component

* WIP: Move bindings logic to `getBlockAttributes`

* WIP: Move bindings setAttributes logic to updateBlockBindings

* Pass only `select` to `getValue` functions

* Remove old editor hook

* Add fallback to postId until context is ready

* Remove setValue post-meta code

* Simplify fallback conditional

* Change bindings destructuring

* Check canBindAttribute in updateBlockAttributes

* Update unit tests to expect a dispatch

* Add conditional in block

* Don't use `getBlockAttributes` inside `getValue`

* Revert "Don't use `getBlockAttributes` inside `getValue`"

This reverts commit 0e91129.

* Avoid processing bindings recursively

* Access context through selector

* Update getBlockAttributes logic

* Don't use fallbacks

* Add edit value posibility for post meta, add function to check if is admin

* Revert "Add edit value posibility for post meta, add function to check if is admin"

This reverts commit 9659455.

* Test editing is allowed in paragraph block

* Test protected fields are not editable

* Revert "Enable parallel processing for PHPCS sniffs (#61700)"

This reverts commit 8331820.

* Add post meta setValue function

* Update tests to check contenteditable

* Update lockAttributesEditing default

* Pass arguments to lockAttributesEditing

* Check user can edit post meta

* Check field is exposed in the REST API

* Disable editing in templates

* Add fallback for postId

* Revert "Revert "Enable parallel processing for PHPCS sniffs (#61700)""

This reverts commit e74d71c.

* Adapt old tests

* Simplify lockAttributesEditing fallbacks

* Don't use fallback for context

* Add postType fallback when locking controls

* Pass context in rich text

* Check contenteditable attribute in test

* Change name to `canUserEditValue`

* Revert changes caused by rebasing

* Add space back

* Pass block context through rich text

* Change imports

* Transform block attributes into bindings in split selection

* Pass context to in use-input

* Change REST API check

* Use getBoundAttributesValues

* Don't split when attribute is bound

* Revert changes caused by rebase

* Cover more blocks when editing custom fields

* Add a warning when pasting blocks

---------

Co-authored-by: Carlos Bravo <[email protected]>
carstingaxion pushed a commit to carstingaxion/gutenberg that referenced this pull request Jun 4, 2024
* Create bindings util for transforming block attributes

* Get attributes and name from the store

* Change function to return only the bound attributes

* Add action, selector, and reducer for block context

* Sync store in edit component

* Revert "Sync store in edit component"

This reverts commit 988c4b6.

* Move logic to BlockContextProvider

* Change parent context logic

* Use useLayoutEffect

* Go back to syncing store in edit component

* WIP: Move bindings logic to `getBlockAttributes`

* WIP: Move bindings setAttributes logic to updateBlockBindings

* Pass only `select` to `getValue` functions

* Remove old editor hook

* Add fallback to postId until context is ready

* Remove setValue post-meta code

* Simplify fallback conditional

* Change bindings destructuring

* Check canBindAttribute in updateBlockAttributes

* Update unit tests to expect a dispatch

* Add conditional in block

* Don't use `getBlockAttributes` inside `getValue`

* Revert "Don't use `getBlockAttributes` inside `getValue`"

This reverts commit 0e91129.

* Avoid processing bindings recursively

* Access context through selector

* Update getBlockAttributes logic

* Don't use fallbacks

* Add edit value posibility for post meta, add function to check if is admin

* Revert "Add edit value posibility for post meta, add function to check if is admin"

This reverts commit 9659455.

* Test editing is allowed in paragraph block

* Test protected fields are not editable

* Revert "Enable parallel processing for PHPCS sniffs (WordPress#61700)"

This reverts commit 8331820.

* Add post meta setValue function

* Update tests to check contenteditable

* Update lockAttributesEditing default

* Pass arguments to lockAttributesEditing

* Check user can edit post meta

* Check field is exposed in the REST API

* Disable editing in templates

* Add fallback for postId

* Revert "Revert "Enable parallel processing for PHPCS sniffs (WordPress#61700)""

This reverts commit e74d71c.

* Adapt old tests

* Simplify lockAttributesEditing fallbacks

* Don't use fallback for context

* Add postType fallback when locking controls

* Pass context in rich text

* Check contenteditable attribute in test

* Change name to `canUserEditValue`

* Revert changes caused by rebasing

* Add space back

* Pass block context through rich text

* Change imports

* Transform block attributes into bindings in split selection

* Pass context to in use-input

* Change REST API check

* Use getBoundAttributesValues

* Don't split when attribute is bound

* Revert changes caused by rebase

* Cover more blocks when editing custom fields

* Add a warning when pasting blocks

---------

Co-authored-by: Carlos Bravo <[email protected]>
patil-vipul pushed a commit to patil-vipul/gutenberg that referenced this pull request Jun 17, 2024
* Create bindings util for transforming block attributes

* Get attributes and name from the store

* Change function to return only the bound attributes

* Add action, selector, and reducer for block context

* Sync store in edit component

* Revert "Sync store in edit component"

This reverts commit 988c4b6.

* Move logic to BlockContextProvider

* Change parent context logic

* Use useLayoutEffect

* Go back to syncing store in edit component

* WIP: Move bindings logic to `getBlockAttributes`

* WIP: Move bindings setAttributes logic to updateBlockBindings

* Pass only `select` to `getValue` functions

* Remove old editor hook

* Add fallback to postId until context is ready

* Remove setValue post-meta code

* Simplify fallback conditional

* Change bindings destructuring

* Check canBindAttribute in updateBlockAttributes

* Update unit tests to expect a dispatch

* Add conditional in block

* Don't use `getBlockAttributes` inside `getValue`

* Revert "Don't use `getBlockAttributes` inside `getValue`"

This reverts commit 0e91129.

* Avoid processing bindings recursively

* Access context through selector

* Update getBlockAttributes logic

* Don't use fallbacks

* Add edit value posibility for post meta, add function to check if is admin

* Revert "Add edit value posibility for post meta, add function to check if is admin"

This reverts commit 9659455.

* Test editing is allowed in paragraph block

* Test protected fields are not editable

* Revert "Enable parallel processing for PHPCS sniffs (WordPress#61700)"

This reverts commit 8331820.

* Add post meta setValue function

* Update tests to check contenteditable

* Update lockAttributesEditing default

* Pass arguments to lockAttributesEditing

* Check user can edit post meta

* Check field is exposed in the REST API

* Disable editing in templates

* Add fallback for postId

* Revert "Revert "Enable parallel processing for PHPCS sniffs (WordPress#61700)""

This reverts commit e74d71c.

* Adapt old tests

* Simplify lockAttributesEditing fallbacks

* Don't use fallback for context

* Add postType fallback when locking controls

* Pass context in rich text

* Check contenteditable attribute in test

* Change name to `canUserEditValue`

* Revert changes caused by rebasing

* Add space back

* Pass block context through rich text

* Change imports

* Transform block attributes into bindings in split selection

* Pass context to in use-input

* Change REST API check

* Use getBoundAttributesValues

* Don't split when attribute is bound

* Revert changes caused by rebase

* Cover more blocks when editing custom fields

* Add a warning when pasting blocks

---------

Co-authored-by: Carlos Bravo <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Build Tooling Issues or PRs related to build tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Enable parallel processing for PHPCS sniffs
4 participants