Skip to content

Conversation

@kukulich
Copy link
Contributor

@kukulich kukulich commented May 5, 2022

No description provided.

@kukulich kukulich force-pushed the optimized branch 3 times, most recently from c7190a4 to a5cc9dd Compare May 5, 2022 16:31
@staabm
Copy link
Contributor

staabm commented Dec 3, 2022

@kukulich just found this PR. Is it still relevant? Is there a concrete problem beeing solved here (it looks like its a performance improvement)?

@kukulich
Copy link
Contributor Author

kukulich commented Dec 3, 2022

I discussed it with @ondrejmirtes directly. I think we tried to get rid of the reqexp in OptimizedDirectorySourceLocator.php::findSymbols() because it has some problems. However I think there were some problems with the failing tests and it was not caused by this PR. I will try to rebase this PR to refresh the checks.

@ondrejmirtes
Copy link
Member

This PR makes future maintenance of this locator easier but right now has worse performance.

I have a plan to address this - we could make PHPStan faster because right now, the same work is done by all child processes which means it's done 8 times during each run. Instead, we could populate some cache in the main process before launching child processes, and those child processes could just read the cached data instead of doing all the work again.

@kukulich kukulich changed the base branch from 1.7.x to 1.9.x December 5, 2022 09:00
@kukulich
Copy link
Contributor Author

kukulich commented Dec 5, 2022

Rebased.

@kukulich kukulich marked this pull request as ready for review December 5, 2022 09:19
@phpstan-bot
Copy link
Collaborator

This pull request has been marked as ready for review.

* @author Jordi Boggiano <j.boggiano@seld.be>
* @see https://github.com/composer/composer/pull/10107
*/
class PhpFileCleaner
Copy link
Contributor

Choose a reason for hiding this comment

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

I remember seeing blackfire profiles where the PhpFileCleaner logic was on the hot path.

really like seeing this file beeing deleted :)

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