Repository navigation
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: PHP 8.4 with PCOV: PHP 8.5 with PCOV: PHP 8.4 without PCOV: PHP 8.5 without PCOV: 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. |
|
3 extensions turned out to cause issues, the rest is within noise level: 
Most of them can simply be disabled and don't actually need to be unloaded which allows us to still reflect on the methods they add. |
|
today while looking into phpstan consuming repos, I stumbled over kimai which was suprisingly slow in CI: it takes 45 seconds in CI, while it takes 5 seconds on my local machine. I guess its also related to pcov @AJenbo please rebase and target 2.3.x @SanderMuller please review |
Did you test on PHP 8.2? wonder if PHP 8.5 itself is also faster. And Kimai uses the standard ubuntu runner I think, which has only a couple cores. Curious what part of 45s vs 5s is due to which factors (Amount of cores / PHP version / pcov) |
|
@staabm I'm working on a bit of a rewrite where it handle all 4 extension and jit settings to avoid repeated reboots and consolidating all of that logic and make it simple to add any future extensions that needs managment, I was unable to make progress on it over the past two weeks because of some things at work, but I expect to finish it this week. Thanks for the patience. |
the repo uses 2 phpstan jobs: pcov loaded: 34s + 32s = 66s (PHP 8.2) |
|
I measured the premise rather than the diff, since a rewrite is coming. The effect is real and a bit larger than the title says. 4524-file project, cold, released 2.2.14 phar, PHP 8.5.8, pcov 1.0.12 built locally, 3 interleaved rounds, medians. I asserted the pcov state in all 15 processes with a
So pcov costs +50% wall and +49% CPU, and turning it off gets all of it back. @ondrejmirtes, that also answers your question on Three constraints that decide the shape, all checked here:
Workers are forked on this machine, on the phar and on a source checkout alike. The phar already restarts itself unconditionally to load the turbo extension, and it does so under What I did not check: any of this on Linux. kimai's CI is where the report came from. Fork needs pcntl and a turbo binary for that platform, so the restart picture there may differ. |
Good news, I'm on Linux. |
|
just adding a side-note: might be worth to also restart for the |
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?