Performance: Prevent sorting of files in PhpFilesFinder #4164
Conversation
| ->in($directories); | ||
|
|
||
| if ($sortByName) { | ||
| $finder->sortByName(); |
There was a problem hiding this comment.
before this PR we sorted everything everytime. this sorting is slow and should only be used when necessary
|
The order is releveant to reproducible run. Windows/*nix/Mac sorts files in a different way. First discovered classes are available to PHPStan reflection. Yet, I think we can avoid the sorting to get better performance 👍 Btw, can you try to drop the sorting completely? If CI passing well, we should go for it |
disabling it makes e2e test fail. it seems the change of this PR does not influence the final result order
maybe we could sort the results when rendering the diff, instead of reading the files sorted? |
staabm
changed the title
Sounds good 👍 Feel free to add it to this PR instead, so we have it all finished on merge |
|
@staabm the commit with double quote will broke the scoped build, I think we need to fix that eventually. @rez1dent3 since you had initialized to use github output at PR: Could you help verify how escaped get commit message on scoped build for us? The issue is the build will broke if the commit has double quote, eg: |
staabm
force-pushed
the
no-sort
branch
2 times, most recently
from
d4951cf
to
4652c4e
Compare
I had a look into the possible perf impact. removing the sorting alltogehter brings only a negligible perf benefit.
I know we had the same problem in phpstan-src. let me have a look |
in phpstan-src this seems to work.. it looks pretty similar to what we have in rector-src though |
|
Fix in #4175 |
|
This PR should be good to go, see #4164 (comment) |
|
Thank you @staabm , let's give it a try 👍 |
|
@samsonasik Hello. Got it? If not, then I can look on Wednesday. |
|
It was fixed with #4178 |
From my understanding the order in which PhpFilesFinder gets its files from sfFinder is not relevant for the final result.
with this PR, I get 25% faster rector run with a 7% safe in memory