perf: Improve performance of Box::collectRemainingFiles() #1560
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The `->notPath()` filter of Symfony Finder is either accepting a “literal path” or a regular expression and converts each literal path to a regular expression and then filters the file stream by applying each regex individually. This leads to O(bufferedFileNames^2) calls to `preg_match()`, well over 20 million for PHPStan. Fix this by performing a simple `in_array()` check on the buffered files. This brings down the time to compile PHPStan from 2:35 to 0:25 on my machine. I have verified that the resulting PHPStan Phar hash is identical before and after this change. Fixes box-project#1551.
TimWolla
force-pushed
the
box-sort-performance
branch
from
e449e70 to
36860ae
Compare
TimWolla
changed the title
Member
|
Wow I didn't realise that, awesome, thanks! |
Contributor
|
Wild. Appreciate this, Tim. @theofidry, you still keen to keep this one behind a feature flag now we have this improvement in place? I don't mind either way but happy to remove the need for the flag if we feel this is now in a better spot. |
Member
|
If we're confident the perf impact is <10-20% I'm fine with making it the default and making the existing flag a noop option. Actually this could already be assessed without any other changes. We know have a few projects for which we know the impact was quite big and for which we can compare the build with and without the flag. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The
->notPath()filter of Symfony Finder is either accepting a “literal path” or a regular expression and converts each literal path to a regular expression and then filters the file stream by applying each regex individually. This leads to O(bufferedFileNames^2) calls topreg_match(), well over 20 million for PHPStan.Fix this by performing a simple
in_array()check on the buffered files. This brings down the time to compile PHPStan from 2:35 to 0:25 on my machine.I have verified that the resulting PHPStan Phar hash is identical before and after this change.
Fixes #1551.
Tideways Comparison showing the improvement: https://app.tideways.io/share/trace/55089586-d71b-4fe5-a00a-08482ad2df06/compare/3abe5b8b-a960-4c0a-9c43-437a3dc23630/callgraph (link valid for 1 month, screenshot for posterity).