From 0bbaa98ddb27c06e655711acf48779c09f9db209 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Wed, 2 Sep 2026 17:17:27 +0200 Subject: [PATCH 1/2] Store the result cache in a framed serialize() format instead of a var_export'd PHP file The cache is a var_export'd PHP file hydrated with include. Including a multi-megabyte PHP source retains its compiled op_arrays and interned strings for the process lifetime, and where OPCache is on it also occupies shared memory: on a 2485 file project the file claims 39.9 MB of it as a single 41,739,216 byte script entry, against a 128 MB default opcache.memory_consumption. The file is now framed serialize() payloads: `` on the first line, then `name length` or `name* count` headers, each followed by length-prefixed payloads. Written frame by frame, and the array sections entry by entry, because serializing a section in one call holds it in memory twice over - exportedNodes alone is 39 MB on a 4524 file project - which is the trap the var_export writer avoided the same way. errors, locallyIgnoredErrors, collectedData and exportedNodes are the sections restore() hands back as callbacks. The var_export format defers them by writing one closure per section, which still leaves PHP compiling the array literal inside it. Here each section is one frame, so the reader remembers where it starts, walks past its entries without unserializing them, and decodes on demand. Walking rather than seeking wholesale keeps the framing of the whole file validated up front, so a damaged cache is discarded by restore() rather than throwing later inside a callback. Measured, main process, interleaved, every figure repeated across rounds: opening the cache 0.39s and 325 MB against 0.01s and 14 MB, or 124 MB with all four lazy sections materialised; warm peak -61% at 4524 files and -56% at 2485. The file on disk is 12 to 35% bigger. A format that can be read half way has to refuse to, so every frame violation throws. fseek() past the end of a file succeeds, which is why the skip walk compares the position with the file size after every entry - without that a truncated section is handed out as a callback pointing past the end. No cache version bump is needed: a cache in the old PHP format fails to unserialize and is discarded like any other unreadable file. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/e2e-tests.yml | 18 + .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-lazy.php | 13 + e2e/result-cache-truncated/truncate.php | 13 + .../ResultCache/ResultCacheManager.php | 313 +++++++++++++++--- 8 files changed, 338 insertions(+), 51 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-lazy.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 226f3860e99..fc5226a6006 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -161,6 +161,24 @@ jobs: echo "$OUTPUT" ../bashunit -a contains 'Result cache restored. 0 files will be reanalysed.' "$OUTPUT" ../bashunit -a not_contains 'metadata do not match' "$OUTPUT" + - script: | + cd e2e/result-cache-truncated + ../../bin/phpstan -vvv + # Cut inside the first section's payload: the cache is damaged, not merely stale, so + # it has to be discarded rather than read as far as it goes. + php truncate.php + OUTPUT=$(../../bin/phpstan -vvv 2>&1) + echo "$OUTPUT" + ../bashunit -a contains 'an error occurred while loading the cache file' "$OUTPUT" + ../bashunit -a contains 'Result cache is saved.' "$OUTPUT" + # And once more inside a section that is read lazily, which is a different code path. + php truncate-lazy.php + OUTPUT=$(../../bin/phpstan -vvv 2>&1) + echo "$OUTPUT" + ../bashunit -a contains 'is truncated at entry' "$OUTPUT" + OUTPUT=$(../../bin/phpstan -vvv 2>&1) + echo "$OUTPUT" + ../bashunit -a contains 'Result cache restored.' "$OUTPUT" - script: | cd e2e/bug-14514 composer install diff --git a/.gitignore b/.gitignore index bb2eb516626..5114d343238 100644 --- a/.gitignore +++ b/.gitignore @@ -20,3 +20,4 @@ e2e/bashunit /e2e/result-cache-atomic-save/tmp /e2e/result-cache-moved-tmpdir/tmp-a /e2e/result-cache-moved-tmpdir/tmp-b +/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-lazy.php b/e2e/result-cache-truncated/truncate-lazy.php new file mode 100644 index 00000000000..603a941e6a7 --- /dev/null +++ b/e2e/result-cache-truncated/truncate-lazy.php @@ -0,0 +1,13 @@ + is never reached), so a downgrade + * degrades to a silent full analysis instead. + */ + private const SERIALIZED_FILE_PREFIX = ''; + + /** + * Sections restore() hands back as callbacks instead of arrays, so a run that never asks for them + * never pays for decoding them. Each is a whole array frame in the file, so the reader only has to + * remember where it starts and walk past its entries. + */ + private const LAZY_SECTIONS = ['errors', 'locallyIgnoredErrors', 'collectedData', 'exportedNodes']; + /** @var array */ private array $fileHashes = []; @@ -274,7 +301,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) { @unlink($cacheFilePath); @@ -1448,9 +1480,11 @@ private function save( $pid = getmypid(); $temporaryFile = sprintf('%s.%s.tmp', $file, $pid === false ? uniqid() : $pid); - // 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($temporaryFile, 'w'); if ($handle === false) { $error = error_get_last(); @@ -1461,38 +1495,18 @@ private function save( $renamed = false; 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); fclose($handle); $closed = true; @@ -1525,30 +1539,227 @@ 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; + } + + $stat = fstat($handle); + $fileSize = $stat === false ? 0 : $stat['size']; + $closeHandle = true; + + try { + if (rtrim((string) fgets($handle), "\n") !== self::SERIALIZED_FILE_PREFIX) { + return null; + } + + $data = []; + $lazy = array_fill_keys(self::LAZY_SECTIONS, false); + while (($header = fgets($handle)) !== false) { + $header = rtrim($header, "\n"); + if ($header === '') { + continue; + } + + $parts = explode(' ', $header, 2); + if (count($parts) !== 2) { + throw new RuntimeException(sprintf('Malformed frame header "%s".', $header)); + } + + [$name, $size] = $parts; + if (!str_ends_with($name, '*')) { + $data[$name] = $this->readFrame($handle, (int) $size); + + continue; + } + + $name = substr($name, 0, -1); + $count = (int) $size; + if (!array_key_exists($name, $lazy)) { + $data[$name] = $this->readEntryFrames($handle, $count); + + continue; + } + + // The entries are walked rather than decoded: that validates the framing of the whole + // file up front, so a damaged cache is still discarded by restore() instead of failing + // later inside the callback, and leaves the payloads to be unserialized on demand. + $offset = ftell($handle); + if ($offset === false) { + throw new RuntimeException(sprintf('Cannot tell the position of section "%s".', $name)); + } + + $this->skipEntryFrames($handle, $count, $fileSize, $name); + $lazy[$name] = true; + $data[$name . 'Callback'] = fn (): array => $this->readEntryFramesAt($handle, $offset, $count, $name); + } + + foreach ($lazy as $name => $seen) { + if ($seen) { + continue; + } + + $data[$name . 'Callback'] = static fn (): array => []; + } + + $closeHandle = false; + + return $data; + } finally { + if ($closeHandle) { + fclose($handle); + } + } + } + + /** + * Walks past an array frame's entries without unserializing them, checking as it goes that the + * file really holds them. + * + * @param resource $handle + */ + private function skipEntryFrames($handle, int $count, int $fileSize, string $name): void + { + for ($i = 0; $i < $count; $i++) { + $length = fgets($handle); + if ($length === false) { + throw new RuntimeException(sprintf('Section "%s" ended after %d of %d entries.', $name, $i, $count)); + } + + $length = (int) rtrim($length, "\n"); + if ($length <= 0) { + throw new RuntimeException(sprintf('Frame length %d is not positive.', $length)); + } + + // fseek() past the end of a file succeeds, so the position is what catches a section the + // file does not actually hold. + if (fseek($handle, $length, SEEK_CUR) !== 0) { + throw new RuntimeException(sprintf('Cannot skip entry %d of section "%s".', $i, $name)); + } + + $position = ftell($handle); + if ($position === false || $position > $fileSize) { + throw new RuntimeException(sprintf('Section "%s" is truncated at entry %d of %d.', $name, $i, $count)); + } + } + } + + /** + * @param resource $handle + * @return array + */ + private function readEntryFramesAt($handle, int $offset, int $count, string $name): array + { + if (fseek($handle, $offset) !== 0) { + throw new RuntimeException(sprintf('Cannot seek to section "%s".', $name)); + } + + return $this->readEntryFrames($handle, $count); + } + + /** + * @param resource $handle + * @return array + */ + private function readEntryFrames($handle, int $count): array + { + $entries = []; + for ($i = 0; $i < $count; $i++) { + $length = fgets($handle); + if ($length === false) { + 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)) { + throw new RuntimeException('An entry frame did not contain an array.'); + } + + foreach ($entry as $key => $value) { + $entries[$key] = $value; + } + } + + return $entries; + } + + /** + * 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 CI cache artifact + * archived or restored half way, an interrupted copy, a disk that filled up. 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) { + 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)); } - $this->writeToHandle($handle, $file, "\n)"); + return $value; } /** From 25e7493f97eb435fb05187ad08e81777cef1f765 Mon Sep 17 00:00:00 2001 From: Sander Muller Date: Wed, 2 Sep 2026 20:02:41 +0200 Subject: [PATCH 2/2] Discard the cache when the cached results cannot be reconstructed unserialize() has no reconstruction hook, so a cache written by a PHPStan whose classes have since changed can fail while the objects are rebuilt: a property the payload does not carry stays uninitialized and reading it throws. var_export absorbed this because __set_state() reconstructs through the constructor, which applies the declared defaults - renaming Error::$tip is harmless there and aborted the run here. cacheVersion and phpstanVersion keep a released version away from another release's objects, but phpstanVersion comes from Composer's installed.php rather than the working tree, so a source checkout keeps one value across every edit of these classes. That is where it is reachable, and it is also where PHPStan is developed. The four callbacks are now invoked inside restore()'s failure path, so a cache that cannot be read back is discarded and everything re-analysed, the same as for a damaged file. Before this the same shape aborted the command with an uncaught error and a usage dump - on both formats, since the var_export closures are evaluated at the same point. e2e/result-cache-stale-objects renames Error::$message inside the cache file, which is that shape without needing two PHPStan versions installed, keeping the byte length so the frame headers stay valid. The run reports the cause and re-analyses; without the change it dies on the uninitialized property. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/e2e-tests.yml | 16 +++++++++ .gitignore | 1 + e2e/result-cache-stale-objects/phpstan.neon | 5 +++ .../rename-property.php | 25 ++++++++++++++ e2e/result-cache-stale-objects/src/Foo.php | 13 ++++++++ e2e/result-cache-stale-objects/tmp/.gitignore | 2 ++ .../ResultCache/ResultCacheManager.php | 33 ++++++++++++++++--- 7 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 e2e/result-cache-stale-objects/phpstan.neon create mode 100644 e2e/result-cache-stale-objects/rename-property.php create mode 100644 e2e/result-cache-stale-objects/src/Foo.php create mode 100644 e2e/result-cache-stale-objects/tmp/.gitignore diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index fc5226a6006..ec59676e1be 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -179,6 +179,22 @@ jobs: OUTPUT=$(../../bin/phpstan -vvv 2>&1) echo "$OUTPUT" ../bashunit -a contains 'Result cache restored.' "$OUTPUT" + - script: | + cd e2e/result-cache-stale-objects + OUTPUT=$(../bashunit -a exit_code "1" "../../bin/phpstan -vvv") + echo "$OUTPUT" + # Renaming a property inside the cache file is what a cache written by a PHPStan whose + # classes have changed since looks like: the payload does not carry the property the class + # now declares, so reconstructing the object leaves it uninitialized and reading it throws. + php rename-property.php + OUTPUT=$(../bashunit -a exit_code "1" "../../bin/phpstan -vvv") + echo "$OUTPUT" + ../bashunit -a contains 'could not be read back' "$OUTPUT" + ../bashunit -a contains 'Result cache is saved.' "$OUTPUT" + # The cache written in its place is usable again. + OUTPUT=$(../bashunit -a exit_code "1" "../../bin/phpstan -vvv") + echo "$OUTPUT" + ../bashunit -a contains 'Result cache restored.' "$OUTPUT" - script: | cd e2e/bug-14514 composer install diff --git a/.gitignore b/.gitignore index 5114d343238..9c6e970e8b3 100644 --- a/.gitignore +++ b/.gitignore @@ -20,4 +20,5 @@ e2e/bashunit /e2e/result-cache-atomic-save/tmp /e2e/result-cache-moved-tmpdir/tmp-a /e2e/result-cache-moved-tmpdir/tmp-b +/e2e/result-cache-stale-objects/tmp /e2e/result-cache-truncated/tmp diff --git a/e2e/result-cache-stale-objects/phpstan.neon b/e2e/result-cache-stale-objects/phpstan.neon new file mode 100644 index 00000000000..6daefc8c39c --- /dev/null +++ b/e2e/result-cache-stale-objects/phpstan.neon @@ -0,0 +1,5 @@ +parameters: + level: 8 + tmpDir: tmp + paths: + - src diff --git a/e2e/result-cache-stale-objects/rename-property.php b/e2e/result-cache-stale-objects/rename-property.php new file mode 100644 index 00000000000..f7ed99c7d43 --- /dev/null +++ b/e2e/result-cache-stale-objects/rename-property.php @@ -0,0 +1,25 @@ +fullAnalysis( + sprintf('Result cache not used because the cached results could not be read back: %s', $e->getMessage()), + $allAnalysedFiles, + $meta, + $currentFileHashes, + $output, + ); + } $filteredErrors = []; $filteredLocallyIgnoredErrors = []; $filteredLinesToIgnore = [];