Skip to content

Conversation

@staabm
Copy link
Contributor

@staabm staabm commented Nov 26, 2025

shot in the dark for the problem seen in phpstan/phpstan#13852


This PR adds caching when called with a array of files.

Before this PR the existing caching did only work for directories.

I tried this PR on the phpunit codebase, which proofed that with this change the numer pf findSymbols calls will be reduced to the files actually changed. Unrelated files (like bootstrap files) will no longer be processed for symbols)


this PR combined with #4576 I can see the test-suite running with php tests/vendor/bin/paratest in 38-40 seconds

@staabm
Copy link
Contributor Author

staabm commented Nov 27, 2025

before this PR: running time php bin/phpstan analyze on phpstan-src@63ee5de reports OptimizedDirectorySourceLocatorFactory->findSymbols beeing invoked..

  • 4248x when no cache exists
  • 24x when no files changed
  • 24x when src/Cache/CacheItem.php file is changed

after this PR:
running time php bin/phpstan analyze reports OptimizedDirectorySourceLocatorFactory->findSymbols beeing invoked..

  • 4248x when no cache exists
  • 0x when no files changed
  • 2x when src/Cache/CacheItem.php file is changed

I think this will fix phpstan/phpstan#13852


looking at the windows CI builds it seems those are ~20 seconds faster compared to other currently open PRs

@staabm staabm marked this pull request as ready for review November 27, 2025 06:31
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

Copy link
Member

@ondrejmirtes ondrejmirtes left a comment

Choose a reason for hiding this comment

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

Yeah I think this is an obvious improvement. I also wanted to suggest to share the common code betweet createByFiles and createByDirectory which you already did.

I have an idea for improvement. createByFiles is called from a single place. I think we should add a 2nd parameter string $uniqueCacheIdentifier which will identify the entry in the cache.

That will allow us for the identifier to become the cache key. Your current approach has a downside - when a file is removed/added, the whole cache has to go. But with a stable identifier, we'll only replace the cached parts which need replacing.

@ondrejmirtes ondrejmirtes merged commit 522421b into phpstan:2.1.x Nov 27, 2025
607 of 637 checks passed
@ondrejmirtes
Copy link
Member

Thank you!

@staabm staabm deleted the filec branch November 27, 2025 10:15
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