Disable PCOV if installed (40% faster on affected systems) - #6253
Conversation
|
Could you show a proof in a reproducer repo how much pcov affects a github actions CI build? |
|
Scout honer and screen shots is the best I can do: So 35-45% on GitHub action as well so the effect is probably pretty flat. The original measure was on PHPStan it self from my workstation, this is a LARGE Laravel lib (Larastan + phpstan-phpunit + phpstan-mockery + phpstan-strict-rules, no Bladestan since it's a lib with no templates) |
|
Nice. I agree that we should reflect composer/xdebug-handler 1:1 in handling this (regarding api and process restarts etc) Might be even worth open a issue on composer and see how they think about doing similar things for composer (and measure impact of pcov on composer itself) |
| $errorOutput->writeLineFormatted('<comment>Tip: The pcov extension is active, which makes this run slower than it needs to be.</comment>'); | ||
| $errorOutput->writeLineFormatted('pcov hooks into every function call in the process, even though PHPStan never collects code coverage.'); | ||
| $errorOutput->writeLineFormatted(sprintf('PHPStan disables it in its worker processes - run PHPStan with "php -d %s" to disable it in the main process too.', PcovHelper::DISABLED_INI_SETTING)); | ||
| $errorOutput->writeLineFormatted(sprintf('Set %s=1 if you need pcov to stay enabled.', PcovHelper::ALLOW_ENV_VARIABLE)); |
There was a problem hiding this comment.
Do you think there's a valid use case? If not you can remove the env var support.
Also how different are the runs with pcov off, and with pcov on but PHPStan restarting? I'm pondering not printing any message at all, the output is really noisy already.
There was a problem hiding this comment.
No not really, it was mostly to mirror the other on in case it got split out, and I wasn't sure how it would look in PHPStan's own CI but that turned out to be a non issue.
|
We should assert we don't need 2 restarts per process when xdebug and pcov are enabled? |
|
Oh yeah, no idea whether we can hook onto XdebugHandler like that. Also there's yet another restart in bin/phpstan now - for turbo-ext, under certain circumstances. It's done with pcntl_exec which might be nicer than proc_open (if available) but not sure. |
|
Maybe we could build our own system - find reasons for restart and if |
|
This issue makes me think that other profiler extensions like blackfire, php-spx might also have the same problem We should measure that |
|
I just found one issue, the new fork work means we have to restart the main process or the workers will inherit the pcov state.
Good call, maybe the realization here is that we should build a PHPStan dedicated helper for all of these, detect them all, disable them all in one restart, update when new ones are found. xdebug can get special handeling inside of it for allowing debugging. |
|
And --debug should never restart. I actually sometimes need to run with SPX/Blackfire 😂 And I don't think we should pollute the options with --pcov, --blackfire, --spx etc. |
I have something working, I'll go over the various extensions to see how much they each affect performance. |





Very 2017 Déjà vu, this often affect CI where PCOV may be installed for faster coverage then xdebug, but when coverage isn't needed it can still hurt performance quite a bit (xdebug was 400% if I remember correctly).
Unlike xdebugHandler this isn't ported from another package or spun out from one. If you prefer I could create ajenbo/pcov-handler and then start shopping it around other projects as well. But even for that case maybe it would be good to start with it as part of phpstan and then later spin it off from there?