faster FileHelper::normalizePath()
#735
Conversation
| if ($matches !== null) { | ||
| [, $scheme, $path] = $matches; | ||
| } else { | ||
| $scheme = null; | ||
| $path = $originalPath; | ||
| } | ||
|
|
||
| $path = str_replace('\\', '/', $path); | ||
| $path = Strings::replace($path, '~/{2,}~', '/'); | ||
| $path = str_replace(['\\', '//', '///', '////'], '/', $path); |
There was a problem hiding this comment.
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.
src/File/FileHelper.php
Outdated
| @@ -41,16 +41,21 @@ public function absolutizePath(string $path): string | |||
| /** @api */ | |||
| public function normalizePath(string $originalPath, string $directorySeparator = DIRECTORY_SEPARATOR): string | |||
| { | |||
| $matches = \Nette\Utils\Strings::match($originalPath, '~^([a-z]+)\\:\\/\\/(.+)~'); | |||
| $isLocalPath = $originalPath && $originalPath[0] === '/'; | |||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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://?
|
Thank you! |
running phpstan on one of our projects consistently leads to a profile like
see that
FileHelper::normalizePath()is a very hot path, mostly dominated by PCRE invocations.this PR proposes small adjustments so
normalizeno longer needs to invoke PCRE in the common case.with this changes we get a solid speed improvement:
also note a improvement in memory consumption.
profiles compared are based on the latest commit on master b726e08