From c27d50523173b2980966ece70e08d89b1fa605f9 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Thu, 2 Jul 2026 21:21:24 +0200 Subject: [PATCH 1/2] Store the result cache in a framed serialize() format instead of a var_export'd PHP file The result cache is written as a var_export'd PHP file and hydrated with `include`. Including a multi-megabyte PHP source has a hidden cost: its compiled op_arrays and interned strings stay retained for the process lifetime. This switches the file to serialize()/unserialize(), which produces only the values. The `errorsCallback`/`collectedDataCallback`/`exportedNodesCallback` closures existed to embed object graphs in the PHP file; `restore()` invoked all of them unconditionally right after the include, so plain entries are equivalent. The file is written frame by frame, and the array sections entry by entry, mirroring what the writer this replaces already did. That is not incidental: serializing the payload in one call holds the whole cache in memory twice over, and on the first project below that is 53 MB, 39 MB of it exportedNodes alone, so per-section streaming would not be enough either. The reading side is framed for the same reason, so restoring never holds the file and the values at once. Format: `` on the first line, then `name length` or `name* count` headers, each followed by length-prefixed serialized payloads. Each array entry is serialized as a single-element array so its key travels with it, which keeps string and integer keys distinct. ## Memory `memory_get_peak_usage(true)` on the main process, two projects, interleaved A/B. Every figure repeated across rounds and identical each time. | | | cold peak | warm peak | cache on disk | | --- | --- | --- | --- | --- | | doctrine/symfony, 4520 files | base | 154.5 MB | 320.1 MB | 39.6 MB | | | this change | 126.5 MB | 178.0 MB | 52.8 MB | | | | -18.1% | -44.4% | +33% | | Laravel-ecosystem vendor tree, 7701 files | base | 278.0 MB | 670.5 MB | 104.2 MB | | | this change | 219.0 MB | 330.0 MB | 132.3 MB | | | | -21.2% | -50.8% | +27% | A warm run that reanalyses a changed file peaks the same as a pure restore (178.0 MB against base's 320.1 MB on the first project), because the peak is the cache restore rather than the reanalysis. ## CPU Unchanged where the analysis dominates, cheaper where the cache does. First project, best of three: | | cold wall | cold CPU | warm wall | warm CPU | | --- | --- | --- | --- | --- | | base | 20.05s | 127.73s | 1.76s | 1.50s | | this change | 20.18s | 127.70s | 1.49s | 1.36s | The per-entry framing adds roughly 4500 serialize() calls on save and is invisible against a 128s analysis; the warm restore gets faster. ## Format transition No cache version bump is needed, and both directions were exercised with real builds: - upgrade: an old-format PHP file has no framed header, so it is discarded exactly like a corrupted cache file today (verbose notice unchanged, then a full analysis). - downgrade: the payload sits after ``, so an older PHPStan including the new file returns null immediately. Verified on a 55 MB cache file: it returns NULL and echoes 0 bytes to stdout. Without the prefix `include` would print the whole payload as inline text and wreck CI logs and machine-readable formats. ## Verification - Output byte-identical to base: 3267 raw error lines, cold and warm, on the first project. - All 22 result-cache e2e scenarios from the workflow run locally: 20 pass. The two that do not, result-cache-5 and result-cache-restore-without-reflection, fail identically on unmodified base; the latter fails in container construction for environment reasons unrelated to the cache. - Full test suite (21323 tests), self-analysis and coding standard clean. Measured on one machine (macOS, arm64). Retained-memory behaviour can differ on Linux, so the CI run is worth reading rather than assuming these ratios carry over. --- .../ResultCache/ResultCacheManager.php | 210 +++++++++++++----- 1 file changed, 155 insertions(+), 55 deletions(-) diff --git a/src/Analyser/ResultCache/ResultCacheManager.php b/src/Analyser/ResultCache/ResultCacheManager.php index 51736cb0292..8a09f255f5d 100644 --- a/src/Analyser/ResultCache/ResultCacheManager.php +++ b/src/Analyser/ResultCache/ResultCacheManager.php @@ -42,7 +42,9 @@ use function error_get_last; use function explode; use function fclose; +use function fgets; use function fopen; +use function fread; use function fwrite; use function get_loaded_extensions; use function getenv; @@ -53,13 +55,17 @@ use function is_file; use function ksort; use function microtime; +use function rtrim; +use function serialize; use function sort; use function sprintf; +use function str_ends_with; use function str_starts_with; +use function strlen; use function substr; use function time; use function unlink; -use function var_export; +use function unserialize; use const PHP_VERSION_ID; /** @@ -72,6 +78,15 @@ final class ResultCacheManager private const CACHE_VERSION = 'v13-packageDependencies'; + /** + * The cache file is serialize() output, but an older PHPStan reading it would + * include it as PHP and echo the whole multi-megabyte content to stdout as + * inline text before discarding it. This prefix makes such an include return + * null immediately (the text after ?> is never reached), so a downgrade + * degrades to a silent full analysis instead. + */ + private const SERIALIZED_FILE_PREFIX = ''; + /** @var array */ private array $fileHashes = []; @@ -211,7 +226,12 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? } try { - $data = require $cacheFilePath; + // The cache used to be a var_export'd PHP file loaded via include. Including a + // multi-megabyte PHP source retains its compiled op_arrays and interned strings + // for the process lifetime; unserialize() produces only the values. A cache file + // in the old PHP format fails to unserialize and is discarded below like any + // other corrupted file, so no cache version bump is needed for the transition. + $data = $this->readCacheFile($cacheFilePath); } catch (Throwable $e) { if ($output->isVeryVerbose()) { $output->writeLineFormatted(sprintf('Result cache not used because an error occurred while loading the cache file: %s', $e->getMessage())); @@ -449,12 +469,12 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? $filesToAnalyse = []; $invertedDependenciesToReturn = []; $invertedUsedTraitDependenciesToReturn = []; - $errors = $data['errorsCallback'](); - $locallyIgnoredErrors = $data['locallyIgnoredErrorsCallback'](); + $errors = $data['errors']; + $locallyIgnoredErrors = $data['locallyIgnoredErrors']; $linesToIgnore = $data['linesToIgnore']; $unmatchedLineIgnores = $data['unmatchedLineIgnores']; - $collectedData = $data['collectedDataCallback'](); - $exportedNodes = $data['exportedNodesCallback'](); + $collectedData = $data['collectedData']; + $exportedNodes = $data['exportedNodes']; $filteredErrors = []; $filteredLocallyIgnoredErrors = []; $filteredLinesToIgnore = []; @@ -1212,9 +1232,11 @@ private function save( $file = $this->cacheFilePath; - // streamed to the file section by section - building the whole - // var_export()ed contents in memory at once would take up roughly - // twice the size of the resulting file in the main process + // Written frame by frame, and the array sections entry by entry, so the peak cost of saving is + // one entry rather than the whole cache. Serializing the payload in one call would hold the + // entire cache in memory twice over - on one project that is 53 MB serialized, 39 MB of it + // exportedNodes alone - which is the same trap the var_export writer this replaces avoided by + // streaming. $handle = @fopen($file, 'w'); if ($handle === false) { $error = error_get_last(); @@ -1222,38 +1244,18 @@ private function save( } try { - $this->writeToHandle($handle, $file, " " . var_export($lastFullAnalysisTime, true) . ", - 'meta' => " . var_export($meta, true) . ", - 'projectExtensionFiles' => " . var_export($projectExtensionFiles, true) . ", - 'errorsCallback' => static function (): array { return "); - $this->streamArrayVarExportToHandle($handle, $file, $errors); - $this->writeToHandle($handle, $file, "; }, - 'locallyIgnoredErrorsCallback' => static function (): array { return "); - $this->streamArrayVarExportToHandle($handle, $file, $locallyIgnoredErrors); - $this->writeToHandle($handle, $file, "; }, - 'linesToIgnore' => "); - $this->streamArrayVarExportToHandle($handle, $file, $linesToIgnore); - $this->writeToHandle($handle, $file, ", - 'unmatchedLineIgnores' => "); - $this->streamArrayVarExportToHandle($handle, $file, $unmatchedLineIgnores); - $this->writeToHandle($handle, $file, ", - 'collectedDataCallback' => static function (): array { return "); - $this->streamArrayVarExportToHandle($handle, $file, $collectedData); - $this->writeToHandle($handle, $file, "; }, - 'dependencies' => "); - $this->streamArrayVarExportToHandle($handle, $file, $invertedDependencies); - $this->writeToHandle($handle, $file, ", - 'packageDependencies' => "); - $this->streamArrayVarExportToHandle($handle, $file, $packageDependencies); - $this->writeToHandle($handle, $file, ", - 'exportedNodesCallback' => static function (): array { return "); - $this->streamArrayVarExportToHandle($handle, $file, $exportedNodes); - $this->writeToHandle($handle, $file, '; }, -]; -'); + $this->writeToHandle($handle, $file, self::SERIALIZED_FILE_PREFIX . "\n"); + $this->writeValueFrame($handle, $file, 'lastFullAnalysisTime', $lastFullAnalysisTime); + $this->writeValueFrame($handle, $file, 'meta', $meta); + $this->writeValueFrame($handle, $file, 'projectExtensionFiles', $projectExtensionFiles); + $this->writeArrayFrame($handle, $file, 'errors', $errors); + $this->writeArrayFrame($handle, $file, 'locallyIgnoredErrors', $locallyIgnoredErrors); + $this->writeArrayFrame($handle, $file, 'linesToIgnore', $linesToIgnore); + $this->writeArrayFrame($handle, $file, 'unmatchedLineIgnores', $unmatchedLineIgnores); + $this->writeArrayFrame($handle, $file, 'collectedData', $collectedData); + $this->writeArrayFrame($handle, $file, 'dependencies', $invertedDependencies); + $this->writeArrayFrame($handle, $file, 'packageDependencies', $packageDependencies); + $this->writeArrayFrame($handle, $file, 'exportedNodes', $exportedNodes); } finally { fclose($handle); } @@ -1271,30 +1273,128 @@ private function writeToHandle($handle, string $file, string $contents): void } /** - * Streams the var_export() representation of an array to the file entry - * by entry, producing output byte-identical to var_export($values, true). + * A single value, as `name length\n` followed by that many bytes. * - * var_export() builds the whole export in memory even when told to print it, - * so exporting a big section in one call would take up as much memory - * as the resulting file section itself. + * @param resource $handle + */ + private function writeValueFrame($handle, string $file, string $name, mixed $value): void + { + $blob = serialize($value); + $this->writeToHandle($handle, $file, $name . ' ' . strlen($blob) . "\n"); + $this->writeToHandle($handle, $file, $blob); + } + + /** + * An array, as `name* count\n` followed by one length-prefixed frame per entry. * - * Each entry is exported wrapped in a single-entry array whose "array (\n" - * prefix and "\n)" suffix are stripped, yielding the same bytes (including - * indentation) the entry would get inside the full export. Indenting the lines - * of a standalone value export would corrupt multi-line string contents instead. + * Each entry is serialized as a single-element array so its key travels with it, which keeps string + * and integer keys distinct without a second frame for the key. * * @param resource $handle * @param array $values */ - private function streamArrayVarExportToHandle($handle, string $file, array $values): void + private function writeArrayFrame($handle, string $file, string $name, array $values): void { - $this->writeToHandle($handle, $file, 'array ('); + $this->writeToHandle($handle, $file, $name . '* ' . count($values) . "\n"); foreach ($values as $key => $value) { - $entry = var_export([$key => $value], true); - $this->writeToHandle($handle, $file, "\n" . substr($entry, 8, -2)); + $blob = serialize([$key => $value]); + $this->writeToHandle($handle, $file, strlen($blob) . "\n"); + $this->writeToHandle($handle, $file, $blob); } + } + + /** + * Read a framed cache file back, one frame at a time. + * + * Returns null for anything that is not this format, which is how a cache written by an older + * PHPStan is detected: the caller discards it and analyses everything, exactly as it does for a + * corrupted file. + * + * @return array|null + */ + private function readCacheFile(string $cacheFilePath): ?array + { + $handle = @fopen($cacheFilePath, 'r'); + if ($handle === false) { + return null; + } + + try { + if (rtrim((string) fgets($handle), "\n") !== self::SERIALIZED_FILE_PREFIX) { + return null; + } + + $data = []; + while (($header = fgets($handle)) !== false) { + $header = rtrim($header, "\n"); + if ($header === '') { + continue; + } + + $parts = explode(' ', $header, 2); + if (count($parts) !== 2) { + return null; + } + + [$name, $size] = $parts; + if (!str_ends_with($name, '*')) { + $data[$name] = $this->readFrame($handle, (int) $size); + + continue; + } + + $entries = $this->readEntryFrames($handle, (int) $size); + if ($entries === null) { + return null; + } + + $data[substr($name, 0, -1)] = $entries; + } + + return $data; + } finally { + fclose($handle); + } + } + + /** + * @param resource $handle + * @return array|null + */ + private function readEntryFrames($handle, int $count): ?array + { + $entries = []; + for ($i = 0; $i < $count; $i++) { + $length = fgets($handle); + if ($length === false) { + return null; + } + + $entry = $this->readFrame($handle, (int) rtrim($length, "\n")); + if (!is_array($entry)) { + return null; + } + + foreach ($entry as $key => $value) { + $entries[$key] = $value; + } + } + + return $entries; + } + + /** + * @param resource $handle + */ + private function readFrame($handle, int $length): mixed + { + if ($length <= 0) { + return null; + } + + $blob = fread($handle, $length); - $this->writeToHandle($handle, $file, "\n)"); + return $blob === false ? null : @unserialize($blob); } /** From de278ebb35897d7a2369ace575b4ae34e1ac7cda Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Wed, 19 Aug 2026 11:03:56 +0200 Subject: [PATCH 2/2] Discard a damaged framed cache instead of reading it half way A cache file that is this format but truncated - which is what a process killed during the save leaves behind, since the file is streamed to its final path - was read as far as it went and the missing frames returned as null. `is_array()` on the assembled array then passed, and the first missing section surfaced as a TypeError from isMetaDifferent() far away from the cause. Every format violation now throws, so restore() discards the file and analyses everything, which is what the var_export format did by way of a ParseError from the include. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/e2e-tests.yml | 5 ++ .gitignore | 1 + e2e/result-cache-truncated/phpstan.neon | 5 ++ e2e/result-cache-truncated/src/Bar.php | 13 ++++++ e2e/result-cache-truncated/src/Foo.php | 13 ++++++ e2e/result-cache-truncated/truncate.php | 13 ++++++ .../ResultCache/ResultCacheManager.php | 46 +++++++++++++------ 7 files changed, 83 insertions(+), 13 deletions(-) create mode 100644 e2e/result-cache-truncated/phpstan.neon create mode 100644 e2e/result-cache-truncated/src/Bar.php create mode 100644 e2e/result-cache-truncated/src/Foo.php create mode 100644 e2e/result-cache-truncated/truncate.php diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index b9df7364e26..952cb69cf17 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -124,6 +124,11 @@ jobs: mv src/Foo.php.orig src/Foo.php echo -n > phpstan-baseline.neon ../../bin/phpstan -vvv + - script: | + cd e2e/result-cache-truncated + ../../bin/phpstan -vvv + php truncate.php + ../../bin/phpstan -vvv - script: | cd e2e/bug-14514 composer install diff --git a/.gitignore b/.gitignore index a19ae4998f6..a2cdd4a7433 100644 --- a/.gitignore +++ b/.gitignore @@ -15,3 +15,4 @@ tmp/.memory_limit e2e/bashunit /.phpbench +/e2e/result-cache-truncated/tmp diff --git a/e2e/result-cache-truncated/phpstan.neon b/e2e/result-cache-truncated/phpstan.neon new file mode 100644 index 00000000000..6daefc8c39c --- /dev/null +++ b/e2e/result-cache-truncated/phpstan.neon @@ -0,0 +1,5 @@ +parameters: + level: 8 + tmpDir: tmp + paths: + - src diff --git a/e2e/result-cache-truncated/src/Bar.php b/e2e/result-cache-truncated/src/Bar.php new file mode 100644 index 00000000000..8b38f8ea2a6 --- /dev/null +++ b/e2e/result-cache-truncated/src/Bar.php @@ -0,0 +1,13 @@ +doBar(); + } + +} diff --git a/e2e/result-cache-truncated/truncate.php b/e2e/result-cache-truncated/truncate.php new file mode 100644 index 00000000000..ad3b7e98508 --- /dev/null +++ b/e2e/result-cache-truncated/truncate.php @@ -0,0 +1,13 @@ +readEntryFrames($handle, (int) $size); - if ($entries === null) { - return null; - } - - $data[substr($name, 0, -1)] = $entries; + $data[substr($name, 0, -1)] = $this->readEntryFrames($handle, (int) $size); } return $data; @@ -1359,20 +1355,20 @@ private function readCacheFile(string $cacheFilePath): ?array /** * @param resource $handle - * @return array|null + * @return array */ - private function readEntryFrames($handle, int $count): ?array + private function readEntryFrames($handle, int $count): array { $entries = []; for ($i = 0; $i < $count; $i++) { $length = fgets($handle); if ($length === false) { - return null; + throw new RuntimeException(sprintf('Cache file ended after %d of %d entries.', $i, $count)); } $entry = $this->readFrame($handle, (int) rtrim($length, "\n")); if (!is_array($entry)) { - return null; + throw new RuntimeException('An entry frame did not contain an array.'); } foreach ($entry as $key => $value) { @@ -1384,17 +1380,41 @@ private function readEntryFrames($handle, int $count): ?array } /** + * A frame's payload, or an exception when the file does not hold one. + * + * Every failure here means a cache file that is this format but damaged - a process killed + * mid-save leaves exactly that, since the file is written in place. restore() turns the + * exception into a discarded cache and a full analysis, the same way it handles the parse + * error an incomplete var_export'd file used to produce. Returning a value instead would + * hand a half-read cache to the caller, where the missing pieces surface as type errors far + * from the cause. + * + * false is treated as failure because unserialize() reports failure that way and no value in + * the cache is a bare false: the sections are arrays and lastFullAnalysisTime is an int. + * * @param resource $handle */ private function readFrame($handle, int $length): mixed { if ($length <= 0) { - return null; + throw new RuntimeException(sprintf('Frame length %d is not positive.', $length)); } $blob = fread($handle, $length); + if ($blob === false || strlen($blob) !== $length) { + throw new RuntimeException(sprintf( + 'Expected a %d byte frame, read %d bytes.', + $length, + $blob === false ? 0 : strlen($blob), + )); + } + + $value = @unserialize($blob); + if ($value === false) { + throw new RuntimeException(sprintf('A %d byte frame could not be unserialized.', $length)); + } - return $blob === false ? null : @unserialize($blob); + return $value; } /**