Skip to content

use is_file() over file_exists(), which is faster#724

Merged
ondrejmirtes merged 2 commits intophpstan:masterfrom
staabm:is-file
Oct 25, 2021
Merged

use is_file() over file_exists(), which is faster#724
ondrejmirtes merged 2 commits intophpstan:masterfrom
staabm:is-file

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented Oct 20, 2021

file_exists() is up to 100 times slower

Inspired by https://bugs.php.net/bug.php?id=78285

Comment on lines 42 to 44
Copy link
Copy Markdown
Contributor Author

@staabm staabm Oct 20, 2021

Choose a reason for hiding this comment

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

this is the only spot where logic was changed within the PR.

I have moved the fast is_file positive-lookup case in the front and moved the less-likely file_exists negative case into the else-if

@staabm staabm changed the title use is_file() over file_exists() which is faster use is_file() over file_exists(), which is faster Oct 20, 2021
@staabm
Copy link
Copy Markdown
Contributor Author

staabm commented Oct 25, 2021

I was not able to come up with a profile which shows a measurable improvement by this PR.

only thing I can say is, that is_file() and friends are doing way less syscalls and therefore perform better.
in case this is good enough for you as an argument feel free to merge.

in case you wanne play safe, we can close here. its up to you.

@ondrejmirtes
Copy link
Copy Markdown
Member

The only risk here is that file_exists can return true for directories, but after careful review I don't think we're pasing a directory in any of the changed lines.

@ondrejmirtes ondrejmirtes merged commit a1e2fd9 into phpstan:master Oct 25, 2021
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you!

@staabm staabm deleted the is-file branch October 25, 2021 15:44
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.

3 participants