Repository navigation
Speed up single-file and editor-mode runs: skip unsaved exported nodes and OPcache that can't pay off - #6988
Open
ondrejmirtes wants to merge 3 commits into
Open
ondrejmirtes wants to merge 3 commits into
ondrejmirtes wants to merge 3 commits into
Conversation
…ache is saved The full-analysis save callback fetched them before doSave() decided not to save at all - a run on files passed on the command line or in editor mode never saves - so every such run parsed each file the analysed code depends on for nothing. doSave() now receives them as a callback and calls it only once the cache is known to be saved. WordPress src/index.php, which requires four files: 0.98 s -> 0.70 s with --debug and PHPSTAN_RESTART=none. Amp-Thread-ID: https://ampcode.com/threads/T-01a1b250-30d4-72bb-8d3e-b0687619f435
Compiling PHPStan into OPcache costs about half a second per process, mostly the optimizer, which only a long enough analysis earns back. Since the restart activated it, a single-file run paid that twice - in the main process and in the spawned worker - and took twice as long as before. - The restart no longer activates OPcache in the main process when the workers will be spawned (a phar without the turbo extension): only forked workers use what the main process compiled, otherwise it just boots and collects results, and was measured slower with OPcache at every size. - Nor when the command line names at most OpcacheSettings::FEW_FILES_LIMIT files and nothing else to analyse - `analyse src/Foo.php`, the way editors run PHPStan. The restart still happens for the other reasons (turbo, xdebug, ...). - Spawned analysis workers get the OPcache entries only when there are more than that many files to analyse - which also covers an incremental run re-analysing a few files. e2e/phar-run analyses 11 files so its workers keep asserting the OPcache they get, and a new phar-run step asserts a single-file run neither restarts nor spawns its worker with OPcache. The bug-15184 and bootstrap-autoloader-wrong-file spawned runs get OPcache from the php.ini, the crash they guard against needs it. Amp-Thread-ID: https://ampcode.com/threads/T-01a1b250-30d4-72bb-8d3e-b0687619f435
…ests A change to phar.yml makes the PHAR checksum and prefix-diff jobs compare the PR's PHAR with the base one, which only a compiler-only change passes. The restart-manager row checks the same from source and also covers forked workers: a single file analysed with OPcache dormant restarts nothing, and neither the forked nor the spawned worker runs with OPcache. It fails on the base commit. Amp-Thread-ID: https://ampcode.com/threads/T-01a1b250-30d4-72bb-8d3e-b0687619f435
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.
Single-file and editor-mode runs got about twice as slow after 2.2.13. There are two causes, one per commit.
Exported nodes fetched for a cache that is never saved. Since 3eb12b6, the full-analysis save callback in
ResultCacheManager::process()calledaddNonAnalysedExportedNodes()beforedoSave()checked whether it saves anything. A run on files named on the command line, or in editor mode, never saves, so it parsed every non-analysed file the analysed code depends on for nothing.doSave()now receives the nodes as a callback and calls it only after the "cache won't be saved" checks. On WordPresssrc/index.php, which requires 4 files: 0.98 s → 0.70 s with--debug PHPSTAN_RESTART=none.OPcache activated where it can't pay off. Compiling PHPStan into OPcache costs about 0.45 s per process, mostly the optimizer. The restart and the spawned workers paid that on every run, even a one-file run:
OpcacheSettings::FEW_FILES_LIMIT(10) files and no directory, e.g.analyse src/Foo.php, the way editors run PHPStan. The restart still happens for the other reasons (turbo, xdebug, pcov, uopz).Break-even, bare phar with spawned workers, WordPress
wp-includesfiles (heavy ones): OPcache costs 0.4 s for 1 file, 0.35 s for 5 files, and gains 0.24 s at 20 files (9.4 s total). So 10 is a conservative limit.Large runs are not slower. All of
src/wp-includes/*.php(263 files, best of 2): bare phar 25.5 s → 21.5 s, with turbo 12.7 s → 11.1 s. The first commit accounts for part of that, since a run on named files never saves the cache.Wall time, best of 5 (one warm-up run discarded)
analyse src/index.phpanalyse -l 5 test.phpsrc/index.php, turbo binary next to the pharSetup: PHP 8.4.27 NTS on Linux x86_64 with 16 cores, OPcache loaded but dormant on CLI (the default). 2.1.34 and 2.3.0 are the release PHARs. 2.3.x-dev (8af6dd7) and this PR were built the same way (
compiler/bin/prepare+ box +resign.php, as in phar.yml). The first two rows use bare PHARs with no turbo binary, so their workers are spawned. The turbo rows put a locally builtphpstan_turbo.sonext to the PHAR, which is how the Composer package ships it, so their workers are forked. wordpress-develop uses its ownphpstan.neon.dist(level 5). For 2.1.34 it ran in a copy with phpstan-phpunit 2.0.12 / phpstan-strict-rules 2.0.7 andphpVersion.max: 80599, because the current versions require 2.3.Tests
RestartManagerTest: the command-line heuristic (option values,--, directories, the limit,--watch/--pro, other commands).PHP_INI_SCAN_DIR). Without that, these one-file runs would no longer have OPcache and would pass whether or not the bug is fixed. I checked locally that the step still reproduces the crash onceinvalidateIncludedFiles()is disabled.make phpstan,make cs,tests/PHPStan/{Analyser/ResultCache,Process,Parallel,Command,Build/PreAutoloadFilesTest}, and the restart-manager e2e opcache and--debugrows.