Skip to content

ParentDirectoryRelativePathHelper: Move repetative work into constructor - #6611

Merged
staabm merged 1 commit into
phpstan:2.3.xfrom
staabm:lessp
Sep 27, 2026
Merged

staabm merged 1 commit into
phpstan:2.3.xfrom
staabm:lessp

Conversation

@staabm

@staabm staabm commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

getFileNameParts shows up in profiles:

grafik

before PR

➜  phpstan-src git:(2.3.x) hyperfine --prepare 'php bin/phpstan clear-result-cache' 'bin/phpstan analyse -l 8 src/Analyser/ src/Rules/ src/Type/ -q' -i --runs=5
Benchmark 1: bin/phpstan analyse -l 8 src/Analyser/ src/Rules/ src/Type/ -q
  Time (mean ± σ):      8.466 s ±  0.054 s    [User: 72.162 s, System: 12.171 s]
  Range (min … max):    8.411 s …  8.538 s    5 runs
 
  Warning: Ignoring non-zero exit code.

after PR

➜  phpstan-src git:(lessp) hyperfine --prepare 'php bin/phpstan clear-result-cache' 'bin/phpstan analyse -l 8 src/Analyser/ src/Rules/ src/Type/ -q' -i --runs=5
Benchmark 1: bin/phpstan analyse -l 8 src/Analyser/ src/Rules/ src/Type/ -q
  Time (mean ± σ):      8.341 s ±  0.068 s    [User: 71.140 s, System: 11.874 s]
  Range (min … max):    8.244 s …  8.410 s    5 runs
 
  Warning: Ignoring non-zero exit code.

@staabm

staabm commented Sep 27, 2026

Copy link
Copy Markdown
Contributor Author

//cc @SanderMuller

@staabm
staabm marked this pull request as ready for review September 27, 2026 09:03
@phpstan-bot

Copy link
Copy Markdown
Collaborator

This pull request has been marked as ready for review.

Comment on lines -55 to -56
$parentPath = implode('/', array_slice($parentParts, 0, $i + 1));
$filenamePath = implode('/', array_slice($filenameParts, 0, $i + 1));

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.

less string juggling in the loop

@SanderMuller SanderMuller left a comment

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.

I checked that the output stays the same, and measured the gain.

Comparing one part at a time gives the same answer as comparing the joined prefixes. A part never contains /, so two equally long lists of parts join to the same string only if every part is equal.

I also ran the 2.3.x 3bc4b93d6 version and this PR's version of getFilenameParts() side by side on 1,032,768 parent and file pairs. The random pairs mix both separators, empty parts, ., .., phar://, a drive letter and non-ASCII parts. The rest are every combination of up to three parts from a, b, an empty part and ... All results match, and 226,358 of them contain ... With a wrong comparison planted in the new version, the same run reports 6,332 mismatches, so it does catch a difference.

  • ParentDirectoryRelativePathHelperTest (18 tests), make tests (22366 tests), make phpstan and phpcs pass on this PR. The class is not shadowed by turbo.
  • One call takes about 290 ns instead of about 805 ns (user CPU, 300,000 calls over three typical paths, 5 runs each).
  • I ran your command, analyse -l 8 src/Analyser/ src/Rules/ src/Type/, with the result cache cleared, in 5 interleaved rounds. User+sys CPU is 86.5 s median (85.1-89.9) on 2.3.x and 85.5 s (83.2-86.8) here. That is about 1% less. The ranges overlap at a load of 19-31 on this machine, so this agrees with your 1.4% but does not narrow it down.

Each of the 12 red checks also fails on #6609 or #6604 today, so none of them comes from this change.

@staabm
staabm merged commit ebaf7f9 into phpstan:2.3.x Sep 27, 2026
901 of 914 checks passed
@staabm
staabm deleted the lessp branch September 27, 2026 12:04
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