Skip to content

Skip rewriting an unchanged result cache #6432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
zonuexe wants to merge 3 commits into
base: 2.2.x
Choose a base branch
from
+86 −5
Open

Skip rewriting an unchanged result cache #6432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4040388
Skip rewriting unchanged result cache
zonuexe Sep 13, 2026
b0d397f
fixup! Skip rewriting unchanged result cache
zonuexe Sep 13, 2026
6a243c4
fixup! Skip rewriting unchanged result cache
zonuexe Sep 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions .github/workflows/e2e-tests.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,10 +128,17 @@ jobs:
cd e2e/result-cache-atomic-save
../../bin/phpstan
INODE=$(ls -i tmp/resultCache.php | awk '{print $1}')
OUTPUT=$(../../bin/phpstan -vvv 2>&1)
echo "$OUTPUT"
UNCHANGED=$([ "$INODE" = "$(ls -i tmp/resultCache.php | awk '{print $1}')" ] && echo unchanged || echo replaced)
../bashunit -a equals 'unchanged' "$UNCHANGED"
../bashunit -a contains 'Result cache was not rewritten because it is unchanged.' "$OUTPUT"
echo >> src/Foo.php
../../bin/phpstan
# The cache is written next to its final path and renamed into place, so saving it
# again replaces the file rather than truncating and rewriting the one the next run
# reads. That is what keeps a run killed mid-save from leaving a partial cache behind.
# after a change replaces the file rather than truncating and rewriting the one the
# next run reads. That is what keeps a run killed mid-save from leaving a partial
# cache behind.
REPLACED=$([ "$INODE" != "$(ls -i tmp/resultCache.php | awk '{print $1}')" ] && echo replaced || echo 'written in place')
../bashunit -a equals 'replaced' "$REPLACED"
# A completed save leaves nothing next to the cache file.
Expand Down Expand Up @@ -373,11 +380,13 @@ jobs:
echo "$OUTPUT"
../bashunit -a contains 'Result cache restored. 0 files will be reanalysed.' "$OUTPUT"
../bashunit -a not_contains 'Scanned files with changed symbols' "$OUTPUT"
../bashunit -a contains 'Result cache is saved.' "$OUTPUT"
# The new contents are recorded: the next run has nothing to notice.
OUTPUT=$(../bashunit -a exit_code "1" "../../bin/phpstan analyse -vv --error-format raw")
echo "$OUTPUT"
../bashunit -a not_contains 'Scanned files with changed symbols' "$OUTPUT"
../bashunit -a contains 'Result cache restored. 0 files will be reanalysed.' "$OUTPUT"
../bashunit -a contains 'Result cache was not rewritten because it is unchanged.' "$OUTPUT"
# A changed return type is an exported node change: the file depending on it is
# re-analysed. A changed signature cannot make a new symbol available, so the file with
# the error is left alone.
Expand Down
48 changes: 45 additions & 3 deletions src/Analyser/ResultCache/ResultCacheManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,11 @@ final class ResultCacheManager
/** @var array<string, true> */
private array $alreadyProcessed = [];

private bool $restoredCacheUnchanged = false;

/** @var array<string, string> */
private array $restoredStubFiles = [];

/**
* @param string[] $analysedPaths
* @param string[] $analysedPathsFromConfig
Expand Down Expand Up @@ -268,6 +273,9 @@ private function fullAnalysis(
*/
public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?array $projectConfigArray, Output $output): ResultCache
{
$this->restoredCacheUnchanged = false;
$this->restoredStubFiles = [];

$startTime = microtime(true);
$currentFileHashes = [];
foreach ($allAnalysedFiles as $analysedFile) {
Expand Down Expand Up @@ -384,7 +392,8 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
// make anything be re-analysed. A scanned file can change without any of its symbols changing,
// and then nothing here has to happen at all.
$scannedFilesWithChangedSymbols = [];
if ($this->isMetaDifferent($data['meta'], $meta)) {
$metaDifferent = $this->isMetaDifferent($data['meta'], $meta);
if ($metaDifferent) {
$diffs = $this->getMetaKeyDifferences($data['meta'], $meta);

// Some metadata differences do not invalidate the whole analysis, because the code they
Expand Down Expand Up @@ -631,6 +640,7 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
$filteredCollectedData = [];
$filteredExportedNodes = [];
$newFileAppeared = false;
$dependencyFilesChanged = false;

foreach (array_keys($cachedStubFiles) as $stubFile) {
if (!array_key_exists($stubFile, $errors)) {
Expand Down Expand Up @@ -748,6 +758,7 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
}

if (is_file($notAnalysedFile) && $wasMissing) {
$dependencyFilesChanged = true;
// It exists now. Whether it holds any symbol is beside the point - a file that was named
// and was not there is now there, and that alone changes what the analysis says.
$invertedDependenciesToReturn[$notAnalysedFile] = $dependentFiles;
Expand All @@ -773,6 +784,7 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
continue;
}

$dependencyFilesChanged = true;
// Edited: the same rule as for an analysed file. Nothing the files depending on it can
// see changed unless its exported nodes did, so a body-only edit re-analyses nothing -
// except the classes using a trait declared here, whose body is analysed in their
Expand Down Expand Up @@ -810,6 +822,8 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
}

$dependentFiles = array_merge($dependentFiles, $usedTraitDependentFiles);
} else {
$dependencyFilesChanged = true;
}

foreach ($dependentFiles as $dependentFile) {
Expand Down Expand Up @@ -857,6 +871,9 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
));
}

$this->restoredCacheUnchanged = !$metaDifferent && !$dependencyFilesChanged;
$this->restoredStubFiles = $cachedStubFiles;

return new ResultCache(
filesToAnalyse: $filesToAnalyse,
fullAnalysis: false,
Expand Down Expand Up @@ -1059,7 +1076,30 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache
}
}

$this->save($resultCache->getLastFullAnalysisTime(), $errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, $dependencies, $usedTraitDependencies, $packageDependencies, $exportedNodes, $projectExtensionFiles, $resultCache->getCurrentFileHashes(), $meta);
$stubFiles = $this->getStubFiles();
if (
!$resultCache->isFullAnalysis()
&& $resultCache->getFilesToAnalyse() === []
&& $this->restoredCacheUnchanged
&& $errorsByFile === $resultCache->getErrors()
&& $locallyIgnoredErrorsByFile === $resultCache->getLocallyIgnoredErrors()
&& $linesToIgnore === $resultCache->getLinesToIgnore()
&& $unmatchedLineIgnores === $resultCache->getUnmatchedLineIgnores()
&& $collectedDataByFile === $resultCache->getCollectedData()
&& $packageDependencies === $resultCache->getPackageDependencies()
&& $exportedNodes === $resultCache->getExportedNodes()
&& $projectExtensionFiles === $resultCache->getProjectExtensionFiles()
&& $stubFiles === $this->restoredStubFiles
&& is_file($this->cacheFilePath)
) {
if ($output->isVeryVerbose()) {
$output->writeLineFormatted('Result cache was not rewritten because it is unchanged.');
}

return true;
}

$this->save($resultCache->getLastFullAnalysisTime(), $errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, $dependencies, $usedTraitDependencies, $packageDependencies, $exportedNodes, $projectExtensionFiles, $resultCache->getCurrentFileHashes(), $meta, $stubFiles);

if ($output->isVeryVerbose()) {
$output->writeLineFormatted('Result cache is saved.');
Expand Down Expand Up @@ -1385,6 +1425,7 @@ private function mergeUnmatchedLineIgnores(ResultCache $resultCache, array $fres
* @param array<string, array{string, bool, string}> $projectExtensionFiles
* @param array<string, string> $currentFileHashes
* @param mixed[] $meta
* @param array<string, string> $stubFiles
*/
private function save(
int $lastFullAnalysisTime,
Expand All @@ -1400,6 +1441,7 @@ private function save(
array $projectExtensionFiles,
array $currentFileHashes,
array $meta,
array $stubFiles,
): void
{
$invertedDependencies = [];
Expand Down Expand Up @@ -1476,7 +1518,7 @@ private function save(
// The only point where the StubFilesExtensions may run: the analysis is over, bootstrapFiles
// have been executed, so the extensions can rely on them. restore() reads these hashes back
// instead of running the extensions again.
$meta['stubFiles'] = $this->getStubFiles();
$meta['stubFiles'] = $stubFiles;

// Store paths relative to the anchor so the cache survives a change of the project's absolute
// path prefix (a fresh CI checkout dir, a git worktree). projectConfig inside $meta is already a
Expand Down
30 changes: 30 additions & 0 deletions tests/PHPStan/Command/ResultCacheInfoCommandTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,15 @@
use PHPUnit\Framework\Attributes\Group;
use PHPUnit\Framework\TestCase;
use function array_map;
use function clearstatcache;
use function escapeshellarg;
use function exec;
use function filemtime;
use function implode;
use function md5;
use function sprintf;
use function sys_get_temp_dir;
use function touch;
use function uniqid;
use const PHP_BINARY;

Expand Down Expand Up @@ -141,6 +144,33 @@ public function testChangedFileIsCountedAsFileToAnalyse(): void
$this->assertSame(1, $json['filesToAnalyseCount']);
}

public function testWarmAnalysisDoesNotRewriteUnchangedResultCache(): void
{
[$analyseOutput, $analyseExitCode] = $this->runPhpstan(['analyse', '--no-progress']);
$this->assertSame(0, $analyseExitCode, $analyseOutput);

$resultCachePath = $this->projectDir . '/tmp/resultCache.php';
$oldModificationTime = 946684800;
$this->assertTrue(touch($resultCachePath, $oldModificationTime));

[$warmOutput, $warmExitCode] = $this->runPhpstan(['analyse', '--no-progress', '-vvv']);
$this->assertSame(0, $warmExitCode, $warmOutput);
$this->assertStringContainsString('Result cache was not rewritten because it is unchanged.', $warmOutput);
clearstatcache(true, $resultCachePath);
$this->assertSame($oldModificationTime, filemtime($resultCachePath));

FileSystem::write(
$this->projectDir . '/src/Foo.php',
FileSystem::read($this->projectDir . '/src/Foo.php') . "\n",
);

[$changedOutput, $changedExitCode] = $this->runPhpstan(['analyse', '--no-progress', '-vvv']);
$this->assertSame(0, $changedExitCode, $changedOutput);
$this->assertStringContainsString('Result cache is saved.', $changedOutput);
clearstatcache(true, $resultCachePath);
$this->assertNotSame($oldModificationTime, filemtime($resultCachePath));
}

public function testChangedLevelInvalidatesResultCache(): void
{
[$analyseOutput, $analyseExitCode] = $this->runPhpstan(['analyse', '--no-progress']);
Expand Down
Loading