Skip to content

faster FileHelper::normalizePath()#735

Merged
ondrejmirtes merged 5 commits intophpstan:masterfrom
staabm:speedup-file-helper
Oct 25, 2021
Merged

faster FileHelper::normalizePath()#735
ondrejmirtes merged 5 commits intophpstan:masterfrom
staabm:speedup-file-helper

Conversation

@staabm
Copy link
Copy Markdown
Contributor

@staabm staabm commented Oct 25, 2021

running phpstan on one of our projects consistently leads to a profile like

grafik
see that FileHelper::normalizePath() is a very hot path, mostly dominated by PCRE invocations.

this PR proposes small adjustments so normalize no longer needs to invoke PCRE in the common case.
with this changes we get a solid speed improvement:

grafik

grafik

also note a improvement in memory consumption.

profiles compared are based on the latest commit on master b726e08


$path = str_replace('\\', '/', $path);
$path = Strings::replace($path, '~/{2,}~', '/');
$path = str_replace(['\\', '//', '///', '////'], '/', $path);
Copy link
Copy Markdown
Contributor Author

@staabm staabm Oct 25, 2021

Choose a reason for hiding this comment

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

I assume that we won't see path with more then 4 slashes in a row.
the previous regex supported "any number of slashes" but I think thats more we actually need.

public function normalizePath(string $originalPath, string $directorySeparator = DIRECTORY_SEPARATOR): string
{
$matches = \Nette\Utils\Strings::match($originalPath, '~^([a-z]+)\\:\\/\\/(.+)~');
$isLocalPath = $originalPath && $originalPath[0] === '/';
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

most of the path we get passed start with a / (regular unix path).
these can never match the match-regex below which tries to detect path from within phar files.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about Windows?

Copy link
Copy Markdown
Contributor Author

@staabm staabm Oct 25, 2021

Choose a reason for hiding this comment

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

yeah also thought about that (I am even working on windows most of the time ;-)).

we could either do a educated guess like the 2nd char is a : as in C:\.

maybe it would even be possible to check whether the first char of the string is != 'p', as the code later on is checking for a phar:// prefix. not sure this code is meant to handle more cases other then phar://?

@staabm staabm marked this pull request as ready for review October 25, 2021 12:46
@ondrejmirtes ondrejmirtes merged commit 1505153 into phpstan:master Oct 25, 2021
@ondrejmirtes
Copy link
Copy Markdown
Member

Thank you!

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

4 participants