Skip to content

Performance: Memoize sipped file paths#3495

Merged
samsonasik merged 2 commits intorectorphp:mainfrom
keulinho:memoize-path-skips
Mar 20, 2023
Merged

Performance: Memoize sipped file paths#3495
samsonasik merged 2 commits intorectorphp:mainfrom
keulinho:memoize-path-skips

Conversation

@keulinho
Copy link
Copy Markdown
Contributor

@keulinho keulinho commented Mar 20, 2023

Every AbstractRector checks if it should be skipped and executes all skippers, this leads to a lot of calls to the SkipVoters.

The PathSkipVoter only skips based on the file path, but will be called for each configured rector and file in combination. Each call executed the doesFileInfoMatchPatterns which internally does a pcre match costing a lot of memory.

By memoizing the skipped file path we can greatly reduce the needed memory. See this blackfire screenshot to see the impact:
image
You can see the full blackfire comparison here: https://blackfire.io/profiles/compare/d05668b7-6aea-4f8e-a9ef-88185948bfb7/graph

The used memory is decreased by ~50% in a test run with ~250 files. Also note that this method is called >75 000 times for this small test run.

This is related to rectorphp/rector#7806 and improves the situation for larger code bases quite a lot.

@keulinho keulinho requested a review from TomasVotruba as a code owner March 20, 2023 14:57
@samsonasik
Copy link
Copy Markdown
Member

New phpunit release https://github.com/sebastianbergmann/phpunit/releases/tag/10.0.17 seems cause error:

PHP Fatal error:  Uncaught Error: Non-static method PHPUnit\Event\Facade::seal() cannot be called statically in /home/runner/work/rector-src/rector-src/vendor/brianium/paratest/src/WrapperRunner/WrapperRunner.php:102

You can pin to "phpunit/phpunit": "10.0.16" for now while wait for next paratest compatibility for it.

Copy link
Copy Markdown
Member

@samsonasik samsonasik left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@samsonasik samsonasik merged commit ed11caa into rectorphp:main Mar 20, 2023
@samsonasik
Copy link
Copy Markdown
Member

Thank you @keulinho

@keulinho keulinho deleted the memoize-path-skips branch March 20, 2023 15:22
samsonasik pushed a commit that referenced this pull request May 8, 2023
* Performance: Memoize sipped file paths

* Pin phpunit version
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants