Repository navigation
Spawn the PHPStan Pro worker instead of forking it - #6926
Merged
Merged
Conversation
FixerApplication runs for as long as PHPStan Pro does. A fixer:worker forked from it inherited the container and the config it started with, so an edit of a config file, like errors added to the baseline, was noticed by the file monitor but the reanalysis still used the old ignoreErrors and parameters. A spawned fixer:worker reads the config files again on every analysis. It still forks its own analysis workers where that is supported, see ParallelAnalyser. The validation of the ignoreErrors in FixerApplication is gone too: it checked the config the process started with. fixer:worker validates the current one. Amp-Thread-ID: https://ampcode.com/threads/T-01a1a198-fa57-7583-9394-63372a7e2fa8
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
FixerApplicationruns for as long as PHPStan Pro does. Since #5663 it forksfixer:workerfrom itself when forking is supported. The forked worker inherits the container and config the process started with. When a config file changes, such as errors added to the baseline or anignoreErrorsentry removed, the file monitor notices and a reanalysis starts, but it still uses the oldignoreErrorsand parameters. The errors PHPStan Pro just added to the baseline came back as reported.This PR always spawns
fixer:worker, so it reads the config files again on every analysis. The analysis inside it still forks its own workers throughParallelAnalyser, so the heavy work keeps the fork speedup. The extra cost is one container boot per analysis.Changes:
FixerApplication::analyse()always usesSpawnedProcessPromise.ForkedProcessPromiseand theProcessPromiseinterface are removed, because nothing else used them.FixerWorkerRunnerstays as the worker logic behindFixerWorkerCommand.ignoreErrors, because it only ever checked the startup config.fixer:workeralready validates the current config.Verified by running
--prowith forking on (pcntl, no JIT):phpstan-baseline.neonwere reported again after the reanalysis.fixer:worker, which forked its analysis worker.