Repository navigation
Report worker peak memory when the result cache is used - #6576
Merged
Merged
Conversation
When only some files are analysed again, -vvv shows only the main process's peak memory and drops the workers' peak. This test fails until that is fixed.
When only some files were analysed again, the result was rebuilt from the result cache without the worker count. -vvv then showed only the main process's peak, even though workers had run. Editor mode rebuilt the result the same way.
Contributor
|
//cc @SanderMuller please review :) |
staabm
approved these changes
Sep 24, 2026
SanderMuller
approved these changes
Sep 24, 2026
SanderMuller
left a comment
SanderMuller
left a comment
Contributor
There was a problem hiding this comment.
Thanks, this looks good.
- With the fix, the second run of the e2e fixture prints
Peak memory: 14 MB (main process), 22 MB (largest of 2 forked workers). With the basesrcit prints onlyPeak memory: 14 MB. - I also checked editor mode by hand, with
--tmp-file/--instead-ofafter a partial restore. It shows the worker line with the fix, and loses it again when only theAnalyseApplication.phpchange is reverted. - The other
new AnalyserResult(...)calls withoutworkerCountare the empty-file and non-parallel paths, where 0 is correct. So these two are the only copies. - The full suite,
make phpstanand phpcs on both files pass. Warm-run CPU on a real project is the same, 1.64–1.67 s before and 1.64–1.75 s after.
The three reds that other 2.2.x PRs do not have are not from this change. The phpbench harness does not reach ResultCacheManager or AnalyseApplication, and the two jobs fail on different variants. The IntersectionTypeTest::testIsAcceptedBy failures on Windows with old PHPUnit also happened today on two unrelated 2.3.x PRs.
Contributor
|
Thank you! |
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
3 participants
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.
With
-vvv, PHPStan prints the peak memory of the main process and of the largest worker. When the result cache is partly restored, the worker part is missing, even though workers ran.Cold run on 4 files:
Next run after changing 2 of them:
ResultCacheManager::process()rebuilds theAnalyserResultfor this case and does not passworkerCount. It falls back to 0, soInceptionResultprints only the main process.AnalyseApplication::switchTmpFileInAnalyserResult()drops it the same way in editor mode. Both came with #6297. The fix passesworkerCountthrough in both places. I did not add a test for editor mode.The first commit only adds an e2e test. I pushed the two commits one after the other to 2.2.x in my fork, so you can see the test fail before the fix:
Feel free to squash.