From 63cd54c852e231904b65fa14ef3c083173b9ccea Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 30 Sep 2026 13:30:55 +0200 Subject: [PATCH 01/10] Let a saved cache entry replace the one in the turbo arena Cache::load() publishes every entry it reads from disk to the arena, and arena records were written once - the first publish of a key won. An entry its caller checks after loading it could therefore get stuck there: FileTypeMapper checks its name scope maps against the hashes of the files they were created from, so after a file changed the stale map got published, was rejected, and the fresh map created instead could never replace it. Every later lookup in every worker got the stale map back and created the map again. With every file of Drupal core edited, the workers created 44k name scope maps for 11k files, parsing the file each time. ArenaCache::replace() takes over the key's slot instead, and Cache::save() uses it: a load publishes a copy of the file only when the key has nothing yet, a save always wins. Callers checking their entries after loading them need to do nothing for it. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ --- src/Cache/ArenaCache.php | 12 +++++++++ src/Cache/Cache.php | 9 ++++++- turbo-ext/src/ArenaCache.cpp | 37 ++++++++++++++++++++++++---- turbo-ext/src/generated/ArenaCache.h | 33 ++++++++++++++----------- turbo-ext/tests/arena-smoke.php | 20 +++++++++++++++ 5 files changed, 91 insertions(+), 20 deletions(-) diff --git a/src/Cache/ArenaCache.php b/src/Cache/ArenaCache.php index 888b6b74640..fbe7ebea16d 100644 --- a/src/Cache/ArenaCache.php +++ b/src/Cache/ArenaCache.php @@ -60,12 +60,24 @@ public static function lookup(string $key): mixed } /** + * Publishes the value unless the key already has one - the first publish wins. + * * @param mixed $value */ public static function publish(string $key, $value): void { } + /** + * Publishes the value in place of the one the key has - for a value just + * computed because the published one was out of date. + * + * @param mixed $value + */ + public static function replace(string $key, $value): void + { + } + public static function lookupHash(string $recordKey, string $entryKey): mixed { return null; diff --git a/src/Cache/Cache.php b/src/Cache/Cache.php index 8988caf8983..697df298015 100644 --- a/src/Cache/Cache.php +++ b/src/Cache/Cache.php @@ -32,6 +32,13 @@ public function load(string $key, string $variableKey) // cache file. The arena's codec covers scalars, arrays and plain // value objects, and interns repeated strings on read like include() // does; payloads it cannot represent just stay per-worker. + // + // What a load publishes is only a copy of the file and does not + // displace an entry already there. What save() publishes replaces it: + // a caller that finds an entry out of date by checking it after the + // load (FileTypeMapper checks the hashes of the files a name scope map + // was created from) creates the value again and saves it, and every + // process after that has to get the new value, not the one it rejected. $arenaKey = null; if ($this->isArenaUsable()) { $arenaKey = 'fcs:' . $key . "\0" . $variableKey; @@ -62,7 +69,7 @@ public function save(string $key, string $variableKey, $data): void return; } - ArenaCache::publish('fcs:' . $key . "\0" . $variableKey, $data); + ArenaCache::replace('fcs:' . $key . "\0" . $variableKey, $data); } /** diff --git a/turbo-ext/src/ArenaCache.cpp b/turbo-ext/src/ArenaCache.cpp index a268f8a543a..77d3dcac3d5 100644 --- a/turbo-ext/src/ArenaCache.cpp +++ b/turbo-ext/src/ArenaCache.cpp @@ -17,7 +17,7 @@ * exactly as it does when the extension is absent; the PHP twin of this class * is a cache that never hits. * - * Concurrency is lock-free and write-once: + * Concurrency is lock-free, and records are immutable once written: * * - allocation is a compare-and-swap bump cursor in the arena header; * - publication is a compare-and-swap of an index slot from 0 to the record @@ -28,6 +28,10 @@ * - two processes computing the same key race benignly: one CAS wins, the * loser's bytes become dead space. Wasteful, never unsafe — the same * philosophy as the odsl directory-scan lock's cold-cache races. + * - replace() swings an existing key's slot over to a new record with the + * same CAS - for a value the caller has just recomputed because the one + * published before was out of date. The old record stays intact for any + * reader that already found it and becomes dead space, like a loser's. * * Records are self-contained flat blobs of PHP values: scalars, arrays, and * plain userland objects (no serialization hooks, no custom create handler @@ -676,8 +680,9 @@ static bool findRecord(const char *key, size_t keyLen, RecordView *view) } /* Copies a fully-built record into the arena and CAS-publishes it; loses - * gracefully to a concurrent publisher of the same key, as late as it can. */ -static void publishRecord(const char *key, size_t keyLen, uint32_t kind, const WriteBuffer &payload) + * gracefully to a concurrent publisher of the same key, as late as it can. + * With replace, it takes over the key's slot instead - the last writer wins. */ +static void publishRecord(const char *key, size_t keyLen, uint32_t kind, const WriteBuffer &payload, bool replace = false) { if (pt_arena_base == NULL || keyLen > UINT32_MAX) return; @@ -703,7 +708,7 @@ static void publishRecord(const char *key, size_t keyLen, uint32_t kind, const W * here is a page the backing store commits for good - a loser that returns * now costs nothing but the bump it already took. */ RecordView published; - if (findRecord(key, keyLen, &published)) return; + if (!replace && findRecord(key, keyLen, &published)) return; char *record = (char *) pt_arena_base + offset; RecordHeader header; @@ -726,7 +731,13 @@ static void publishRecord(const char *key, size_t keyLen, uint32_t kind, const W } RecordView existing; if (!recordAt(current, &existing)) return; - if (existing.header->keyLen == keyLen && memcmp(existing.key, key, keyLen) == 0) return; /* lost the race: someone published this key first */ + if (existing.header->keyLen == keyLen && memcmp(existing.key, key, keyLen) == 0) { + if (!replace) return; /* lost the race: someone published this key first */ + /* a slot only ever moves between records of the key it was claimed + * for, so a failed CAS just means another replace got in first */ + while (!atomicCasRelease(slot, current, offset)) current = atomicLoadAcquire(slot); + return; + } } /* index congested — give up on this record, it stays dead space */ } @@ -1131,6 +1142,15 @@ class ArenaCache publishRecord(ZSTR_VAL(key), ZSTR_LEN(key), RECORD_KIND_VALUE, payload); } + static void replace(zend_string *key, zval *value) + { + if (pt_arena_base == NULL) return; + WriteBuffer payload; + SerializeCtx ctx; + if (!serializeValue(payload, value, 0, ctx)) return; + publishRecord(ZSTR_VAL(key), ZSTR_LEN(key), RECORD_KIND_VALUE, payload, true); + } + static void lookupHash(zend_string *recordKey, zend_string *entryKey, zval *return_value) { RETVAL_NULL(); @@ -1220,6 +1240,13 @@ PT_MINIT_REGISTRATION(pt_register_arena_cache) phpstanturbo::ArenaCache::publish(key, value); }); + cls.method(sigs::replace, [](INTERNAL_FUNCTION_PARAMETERS) { + zend_string *key; + zval *value; + if (!zp::parse(execute_data, key, value)) RETURN_THROWS(); + phpstanturbo::ArenaCache::replace(key, value); + }); + cls.method(sigs::lookupHash, [](INTERNAL_FUNCTION_PARAMETERS) { zend_string *recordKey, *entryKey; if (!zp::parse(execute_data, recordKey, entryKey)) RETURN_THROWS(); diff --git a/turbo-ext/src/generated/ArenaCache.h b/turbo-ext/src/generated/ArenaCache.h index a4f8f9fb656..36cc66191aa 100644 --- a/turbo-ext/src/generated/ArenaCache.h +++ b/turbo-ext/src/generated/ArenaCache.h @@ -34,12 +34,13 @@ inline constexpr char strings[] = "lookup\0" /* 59 */ "value\0" /* 66 */ "publish\0" /* 72 */ - "recordKey\0" /* 80 */ - "entryKey\0" /* 90 */ - "lookupHash\0" /* 99 */ - "lookupHashAll\0" /* 110 */ - "entries\0" /* 124 */ - "publishHash"; /* 132 */ + "replace\0" /* 80 */ + "recordKey\0" /* 88 */ + "entryKey\0" /* 98 */ + "lookupHash\0" /* 107 */ + "lookupHashAll\0" /* 118 */ + "entries\0" /* 132 */ + "publishHash"; /* 140 */ inline constexpr reg::PackedArg args[] = { reg::packed(0, MAY_BE_STRING), /* create $runId */ reg::packed(6, MAY_BE_NULL | MAY_BE_STRING), /* create return */ @@ -54,13 +55,16 @@ inline constexpr reg::PackedArg args[] = { reg::packed(45, MAY_BE_STRING), /* publish $key */ reg::packed(66, 0), /* publish $value */ reg::packed(6, MAY_BE_VOID), /* publish return */ - reg::packed(80, MAY_BE_STRING), /* lookupHash $recordKey */ - reg::packed(90, MAY_BE_STRING), /* lookupHash $entryKey */ + reg::packed(45, MAY_BE_STRING), /* replace $key */ + reg::packed(66, 0), /* replace $value */ + reg::packed(6, MAY_BE_VOID), /* replace return */ + reg::packed(88, MAY_BE_STRING), /* lookupHash $recordKey */ + reg::packed(98, MAY_BE_STRING), /* lookupHash $entryKey */ reg::packed(6, MAY_BE_ANY), /* lookupHash return */ - reg::packed(80, MAY_BE_STRING), /* lookupHashAll $recordKey */ + reg::packed(88, MAY_BE_STRING), /* lookupHashAll $recordKey */ reg::packed(6, MAY_BE_NULL | MAY_BE_ARRAY), /* lookupHashAll return */ - reg::packed(80, MAY_BE_STRING), /* publishHash $recordKey */ - reg::packed(124, MAY_BE_ARRAY), /* publishHash $entries */ + reg::packed(88, MAY_BE_STRING), /* publishHash $recordKey */ + reg::packed(132, MAY_BE_ARRAY), /* publishHash $entries */ reg::packed(6, MAY_BE_VOID), /* publishHash return */ }; using Sig = reg::Sig; @@ -75,9 +79,10 @@ inline constexpr sigtab::Sig destroy = { { 37 /* destroy */, 0, 5, 0, 5, ZEND_AC inline constexpr sigtab::Sig hasRecord = { { 49 /* hasRecord */, 1, 6, 1, 7, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } }; inline constexpr sigtab::Sig lookup = { { 59 /* lookup */, 1, 8, 1, 9, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } }; inline constexpr sigtab::Sig publish = { { 72 /* publish */, 2, 10, 2, 12, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } }; -inline constexpr sigtab::Sig lookupHash = { { 99 /* lookupHash */, 2, 13, 2, 15, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } }; -inline constexpr sigtab::Sig lookupHashAll = { { 110 /* lookupHashAll */, 1, 16, 1, 17, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } }; -inline constexpr sigtab::Sig publishHash = { { 132 /* publishHash */, 2, 18, 2, 20, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } }; +inline constexpr sigtab::Sig replace = { { 80 /* replace */, 2, 13, 2, 15, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } }; +inline constexpr sigtab::Sig lookupHash = { { 107 /* lookupHash */, 2, 16, 2, 18, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } }; +inline constexpr sigtab::Sig lookupHashAll = { { 118 /* lookupHashAll */, 1, 19, 1, 20, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } }; +inline constexpr sigtab::Sig publishHash = { { 140 /* publishHash */, 2, 21, 2, 23, ZEND_ACC_PUBLIC | ZEND_ACC_STATIC } }; } // namespace sig } // namespace ptdecl::ArenaCache diff --git a/turbo-ext/tests/arena-smoke.php b/turbo-ext/tests/arena-smoke.php index d3f2cc82300..e9ca2e63361 100644 --- a/turbo-ext/tests/arena-smoke.php +++ b/turbo-ext/tests/arena-smoke.php @@ -156,6 +156,7 @@ function waitChild(array $childHandle): array check(ArenaCache::lookupHashAll('sigmap') === $rows, 'child: lookupHashAll identical incl. order and int keys'); check(ArenaCache::lookupHashAll('fixtures') === null, 'child: lookupHashAll on value record is null'); check(ArenaCache::lookupHashAll('missing') === null, 'child: lookupHashAll on missing record is null'); + check(ArenaCache::lookup('replaced') === ['v' => 2], 'child: sees the replaced record'); ArenaCache::publish('from-child', ['pid' => 'child-wrote-this']); global $failures; @@ -170,6 +171,11 @@ function waitChild(array $childHandle): array ArenaCache::publish('contested', $payload); ArenaCache::publish('racer-' . getmypid(), [getmypid()]); check(ArenaCache::lookup('contested') === $payload, 'racer: contested readback identical'); + for ($i = 0; $i < 100; $i++) { + ArenaCache::replace('replaced-race', ['pid' => getmypid(), 'i' => $i, 'rows' => range(1, 20)]); + $replaced = ArenaCache::lookup('replaced-race'); + check(is_array($replaced) && $replaced['rows'] === range(1, 20), 'racer: replaced record readable while others replace it'); + } global $failures; exit($failures === 0 ? 0 : 1); } @@ -261,6 +267,18 @@ function waitChild(array $childHandle): array ArenaCache::publish('fixtures', ['clobbered' => true]); check(ArenaCache::lookup('fixtures') === fixtures(), 'parent: republish does not clobber'); +// replace takes over an existing key - the last write wins - and publishes an absent one +ArenaCache::publish('replaced', ['v' => 1]); +ArenaCache::replace('replaced', ['v' => 2]); +check(ArenaCache::lookup('replaced') === ['v' => 2], 'parent: replace overwrites'); +ArenaCache::publish('replaced', ['v' => 3]); +check(ArenaCache::lookup('replaced') === ['v' => 2], 'parent: publish does not clobber a replaced record'); +ArenaCache::replace('replaced-absent', ['v' => 1]); +check(ArenaCache::lookup('replaced-absent') === ['v' => 1], 'parent: replace publishes an absent key'); +ArenaCache::replace('replaced', ['fn' => static function (): void { +}]); +check(ArenaCache::lookup('replaced') === ['v' => 2], 'parent: a replace that cannot be published keeps the record'); + // ---- child reads everything, writes back ---- [$exitCode, $stdout] = waitChild(spawnChild('child-read', $name)); echo $stdout; @@ -279,6 +297,8 @@ function waitChild(array $childHandle): array } $contested = ArenaCache::lookup('contested'); check($contested === ['winner-takes' => str_repeat('all', 100), 'rows' => range(1, 50)], 'parent: contested record consistent after race'); +$replacedRace = ArenaCache::lookup('replaced-race'); +check(is_array($replacedRace) && $replacedRace['i'] === 99 && $replacedRace['rows'] === range(1, 20), 'parent: racing replaces leave the last write of one of them'); // ---- unlink: existing mappings keep working, new attaches fail ---- ArenaCache::unlinkName(); From a7d99949ddf59339bfc1fd2090aff77a2791de62 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 29 Sep 2026 00:46:29 +0200 Subject: [PATCH 02/10] Keep directory listings between runs Walking the analysed directories costs a readdir() per directory. On Drupal core, 28k directories, that is about a second in the main process, and the worker building the symbol index walks the same directories again. A directory's listing only changes when an entry in it is added, removed or renamed, and each of those updates its mtime and ctime. DirectoryWalker now keeps the listings in tmpDir and reads a directory again only when its stat changed. A directory modified in the second its listing is read is not kept, because a second change in that same second would not show in its stat. The walk yields the same files in the same order as Symfony Finder, and falls back to Finder on Windows and for stream wrappers or unreadable directories. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ --- src/File/DirectoryWalker.php | 285 +++++++++++++++++++++ tests/PHPStan/File/DirectoryWalkerTest.php | 129 ++++++++++ 2 files changed, 414 insertions(+) diff --git a/src/File/DirectoryWalker.php b/src/File/DirectoryWalker.php index 26ef15794d7..6e6cfd0566b 100644 --- a/src/File/DirectoryWalker.php +++ b/src/File/DirectoryWalker.php @@ -2,10 +2,37 @@ namespace PHPStan\File; +use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; use Symfony\Component\Finder\Finder; +use Symfony\Component\Finder\Glob; use function array_key_exists; +use function array_keys; +use function closedir; +use function explode; +use function file_get_contents; +use function file_put_contents; +use function getmypid; use function implode; +use function is_array; +use function is_dir; +use function lstat; +use function mkdir; +use function opendir; +use function preg_match; +use function readdir; +use function rename; +use function rtrim; +use function serialize; +use function sprintf; +use function stat; +use function str_contains; +use function str_starts_with; +use function substr; +use function time; +use function unlink; +use function unserialize; +use const DIRECTORY_SEPARATOR; /** * The raw filesystem walk behind FileFinder. @@ -18,20 +45,66 @@ * * FileMonitor detects changes by re-running the finder and must see the filesystem as it is now, * so it walks uncached and clears the shared walks before each check. + * + * Reading the directories is what a walk costs - on a tree the size of Drupal core (28k directories) + * about a second, every run, in the main process and again in the worker that builds the symbol + * index. What a directory contains changes only when an entry is added, removed or renamed in it, + * and each of those updates the directory's mtime and ctime, so the listings are kept in tmpDir and + * a directory whose stat still matches is not read again. ctime cannot be set back by touch or by + * extracting an archive, and a directory modified in the very second its listing is read is not + * kept: a second change within that same second would leave its stat unchanged. The same technique + * is behind git's untracked cache. + * + * The walk yields what Symfony Finder yields for files()->name()->followLinks() with its default + * ignores - dot files and VCS directories left out, symlinks followed, files in readdir order - and + * falls back to Finder itself for anything it does not handle the same way (Windows paths, stream + * wrappers, unreadable directories), so errors surface exactly as they did. */ #[AutowiredService] final class DirectoryWalker { + private const LISTINGS_FORMAT = 'directoryListings-v1'; + + /** The directories Finder's ignoreVCS() leaves out; the ones starting with a dot are left out anyway. */ + private const VCS_DIRECTORIES = ['_svn' => true, 'CVS' => true, '_darcs' => true]; + /** @var array> */ private array $cachedWalks = []; + /** + * Directory path => [mtime, ctime, inode, entries]. The entries are the directory's names in + * readdir order, each prefixed by d (directory), f (anything else - Finder's files() only leaves + * out directories, so a broken symlink is a file too) or l (symlink - resolved on every walk, + * because its target can change without this directory changing), joined by NUL. + * + * @var array|null + */ + private ?array $listings = null; + + private bool $listingsChanged = false; + + /** + * @param string $tmpDir where the listings are kept between runs, nowhere when empty + */ + public function __construct( + #[AutowiredParameter] + private string $tmpDir = '', + ) + { + } + /** * @param string[] $fileExtensions * @return list */ public function walk(string $directory, array $fileExtensions): array { + $files = $this->walkWithListings($directory, $fileExtensions); + if ($files !== null) { + return $files; + } + $finder = new Finder(); $finder->followLinks(); @@ -62,4 +135,216 @@ public function clearCachedWalks(): void $this->cachedWalks = []; } + /** + * @param string[] $fileExtensions + * @return list|null null when Finder has to do the walk + */ + private function walkWithListings(string $directory, array $fileExtensions): ?array + { + if (DIRECTORY_SEPARATOR !== '/' || str_contains($directory, '://')) { + return null; + } + + // what Finder's normalizeDir() does, the filesystem root aside + $directory = rtrim($directory, '/'); + if ($directory === '') { + return null; + } + + $this->loadListings(); + + $files = []; + $visited = []; + if (!$this->walkDirectory($directory, Glob::toRegex('*.{' . implode(',', $fileExtensions) . '}'), time(), $files, $visited)) { + return null; + } + + $this->saveListings($directory, $visited); + + return $files; + } + + /** + * @param list $files + * @param array $visited + */ + private function walkDirectory(string $directory, string $pattern, int $now, array &$files, array &$visited): bool + { + $stat = @stat($directory); + if ($stat === false) { + return false; + } + + $visited[$directory] = true; + $listing = $this->listings[$directory] ?? null; + if ($listing !== null && $listing[0] === $stat['mtime'] && $listing[1] === $stat['ctime'] && $listing[2] === $stat['ino']) { + $entries = $listing[3]; + } else { + $entries = $this->readDirectory($directory); + if ($entries === null) { + return false; + } + + if ($stat['mtime'] < $now && $stat['ctime'] < $now) { + $this->listings[$directory] = [$stat['mtime'], $stat['ctime'], $stat['ino'], $entries]; + } else { + unset($this->listings[$directory]); + } + $this->listingsChanged = true; + } + + if ($entries === '') { + return true; + } + + foreach (explode("\0", $entries) as $entry) { + $type = $entry[0]; + $name = substr($entry, 1); + $path = $directory . '/' . $name; + if ($type === 'l') { + if (is_dir($path)) { + if (isset(self::VCS_DIRECTORIES[$name])) { + continue; + } + $type = 'd'; + } else { + $type = 'f'; + } + } + + if ($type === 'd') { + if (!$this->walkDirectory($path, $pattern, $now, $files, $visited)) { + return false; + } + + continue; + } + + if (preg_match($pattern, $name) !== 1) { + continue; + } + + $files[] = $path; + } + + return true; + } + + private function readDirectory(string $directory): ?string + { + $handle = @opendir($directory); + if ($handle === false) { + return null; + } + + $entries = []; + while (($name = readdir($handle)) !== false) { + // also skips . and .. - Finder leaves out everything starting with a dot + if (str_starts_with($name, '.')) { + continue; + } + + $stat = @lstat($directory . '/' . $name); + if ($stat === false) { + continue; + } + + $type = $stat['mode'] & 0170000; + if ($type === 0120000) { + $entries[] = 'l' . $name; + } elseif ($type === 0040000) { + if (isset(self::VCS_DIRECTORIES[$name])) { + continue; + } + $entries[] = 'd' . $name; + } else { + $entries[] = 'f' . $name; + } + } + closedir($handle); + + return implode("\0", $entries); + } + + private function getListingsFile(): ?string + { + if ($this->tmpDir === '') { + return null; + } + + return $this->tmpDir . '/cache/directory-listings.bin'; + } + + private function loadListings(): void + { + if ($this->listings !== null) { + return; + } + + $this->listings = []; + $file = $this->getListingsFile(); + if ($file === null) { + return; + } + + $contents = @file_get_contents($file); + if ($contents === false) { + return; + } + + $data = @unserialize($contents, ['allowed_classes' => false]); + if (!is_array($data) || ($data['format'] ?? null) !== self::LISTINGS_FORMAT || !is_array($data['listings'] ?? null)) { + return; + } + + /** @var array $listings */ + $listings = $data['listings']; + $this->listings = $listings; + } + + /** + * @param array $visited + */ + private function saveListings(string $root, array $visited): void + { + if (!$this->listingsChanged || $this->listings === null) { + return; + } + + // A directory under the walked root that the walk did not reach is gone, or no longer + // reachable - either way its listing is not going to be read again. + foreach (array_keys($this->listings) as $directory) { + if (isset($visited[$directory]) || !str_starts_with($directory, $root . '/')) { + continue; + } + + unset($this->listings[$directory]); + } + + $this->listingsChanged = false; + $file = $this->getListingsFile(); + if ($file === null) { + return; + } + + $cacheDirectory = $this->tmpDir . '/cache'; + if (!is_dir($cacheDirectory) && !@mkdir($cacheDirectory, 0777, true) && !is_dir($cacheDirectory)) { + return; + } + + // written next to the final path and renamed into place, so that a concurrent run reads + // either the old listings or the new ones + $pid = getmypid(); + $temporaryFile = sprintf('%s.%s.tmp', $file, $pid === false ? 'x' : $pid); + if (@file_put_contents($temporaryFile, serialize(['format' => self::LISTINGS_FORMAT, 'listings' => $this->listings])) === false) { + return; + } + + if (@rename($temporaryFile, $file)) { + return; + } + + @unlink($temporaryFile); + } + } diff --git a/tests/PHPStan/File/DirectoryWalkerTest.php b/tests/PHPStan/File/DirectoryWalkerTest.php index f76362b6128..7443d0778a9 100644 --- a/tests/PHPStan/File/DirectoryWalkerTest.php +++ b/tests/PHPStan/File/DirectoryWalkerTest.php @@ -3,10 +3,22 @@ namespace PHPStan\File; use PHPStan\Testing\PHPStanTestCase; +use Symfony\Component\Finder\Finder; +use function clearstatcache; use function file_put_contents; +use function implode; +use function max; use function mkdir; +use function rename; +use function rmdir; +use function stat; +use function symlink; use function sys_get_temp_dir; +use function time; use function uniqid; +use function unlink; +use function usleep; +use const DIRECTORY_SEPARATOR; final class DirectoryWalkerTest extends PHPStanTestCase { @@ -63,4 +75,121 @@ public function testCachedWalksAreKeyedByExtensions(): void $this->assertCount(1, $textFiles); } + public function testWalkYieldsWhatFinderYields(): void + { + $tmpDir = $this->createTree(); + + $expected = $this->walkWithFinder($this->directory, ['php', 'sh', '']); + $fresh = (new DirectoryWalker($tmpDir))->walk($this->directory, ['php', 'sh', '']); + $this->waitUntilTheTreeIsInThePast(); + // stores the listings, now that they are no longer racy + (new DirectoryWalker($tmpDir))->walk($this->directory, ['php', 'sh', '']); + $fromListings = (new DirectoryWalker($tmpDir))->walk($this->directory, ['php', 'sh', '']); + + $this->assertNotSame([], $expected); + $this->assertSame($expected, $fresh); + $this->assertSame($expected, $fromListings); + $this->assertFileExists($tmpDir . '/cache/directory-listings.bin'); + } + + public function testListingsSeeAddedAndRemovedFiles(): void + { + $tmpDir = $this->createTree(); + $this->waitUntilTheTreeIsInThePast(); + (new DirectoryWalker($tmpDir))->walk($this->directory, ['php']); + + file_put_contents($this->directory . '/src/Added.php', 'directory . '/src/Nested/Deep.php'); + rename($this->directory . '/src/Foo.php', $this->directory . '/src/Renamed.php'); + + $this->assertSame( + $this->walkWithFinder($this->directory, ['php']), + (new DirectoryWalker($tmpDir))->walk($this->directory, ['php']), + ); + } + + public function testReplacedDirectoryIsReadAgain(): void + { + $tmpDir = $this->createTree(); + $this->waitUntilTheTreeIsInThePast(); + (new DirectoryWalker($tmpDir))->walk($this->directory, ['php']); + + unlink($this->directory . '/src/Nested/Deep.php'); + rmdir($this->directory . '/src/Nested'); + mkdir($this->directory . '/src/Nested'); + file_put_contents($this->directory . '/src/Nested/Other.php', 'assertContains( + $this->directory . '/src/Nested/Other.php', + (new DirectoryWalker($tmpDir))->walk($this->directory, ['php']), + ); + } + + private function createTree(): string + { + if (DIRECTORY_SEPARATOR !== '/') { + $this->markTestSkipped('The listings are not used on Windows, and the tree needs symlinks.'); + } + + $tmpDir = $this->directory . '-tmp'; + mkdir($tmpDir); + + mkdir($this->directory . '/src'); + mkdir($this->directory . '/src/Nested'); + mkdir($this->directory . '/src/.hidden'); + mkdir($this->directory . '/src/CVS'); + mkdir($this->directory . '/bin'); + file_put_contents($this->directory . '/src/Foo.php', 'directory . '/src/Bar.php', 'directory . '/src/readme.md', 'x'); + file_put_contents($this->directory . '/src/.dotfile.php', 'directory . '/src/Nested/Deep.php', 'directory . '/src/.hidden/Hidden.php', 'directory . '/src/CVS/Versioned.php', 'directory . '/bin/tool.sh', '#!/bin/sh'); + file_put_contents($this->directory . '/bin/tool', '#!/bin/sh'); + file_put_contents($this->directory . '/bin/ends-with-dot.', 'x'); + + $outside = $this->directory . '-outside'; + mkdir($outside); + file_put_contents($outside . '/Linked.php', 'directory . '/src/linked-directory'); + symlink($outside . '/Linked.php', $this->directory . '/src/LinkedFile.php'); + symlink($outside . '/Missing.php', $this->directory . '/src/Broken.php'); + + return $tmpDir; + } + + /** + * A listing is kept only for a directory that has not changed in the second it was read. + */ + private function waitUntilTheTreeIsInThePast(): void + { + clearstatcache(); + $latest = 0; + foreach ([$this->directory, $this->directory . '/src', $this->directory . '/src/Nested', $this->directory . '/bin'] as $directory) { + $stat = stat($directory); + $this->assertIsArray($stat); + $latest = max($latest, $stat['mtime'], $stat['ctime']); + } + + while (time() <= $latest) { + usleep(50_000); + } + } + + /** + * @param string[] $fileExtensions + * @return list + */ + private function walkWithFinder(string $directory, array $fileExtensions): array + { + $files = []; + foreach ((new Finder())->followLinks()->files()->name('*.{' . implode(',', $fileExtensions) . '}')->in($directory) as $fileInfo) { + $files[] = $fileInfo->getPathname(); + } + + return $files; + } + } From 993bc7229b13b65d711490918994a1f8d80b2335 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 29 Sep 2026 00:46:53 +0200 Subject: [PATCH 03/10] Reuse the file hashes of the result cache while the files are unchanged Every run hashed every analysed file to find the changed ones, on Drupal core almost a second for 11k files. The result cache now also records each file's size, mtime, ctime, inode and device, and the recorded hash is reused while they match - the same check git does against its index. A signature is only recorded for a file last modified before the second its hash was taken, and nothing is reused on Windows, where ctime is the creation time. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ --- .../ResultCache/ResultCacheManager.php | 155 +++++++++++++++--- .../ResultCachePathTransformer.php | 8 +- 2 files changed, 133 insertions(+), 30 deletions(-) diff --git a/src/Analyser/ResultCache/ResultCacheManager.php b/src/Analyser/ResultCache/ResultCacheManager.php index ae570fb3832..d6e5f22036c 100644 --- a/src/Analyser/ResultCache/ResultCacheManager.php +++ b/src/Analyser/ResultCache/ResultCacheManager.php @@ -69,6 +69,7 @@ use function serialize; use function sort; use function sprintf; +use function stat; use function str_ends_with; use function str_starts_with; use function strlen; @@ -77,6 +78,7 @@ use function uniqid; use function unlink; use function unserialize; +use const DIRECTORY_SEPARATOR; use const PHP_VERSION_ID; use const SEEK_CUR; @@ -147,6 +149,14 @@ final class ResultCacheManager /** @var array */ private array $fileHashes = []; + /** + * The stat signatures of the analysed files whose hash in $fileHashes can be trusted to still + * describe them next time the signature matches - see hashAnalysedFiles(). + * + * @var array + */ + private array $fileStatSignatures = []; + private ?ResultCachePathTransformer $pathTransformer = null; /** @var array */ @@ -277,21 +287,16 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? { $this->restoredCacheUnchanged = false; $this->restoredStubFiles = []; + $this->fileStatSignatures = []; $startTime = microtime(true); - $currentFileHashes = []; - foreach ($allAnalysedFiles as $analysedFile) { - if (!is_file($analysedFile)) { - continue; - } - $currentFileHashes[$analysedFile] = $this->getFileHash($analysedFile); - } + $analysedFileStats = $this->statAnalysedFiles($allAnalysedFiles); if ($debug) { return $this->fullAnalysis( 'Result cache not used because of debug mode.', $allAnalysedFiles, $this->getMeta($allAnalysedFiles, $projectConfigArray), - $currentFileHashes, + $this->hashAnalysedFiles($analysedFileStats, null), $output, ); } @@ -300,7 +305,7 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? 'Result cache not used because only files were passed as analysed paths.', $allAnalysedFiles, $this->getMeta($allAnalysedFiles, $projectConfigArray), - $currentFileHashes, + $this->hashAnalysedFiles($analysedFileStats, null), $output, ); } @@ -311,7 +316,7 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? 'Result cache not used because the cache file does not exist.', $allAnalysedFiles, $this->getMeta($allAnalysedFiles, $projectConfigArray), - $currentFileHashes, + $this->hashAnalysedFiles($analysedFileStats, null), $output, ); } @@ -330,7 +335,7 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? sprintf('Result cache not used because an error occurred while loading the cache file: %s', $e->getMessage()), $allAnalysedFiles, $this->getMeta($allAnalysedFiles, $projectConfigArray), - $currentFileHashes, + $this->hashAnalysedFiles($analysedFileStats, null), $output, ); } @@ -342,7 +347,7 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? 'Result cache not used because the cache file is corrupted.', $allAnalysedFiles, $this->getMeta($allAnalysedFiles, $projectConfigArray), - $currentFileHashes, + $this->hashAnalysedFiles($analysedFileStats, null), $output, ); } @@ -359,6 +364,8 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? $data['linesToIgnore'] = $transformer->absolutizeCompoundKeyed($data['linesToIgnore']); $data['unmatchedLineIgnores'] = $transformer->absolutizeCompoundKeyed($data['unmatchedLineIgnores']); $data['dependencies'] = $transformer->absolutizeDependencies($data['dependencies']); + $currentFileHashes = $this->hashAnalysedFiles($analysedFileStats, $data['dependencies']); + $fileStatSignaturesChanged = $this->fileStatSignaturesDiffer($data['dependencies']); $data['packageDependencies'] = $transformer->absolutizeFileKeyed($data['packageDependencies'] ?? []); $errorsCallback = $data['errorsCallback']; @@ -892,7 +899,7 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? )); } - $this->restoredCacheUnchanged = !$metaDifferent && !$dependencyFilesChanged; + $this->restoredCacheUnchanged = !$metaDifferent && !$dependencyFilesChanged && !$fileStatSignaturesChanged; $this->restoredStubFiles = $cachedStubFiles; return new ResultCache( @@ -1471,10 +1478,7 @@ private function save( foreach ($dependencies as $file => $fileDependencies) { foreach ($fileDependencies as $fileDep) { if (!array_key_exists($fileDep, $invertedDependencies)) { - $invertedDependencies[$fileDep] = [ - 'fileHash' => $currentFileHashes[$fileDep] ?? $this->getDependencyFileHash($fileDep), - 'dependentFiles' => [], - ]; + $invertedDependencies[$fileDep] = $this->createDependencyEntry($fileDep, $currentFileHashes); unset($filesNoOneIsDependingOn[$fileDep]); } $invertedDependencies[$fileDep]['dependentFiles'][] = $file; @@ -1484,11 +1488,7 @@ private function save( foreach ($usedTraitDependencies as $file => $fileUsedTraitDependencies) { foreach ($fileUsedTraitDependencies as $usedTraitFileDep) { if (!array_key_exists($usedTraitFileDep, $invertedDependencies)) { - $invertedDependencies[$usedTraitFileDep] = [ - 'fileHash' => $currentFileHashes[$usedTraitFileDep] ?? $this->getDependencyFileHash($usedTraitFileDep), - 'dependentFiles' => [], - 'usedTraitDependentFiles' => [], - ]; + $invertedDependencies[$usedTraitFileDep] = $this->createDependencyEntry($usedTraitFileDep, $currentFileHashes) + ['usedTraitDependentFiles' => []]; unset($filesNoOneIsDependingOn[$usedTraitFileDep]); } $invertedDependencies[$usedTraitFileDep]['usedTraitDependentFiles'][] = $file; @@ -1504,10 +1504,7 @@ private function save( continue; } - $invertedDependencies[$file] = [ - 'fileHash' => $currentFileHashes[$file] ?? $this->getFileHash($file), - 'dependentFiles' => [], - ]; + $invertedDependencies[$file] = $this->createDependencyEntry($file, $currentFileHashes); } ksort($errors); @@ -2162,6 +2159,112 @@ private function getMeta(array $allAnalysedFiles, ?array $projectConfigArray): a /** * The hash of a file that is depended on, which is allowed not to exist - see MISSING_FILE_HASH. */ + /** + * @param array $currentFileHashes + * @return array{fileHash: string, fileStat?: string, dependentFiles: list} + */ + private function createDependencyEntry(string $file, array $currentFileHashes): array + { + $entry = [ + 'fileHash' => $currentFileHashes[$file] ?? $this->getDependencyFileHash($file), + 'dependentFiles' => [], + ]; + if (array_key_exists($file, $currentFileHashes) && array_key_exists($file, $this->fileStatSignatures)) { + $entry['fileStat'] = $this->fileStatSignatures[$file]; + } + + return $entry; + } + + /** + * @param string[] $allAnalysedFiles + * @return array> the analysed files that exist, with their stat + */ + private function statAnalysedFiles(array $allAnalysedFiles): array + { + $stats = []; + foreach ($allAnalysedFiles as $analysedFile) { + $stat = @stat($analysedFile); + // what is_file() tells: it exists and is a regular file, symlinks followed + if ($stat === false || ($stat['mode'] & 0170000) !== 0100000) { + continue; + } + + $stats[$analysedFile] = $stat; + } + + return $stats; + } + + /** + * Hashing every analysed file is the bulk of what a run with nothing to re-analyse costs - on + * Drupal core almost a second for 11k files. A file whose size, mtime, ctime, inode and device + * are what they were when it was last hashed has not been written to since, so the hash the + * cache recorded then is reused - the check git makes against its index. ctime cannot be set + * back the way mtime can (touch, an extracted archive), and a replaced file has a new inode. + * + * A signature is only recorded for a file last modified before the second its hash was taken: + * the timestamps have a one-second granularity, so a file written again within that same second + * would keep a matching signature over different contents. On Windows the ctime PHP reports is + * the creation time, which a file rewritten in place keeps, so nothing is reused there. + * + * @param array> $analysedFileStats + * @param array, usedTraitDependentFiles?: list}>|null $cachedDependencies + * @return array + */ + private function hashAnalysedFiles(array $analysedFileStats, ?array $cachedDependencies): array + { + $now = time(); + $trustSignatures = DIRECTORY_SEPARATOR === '/'; + $hashes = []; + foreach ($analysedFileStats as $file => $stat) { + $signature = sprintf('%d:%d:%d:%d:%d', $stat['size'], $stat['mtime'], $stat['ctime'], $stat['ino'], $stat['dev']); + $cachedEntry = $cachedDependencies[$file] ?? null; + if ( + $trustSignatures + && $cachedEntry !== null + && ($cachedEntry['fileStat'] ?? null) === $signature + && $cachedEntry['fileHash'] !== self::MISSING_FILE_HASH + && !array_key_exists($file, $this->fileReplacements) + ) { + $hash = $cachedEntry['fileHash']; + $this->fileHashes[$file] = $hash; + } else { + $hash = $this->getFileHash($file); + } + + $hashes[$file] = $hash; + if (!$trustSignatures || $stat['mtime'] >= $now || $stat['ctime'] >= $now) { + continue; + } + + $this->fileStatSignatures[$file] = $signature; + } + + return $hashes; + } + + /** + * Whether a signature recorded by hashAnalysedFiles() differs from the one the restored cache + * holds for the file, which makes the cache worth rewriting even when nothing else changed. + * + * @param array, usedTraitDependentFiles?: list}> $cachedDependencies + */ + private function fileStatSignaturesDiffer(array $cachedDependencies): bool + { + foreach ($this->fileStatSignatures as $file => $signature) { + if (!array_key_exists($file, $cachedDependencies)) { + continue; + } + + if (($cachedDependencies[$file]['fileStat'] ?? null) !== $signature) { + return true; + } + } + + return false; + } + private function getDependencyFileHash(string $path): string { if (!is_file($path)) { diff --git a/src/Analyser/ResultCache/ResultCachePathTransformer.php b/src/Analyser/ResultCache/ResultCachePathTransformer.php index 2e051f5006e..4d27233f0c7 100644 --- a/src/Analyser/ResultCache/ResultCachePathTransformer.php +++ b/src/Analyser/ResultCache/ResultCachePathTransformer.php @@ -234,8 +234,8 @@ public function absolutizeCompoundKeyed(array $byFile): array } /** - * @param array, usedTraitDependentFiles?: list}> $dependencies - * @return array, usedTraitDependentFiles?: list}> + * @param array, usedTraitDependentFiles?: list}> $dependencies + * @return array, usedTraitDependentFiles?: list}> */ public function relativizeDependencies(array $dependencies): array { @@ -252,8 +252,8 @@ public function relativizeDependencies(array $dependencies): array } /** - * @param array, usedTraitDependentFiles?: list}> $dependencies - * @return array, usedTraitDependentFiles?: list}> + * @param array, usedTraitDependentFiles?: list}> $dependencies + * @return array, usedTraitDependentFiles?: list}> */ public function absolutizeDependencies(array $dependencies): array { From 79e2e5432143ac8fb0f3c9327b87000d96557f06 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 29 Sep 2026 00:46:53 +0200 Subject: [PATCH 04/10] Store the dependency graph of the result cache with a table of paths Written out with the paths in full, the graph repeats each file in the dependent list of everything it depends on. On Drupal core that is half a million paths and 42 MB, and each of them was converted between absolute and relative on every save and restore - about half a second. The graph now lists every file once in a path table and refers to it by position, so only the table is converted. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ --- .../ResultCache/ResultCacheManager.php | 123 +++++++++++++++++- .../ResultCachePathTransformer.php | 18 --- 2 files changed, 119 insertions(+), 22 deletions(-) diff --git a/src/Analyser/ResultCache/ResultCacheManager.php b/src/Analyser/ResultCache/ResultCacheManager.php index d6e5f22036c..b64f25aa1e2 100644 --- a/src/Analyser/ResultCache/ResultCacheManager.php +++ b/src/Analyser/ResultCache/ResultCacheManager.php @@ -100,7 +100,7 @@ final class ResultCacheManager */ private const EXTENSIONS_NOT_INVALIDATING_CACHE = ['xdebug', 'blackfire', 'phpstan_turbo']; - private const CACHE_VERSION = 'v19-sharedNamespaceUses'; + private const CACHE_VERSION = 'v20-dependencyGraph'; /** * The recorded hash of a dependency that does not exist. A rule can depend on a path rather than on @@ -363,7 +363,25 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? $data['projectExtensionFiles'] = $transformer->absolutizeFileKeyed($data['projectExtensionFiles']); $data['linesToIgnore'] = $transformer->absolutizeCompoundKeyed($data['linesToIgnore']); $data['unmatchedLineIgnores'] = $transformer->absolutizeCompoundKeyed($data['unmatchedLineIgnores']); - $data['dependencies'] = $transformer->absolutizeDependencies($data['dependencies']); + if (array_key_exists('dependencyGraph', $data)) { + try { + $data['dependencies'] = $this->decodeDependencyGraph($data['dependencyGraph'], $transformer); + } catch (Throwable $e) { + @unlink($cacheFilePath); + + return $this->fullAnalysis( + sprintf('Result cache not used because an error occurred while loading the cache file: %s', $e->getMessage()), + $allAnalysedFiles, + $this->getMeta($allAnalysedFiles, $projectConfigArray), + $this->hashAnalysedFiles($analysedFileStats, null), + $output, + ); + } + unset($data['dependencyGraph']); + } else { + // a cache written by an older version, which the cacheVersion check below discards + $data['dependencies'] = $transformer->absolutizeDependencies($data['dependencies'] ?? []); + } $currentFileHashes = $this->hashAnalysedFiles($analysedFileStats, $data['dependencies']); $fileStatSignaturesChanged = $this->fileStatSignaturesDiffer($data['dependencies']); $data['packageDependencies'] = $transformer->absolutizeFileKeyed($data['packageDependencies'] ?? []); @@ -1550,7 +1568,8 @@ private function save( $linesToIgnore = $transformer->relativizeCompoundKeyed($linesToIgnore); $unmatchedLineIgnores = $transformer->relativizeCompoundKeyed($unmatchedLineIgnores); $collectedData = $transformer->relativizeCollectedData($collectedData); - $invertedDependencies = $transformer->relativizeDependencies($invertedDependencies); + $dependencyGraph = $this->encodeDependencyGraph($invertedDependencies, $transformer); + unset($invertedDependencies); $packageDependencies = $transformer->relativizeFileKeyed($packageDependencies); $exportedNodes = $transformer->relativizeFileKeyed($exportedNodes); $projectExtensionFiles = $transformer->relativizeFileKeyed($projectExtensionFiles); @@ -1590,7 +1609,10 @@ private function save( $this->writeArrayFrame($handle, $file, 'linesToIgnore', $linesToIgnore); $this->writeArrayFrame($handle, $file, 'unmatchedLineIgnores', $unmatchedLineIgnores); $this->writeArrayFrame($handle, $file, 'collectedData', $collectedData); - $this->writeArrayFrame($handle, $file, 'dependencies', $invertedDependencies); + // An older PHPStan reading this file absolutizes the dependencies before it gets to the + // cacheVersion check that makes it discard the file, and fails on a missing section. + $this->writeArrayFrame($handle, $file, 'dependencies', []); + $this->writeValueFrame($handle, $file, 'dependencyGraph', $dependencyGraph); $this->writeArrayFrame($handle, $file, 'packageDependencies', $packageDependencies); $this->writeArrayFrame($handle, $file, 'exportedNodes', $exportedNodes); fclose($handle); @@ -1613,6 +1635,99 @@ private function save( } } + /** + * The dependency graph names every file once in a path table and refers to it by position. + * Written out entry by entry with the paths spelled in full, as it used to be, the graph repeats + * each file in the dependent lists of everything it depends on - on Drupal core half a million + * paths, 42 MB, each of them rewritten between absolute and relative on every save and restore. + * Now it is the 11k paths of the table, and the lists are integers. + * + * @param array, usedTraitDependentFiles?: list}> $invertedDependencies + * @return array{paths: list, entries: list, list|null}>} + */ + private function encodeDependencyGraph(array $invertedDependencies, ResultCachePathTransformer $transformer): array + { + $ids = []; + $paths = []; + $entries = []; + foreach ($invertedDependencies as $file => $fileData) { + if (!isset($ids[$file])) { + $ids[$file] = count($paths); + $paths[] = $file; + } + + $dependentIds = []; + foreach ($fileData['dependentFiles'] as $dependentFile) { + if (!isset($ids[$dependentFile])) { + $ids[$dependentFile] = count($paths); + $paths[] = $dependentFile; + } + $dependentIds[] = $ids[$dependentFile]; + } + + $usedTraitDependentIds = null; + if (array_key_exists('usedTraitDependentFiles', $fileData)) { + $usedTraitDependentIds = []; + foreach ($fileData['usedTraitDependentFiles'] as $dependentFile) { + if (!isset($ids[$dependentFile])) { + $ids[$dependentFile] = count($paths); + $paths[] = $dependentFile; + } + $usedTraitDependentIds[] = $ids[$dependentFile]; + } + } + + $entries[] = [$ids[$file], $fileData['fileHash'], $fileData['fileStat'] ?? null, $dependentIds, $usedTraitDependentIds]; + } + + $relativePaths = []; + foreach ($paths as $path) { + $relativePaths[] = $transformer->relativizePath($path); + } + + return ['paths' => $relativePaths, 'entries' => $entries]; + } + + /** + * @param mixed $dependencyGraph + * @return array, usedTraitDependentFiles?: list}> + */ + private function decodeDependencyGraph($dependencyGraph, ResultCachePathTransformer $transformer): array + { + if (!is_array($dependencyGraph) || !is_array($dependencyGraph['paths'] ?? null) || !is_array($dependencyGraph['entries'] ?? null)) { + throw new RuntimeException('The dependency graph is malformed.'); + } + + $paths = []; + foreach ($dependencyGraph['paths'] as $path) { + $paths[] = $transformer->absolutizePath($path); + } + + $dependencies = []; + foreach ($dependencyGraph['entries'] as [$fileId, $fileHash, $fileStat, $dependentIds, $usedTraitDependentIds]) { + $dependentFiles = []; + foreach ($dependentIds as $dependentId) { + $dependentFiles[] = $paths[$dependentId]; + } + + $entry = ['fileHash' => $fileHash, 'dependentFiles' => $dependentFiles]; + if ($fileStat !== null) { + $entry['fileStat'] = $fileStat; + } + if ($usedTraitDependentIds !== null) { + $usedTraitDependentFiles = []; + foreach ($usedTraitDependentIds as $dependentId) { + $usedTraitDependentFiles[] = $paths[$dependentId]; + } + $entry['usedTraitDependentFiles'] = $usedTraitDependentFiles; + } + + $dependencies[$paths[$fileId]] = $entry; + } + + return $dependencies; + } + /** * @param resource $handle */ diff --git a/src/Analyser/ResultCache/ResultCachePathTransformer.php b/src/Analyser/ResultCache/ResultCachePathTransformer.php index 4d27233f0c7..4413298b015 100644 --- a/src/Analyser/ResultCache/ResultCachePathTransformer.php +++ b/src/Analyser/ResultCache/ResultCachePathTransformer.php @@ -233,24 +233,6 @@ public function absolutizeCompoundKeyed(array $byFile): array return $result; } - /** - * @param array, usedTraitDependentFiles?: list}> $dependencies - * @return array, usedTraitDependentFiles?: list}> - */ - public function relativizeDependencies(array $dependencies): array - { - $result = []; - foreach ($dependencies as $file => $data) { - $data['dependentFiles'] = $this->relativizeList($data['dependentFiles']); - if (array_key_exists('usedTraitDependentFiles', $data)) { - $data['usedTraitDependentFiles'] = $this->relativizeList($data['usedTraitDependentFiles']); - } - $result[$this->relativizePath($file)] = $data; - } - - return $result; - } - /** * @param array, usedTraitDependentFiles?: list}> $dependencies * @return array, usedTraitDependentFiles?: list}> From f8a929cc2e0d76f5764848ea9386d93f6721db6a Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 29 Sep 2026 00:47:01 +0200 Subject: [PATCH 05/10] Leave the exported nodes in the result cache file until they are needed The exported nodes are most of the result cache - on Drupal core 135 MB of 160 MB. A run needs the decoded nodes of the few files that changed, and the rest only has to get into the next cache file unchanged. Decoding all of them on restore and serializing them again on save took about half a second of every run that re-analysed anything. The nodes are now stored as an index of files and lengths followed by the serialized nodes back to back. restore() decodes only the ones it compares, and save() copies the bytes of the others from the old file. The old file is closed before the new one is renamed over it, for Windows. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ --- .../ResultCache/CachedExportedNodes.php | 187 ++++++++++++++++ ...CachedExportedNodesUnreadableException.php | 13 ++ src/Analyser/ResultCache/ResultCache.php | 11 +- .../ResultCache/ResultCacheManager.php | 200 ++++++++++++++++-- .../ResultCache/CachedExportedNodesTest.php | 129 +++++++++++ 5 files changed, 526 insertions(+), 14 deletions(-) create mode 100644 src/Analyser/ResultCache/CachedExportedNodes.php create mode 100644 src/Analyser/ResultCache/CachedExportedNodesUnreadableException.php create mode 100644 tests/PHPStan/Analyser/ResultCache/CachedExportedNodesTest.php diff --git a/src/Analyser/ResultCache/CachedExportedNodes.php b/src/Analyser/ResultCache/CachedExportedNodes.php new file mode 100644 index 00000000000..80262fec3ed --- /dev/null +++ b/src/Analyser/ResultCache/CachedExportedNodes.php @@ -0,0 +1,187 @@ + $locations file => [offset, length] of its serialized nodes + */ + private function __construct( + private $handle, + private array $locations, + ) + { + } + + public static function createEmpty(): self + { + return new self(null, []); + } + + /** + * @param resource $handle + * @param array $locations + */ + public static function createFromFile($handle, array $locations): self + { + return new self($handle, $locations); + } + + public function has(string $file): bool + { + return array_key_exists($file, $this->locations); + } + + /** + * The length of the serialized nodes of a file this holds. + * + * @return positive-int + */ + public function getLength(string $file): int + { + if (!array_key_exists($file, $this->locations)) { + throw new CachedExportedNodesUnreadableException(sprintf('The exported nodes of %s are not in the cache file.', $file)); + } + + return $this->locations[$file][1]; + } + + /** + * @return list + */ + public function getFiles(): array + { + return array_keys($this->locations); + } + + /** + * @param array $files + */ + public function only(array $files): self + { + $locations = []; + foreach ($this->locations as $file => $location) { + if (!array_key_exists($file, $files)) { + continue; + } + + $locations[$file] = $location; + } + + return new self($this->handle, $locations); + } + + /** + * @param array $files + */ + public function without(array $files): self + { + $locations = $this->locations; + foreach (array_keys($files) as $file) { + unset($locations[$file]); + } + + return new self($this->handle, $locations); + } + + /** + * @return array + */ + public function decode(string $file): array + { + $nodes = @unserialize($this->read($file)); + if (!is_array($nodes)) { + throw new CachedExportedNodesUnreadableException(sprintf('The exported nodes of %s could not be unserialized.', $file)); + } + + /** @var array $nodes */ + return $nodes; + } + + /** + * The serialized nodes, as they are in the file. + */ + public function read(string $file): string + { + if (!array_key_exists($file, $this->locations) || !is_resource($this->handle)) { + throw new CachedExportedNodesUnreadableException(sprintf('The exported nodes of %s are not in the cache file.', $file)); + } + + [$offset, $length] = $this->locations[$file]; + if (fseek($this->handle, $offset) !== 0) { + throw new CachedExportedNodesUnreadableException(sprintf('Cannot seek to the exported nodes of %s.', $file)); + } + + $contents = fread($this->handle, $length); + if ($contents === false || strlen($contents) !== $length) { + throw new CachedExportedNodesUnreadableException(sprintf('Cannot read the exported nodes of %s.', $file)); + } + + return $contents; + } + + public function getOffset(string $file): int + { + if (!array_key_exists($file, $this->locations)) { + throw new CachedExportedNodesUnreadableException(sprintf('The exported nodes of %s are not in the cache file.', $file)); + } + + return $this->locations[$file][0]; + } + + /** + * Bytes of the cache file - the serialized nodes of several files stored one after another. + * + * @param positive-int $length + */ + public function readRange(int $offset, int $length): string + { + if (!is_resource($this->handle) || fseek($this->handle, $offset) !== 0) { + throw new CachedExportedNodesUnreadableException(sprintf('Cannot seek to offset %d of the cache file.', $offset)); + } + + $contents = fread($this->handle, $length); + if ($contents === false || strlen($contents) !== $length) { + throw new CachedExportedNodesUnreadableException(sprintf('Cannot read %d bytes at offset %d of the cache file.', $length, $offset)); + } + + return $contents; + } + + public function close(): void + { + if (is_resource($this->handle)) { + fclose($this->handle); + } + + $this->handle = null; + } + +} diff --git a/src/Analyser/ResultCache/CachedExportedNodesUnreadableException.php b/src/Analyser/ResultCache/CachedExportedNodesUnreadableException.php new file mode 100644 index 00000000000..f37295b8233 --- /dev/null +++ b/src/Analyser/ResultCache/CachedExportedNodesUnreadableException.php @@ -0,0 +1,13 @@ +> $dependencies * @param array> $usedTraitDependencies * @param array> $packageDependencies - * @param array> $exportedNodes + * @param array> $exportedNodes the decoded ones - see $cachedExportedNodes for the rest * @param array $projectExtensionFiles * @param array $currentFileHashes */ @@ -44,6 +44,7 @@ public function __construct( private array $usedTraitDependencies, private array $packageDependencies, private array $exportedNodes, + private CachedExportedNodes $cachedExportedNodes, private array $projectExtensionFiles, private array $currentFileHashes, ) @@ -157,6 +158,14 @@ public function getExportedNodes(): array return $this->exportedNodes; } + /** + * The exported nodes carried over from the cache file undecoded, for the files not in getExportedNodes(). + */ + public function getCachedExportedNodes(): CachedExportedNodes + { + return $this->cachedExportedNodes; + } + /** * @return array */ diff --git a/src/Analyser/ResultCache/ResultCacheManager.php b/src/Analyser/ResultCache/ResultCacheManager.php index b64f25aa1e2..bb29cf77fed 100644 --- a/src/Analyser/ResultCache/ResultCacheManager.php +++ b/src/Analyser/ResultCache/ResultCacheManager.php @@ -62,8 +62,11 @@ use function is_array; use function is_dir; use function is_file; +use function is_int; +use function is_string; use function ksort; use function microtime; +use function min; use function rename; use function rtrim; use function serialize; @@ -100,7 +103,7 @@ final class ResultCacheManager */ private const EXTENSIONS_NOT_INVALIDATING_CACHE = ['xdebug', 'blackfire', 'phpstan_turbo']; - private const CACHE_VERSION = 'v20-dependencyGraph'; + private const CACHE_VERSION = 'v21-exportedNodesIndex'; /** * The recorded hash of a dependency that does not exist. A rule can depend on a path rather than on @@ -274,6 +277,7 @@ private function fullAnalysis( usedTraitDependencies: [], packageDependencies: [], exportedNodes: [], + cachedExportedNodes: CachedExportedNodes::createEmpty(), projectExtensionFiles: [], currentFileHashes: $currentFileHashes, ); @@ -395,6 +399,15 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? $exportedNodesCallback = $data['exportedNodesCallback']; $data['exportedNodesCallback'] = static fn (): array => $transformer->absolutizeFileKeyed($exportedNodesCallback()); + $cachedExportedNodes = CachedExportedNodes::createEmpty(); + if (array_key_exists('exportedNodesLocations', $data) && is_array($data['exportedNodesLocations'])) { + $exportedNodesLocations = []; + foreach ($data['exportedNodesLocations'] as $exportedNodesFile => $exportedNodesLocation) { + $exportedNodesLocations[$transformer->absolutizePath($exportedNodesFile)] = $exportedNodesLocation; + } + $cachedExportedNodes = CachedExportedNodes::createFromFile($data['cacheFileHandle'], $exportedNodesLocations); + } + // The stub file hashes get into the meta only at save time, after the analysis - the // only point where the StubFilesExtensions may run, because they can rely on // bootstrapFiles having been executed. Restoring must not run them, so the entry is @@ -685,8 +698,20 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? $filteredUnmatchedLineIgnores = []; $filteredCollectedData = []; $filteredExportedNodes = []; + // the analysed and scanned files whose exported nodes stay in the old cache file until save() + $keptCachedExportedNodes = []; $newFileAppeared = false; $dependencyFilesChanged = false; + $cachedExportedNodesError = null; + $decodeCachedExportedNodes = static function (string $file) use ($cachedExportedNodes, &$cachedExportedNodesError): array { + try { + return $cachedExportedNodes->decode($file); + } catch (CachedExportedNodesUnreadableException $e) { + $cachedExportedNodesError ??= $e->getMessage(); + + return []; + } + }; foreach (array_keys($cachedStubFiles) as $stubFile) { if (!array_key_exists($stubFile, $errors)) { @@ -714,6 +739,8 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? } if (array_key_exists($analysedFile, $exportedNodes)) { $filteredExportedNodes[$analysedFile] = $exportedNodes[$analysedFile]; + } elseif ($cachedExportedNodes->has($analysedFile)) { + $keptCachedExportedNodes[$analysedFile] = true; } if (!array_key_exists($analysedFile, $invertedDependencies)) { // new file @@ -744,7 +771,8 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? // the file may have gained its first symbol, which is exactly what the files with errors are // waiting for. Comparing against an empty list says so, and says nothing changed when the // file still declares nothing. - $cachedFileExportedNodes = $filteredExportedNodes[$analysedFile] ?? []; + $cachedFileExportedNodes = $filteredExportedNodes[$analysedFile] + ?? ($cachedExportedNodes->has($analysedFile) ? $decodeCachedExportedNodes($analysedFile) : []); $exportedNodesChanged = $this->exportedNodesChanged($analysedFile, $cachedFileExportedNodes); if ($exportedNodesChanged === null) { if (count($cachedFileExportedNodes) === 0) { @@ -822,14 +850,18 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? $invertedUsedTraitDependenciesToReturn[$notAnalysedFile] = $usedTraitDependentFiles; } - $cachedFileExportedNodes = $exportedNodes[$notAnalysedFile] ?? null; if ($this->getFileHash($notAnalysedFile) === $notAnalysedFileData['fileHash']) { - if ($cachedFileExportedNodes !== null) { - $filteredExportedNodes[$notAnalysedFile] = $cachedFileExportedNodes; + if (array_key_exists($notAnalysedFile, $exportedNodes)) { + $filteredExportedNodes[$notAnalysedFile] = $exportedNodes[$notAnalysedFile]; + } elseif ($cachedExportedNodes->has($notAnalysedFile)) { + $keptCachedExportedNodes[$notAnalysedFile] = true; } continue; } + $cachedFileExportedNodes = $exportedNodes[$notAnalysedFile] + ?? ($cachedExportedNodes->has($notAnalysedFile) ? $decodeCachedExportedNodes($notAnalysedFile) : null); + $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 - @@ -880,6 +912,18 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? } } + if ($cachedExportedNodesError !== null) { + @unlink($cacheFilePath); + + return $this->fullAnalysis( + sprintf('Result cache not used because the cached results could not be read back: %s', $cachedExportedNodesError), + $allAnalysedFiles, + $meta, + $currentFileHashes, + $output, + ); + } + if ($newFileAppeared || $notAnalysedFileSymbolsChanged) { foreach (array_keys($filteredErrors) as $fileWithError) { $filesToAnalyse[] = $fileWithError; @@ -935,6 +979,7 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? usedTraitDependencies: $invertedUsedTraitDependenciesToReturn, packageDependencies: $packageDependencies, exportedNodes: $filteredExportedNodes, + cachedExportedNodes: $cachedExportedNodes->only($keptCachedExportedNodes), projectExtensionFiles: $data['projectExtensionFiles'], currentFileHashes: $currentFileHashes, ); @@ -1068,7 +1113,7 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache $projectConfigArray = $this->getPathTransformer()->relativizeProjectConfig($projectConfigArray); $meta['projectConfig'] = Neon::encode($projectConfigArray); } - $doSave = function (array $errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, ?array $dependencies, ?array $usedTraitDependencies, ?array $packageDependencies, array $exportedNodes, array $projectExtensionFiles) use ($internalErrors, $resultCache, $output, $onlyFiles, $meta): bool { + $doSave = function (array $errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, ?array $dependencies, ?array $usedTraitDependencies, ?array $packageDependencies, array $exportedNodes, CachedExportedNodes $cachedExportedNodes, array $projectExtensionFiles) use ($internalErrors, $resultCache, $output, $onlyFiles, $meta): bool { if ($onlyFiles) { if ($output->isVeryVerbose()) { $output->writeLineFormatted('Result cache was not saved because only files were passed as analysed paths.'); @@ -1134,6 +1179,7 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache && $collectedDataByFile === $resultCache->getCollectedData() && $packageDependencies === $resultCache->getPackageDependencies() && $exportedNodes === $resultCache->getExportedNodes() + && $cachedExportedNodes->getFiles() === $resultCache->getCachedExportedNodes()->getFiles() && $projectExtensionFiles === $resultCache->getProjectExtensionFiles() && $stubFiles === $this->restoredStubFiles && is_file($this->cacheFilePath) @@ -1145,7 +1191,7 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache return true; } - $this->save($resultCache->getLastFullAnalysisTime(), $errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, $dependencies, $usedTraitDependencies, $packageDependencies, $exportedNodes, $projectExtensionFiles, $resultCache->getCurrentFileHashes(), $meta, $stubFiles); + $this->save($resultCache->getLastFullAnalysisTime(), $errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, $dependencies, $usedTraitDependencies, $packageDependencies, $exportedNodes, $cachedExportedNodes, $projectExtensionFiles, $resultCache->getCurrentFileHashes(), $meta, $stubFiles); if ($output->isVeryVerbose()) { $output->writeLineFormatted('Result cache is saved.'); @@ -1161,7 +1207,7 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache if ($analyserResult->getDependencies() !== null) { $projectExtensionFiles = $this->getProjectExtensionFiles($projectConfigArray, $analyserResult->getDependencies()); } - $saved = $doSave($freshErrorsByFile, $freshLocallyIgnoredErrorsByFile, $analyserResult->getLinesToIgnore(), $analyserResult->getUnmatchedLineIgnores(), $freshCollectedDataByFile, $analyserResult->getDependencies(), $analyserResult->getUsedTraitDependencies(), $analyserResult->getPackageDependencies(), $this->addNonAnalysedExportedNodes($analyserResult->getExportedNodes(), $analyserResult->getDependencies(), $analyserResult->getUsedTraitDependencies()), $projectExtensionFiles); + $saved = $doSave($freshErrorsByFile, $freshLocallyIgnoredErrorsByFile, $analyserResult->getLinesToIgnore(), $analyserResult->getUnmatchedLineIgnores(), $freshCollectedDataByFile, $analyserResult->getDependencies(), $analyserResult->getUsedTraitDependencies(), $analyserResult->getPackageDependencies(), $this->addNonAnalysedExportedNodes($analyserResult->getExportedNodes(), $analyserResult->getDependencies(), $analyserResult->getUsedTraitDependencies(), CachedExportedNodes::createEmpty()), CachedExportedNodes::createEmpty(), $projectExtensionFiles); } else { if ($output->isVeryVerbose()) { $output->writeLineFormatted('Result cache was not saved because it was not requested.'); @@ -1177,7 +1223,9 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache $dependencies = $this->mergeDependencies($resultCache->getDependencies(), $resultCache->getFilesToAnalyse(), $analyserResult->getDependencies()); $usedTraitDependencies = $this->mergeDependencies($resultCache->getUsedTraitDependencies(), $resultCache->getFilesToAnalyse(), $analyserResult->getUsedTraitDependencies()); $packageDependencies = $this->mergePackageDependencies($resultCache->getPackageDependencies(), $resultCache->getFilesToAnalyse(), $analyserResult->getPackageDependencies()); - $exportedNodes = $this->addNonAnalysedExportedNodes($this->mergeExportedNodes($resultCache, $analyserResult->getExportedNodes()), $dependencies, $usedTraitDependencies); + // the re-analysed files take their fresh nodes, what is left of the cached ones stays undecoded + $cachedExportedNodes = $resultCache->getCachedExportedNodes()->without(array_fill_keys($resultCache->getFilesToAnalyse(), true)); + $exportedNodes = $this->addNonAnalysedExportedNodes($this->mergeExportedNodes($resultCache, $analyserResult->getExportedNodes()), $dependencies, $usedTraitDependencies, $cachedExportedNodes); $linesToIgnore = $this->mergeLinesToIgnore($resultCache, $analyserResult->getLinesToIgnore()); $unmatchedLineIgnores = $this->mergeUnmatchedLineIgnores($resultCache, $analyserResult->getUnmatchedLineIgnores()); @@ -1203,7 +1251,7 @@ public function process(AnalyserResult $analyserResult, ResultCache $resultCache $projectExtensionFiles[$file] = [$hash, true, $className]; } } - $saved = $doSave($errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, $dependencies, $usedTraitDependencies, $packageDependencies, $exportedNodes, $projectExtensionFiles); + $saved = $doSave($errorsByFile, $locallyIgnoredErrorsByFile, $linesToIgnore, $unmatchedLineIgnores, $collectedDataByFile, $dependencies, $usedTraitDependencies, $packageDependencies, $exportedNodes, $cachedExportedNodes, $projectExtensionFiles); } $flatErrors = []; @@ -1485,6 +1533,7 @@ private function save( array $usedTraitDependencies, array $packageDependencies, array $exportedNodes, + CachedExportedNodes $cachedExportedNodes, array $projectExtensionFiles, array $currentFileHashes, array $meta, @@ -1571,7 +1620,6 @@ private function save( $dependencyGraph = $this->encodeDependencyGraph($invertedDependencies, $transformer); unset($invertedDependencies); $packageDependencies = $transformer->relativizeFileKeyed($packageDependencies); - $exportedNodes = $transformer->relativizeFileKeyed($exportedNodes); $projectExtensionFiles = $transformer->relativizeFileKeyed($projectExtensionFiles); $file = $this->cacheFilePath; @@ -1614,10 +1662,14 @@ private function save( $this->writeArrayFrame($handle, $file, 'dependencies', []); $this->writeValueFrame($handle, $file, 'dependencyGraph', $dependencyGraph); $this->writeArrayFrame($handle, $file, 'packageDependencies', $packageDependencies); - $this->writeArrayFrame($handle, $file, 'exportedNodes', $exportedNodes); + $this->writeExportedNodes($handle, $file, $exportedNodes, $cachedExportedNodes, $transformer); fclose($handle); $closed = true; + // the old file is where the cached exported nodes were copied from, and Windows does not + // replace a file that is still open + $cachedExportedNodes->close(); + if (!@rename($temporaryFile, $file)) { $error = error_get_last(); throw new CouldNotWriteFileException($file, $error !== null ? $error['message'] : 'unknown cause'); @@ -1728,6 +1780,93 @@ private function decodeDependencyGraph($dependencyGraph, ResultCachePathTransfor return $dependencies; } + /** + * The exported nodes, as an index of the files and the lengths of their serialized nodes, followed + * by the serialized nodes back to back - so that the next run can find the few it needs without + * decoding the rest, and pass the rest on by copying their bytes, as this does with the ones it + * did not decode. + * + * @param resource $handle + * @param array> $exportedNodes + */ + private function writeExportedNodes($handle, string $file, array $exportedNodes, CachedExportedNodes $cachedExportedNodes, ResultCachePathTransformer $transformer): void + { + $serializedNodes = []; + foreach ($exportedNodes as $exportedNodesFile => $fileExportedNodes) { + $serializedNodes[$exportedNodesFile] = serialize($fileExportedNodes); + } + + $files = []; + foreach ($cachedExportedNodes->getFiles() as $cachedFile) { + $files[$cachedFile] = true; + } + foreach (array_keys($serializedNodes) as $exportedNodesFile) { + $files[$exportedNodesFile] = true; + } + ksort($files); + + $index = []; + $size = 0; + foreach (array_keys($files) as $exportedNodesFile) { + $length = array_key_exists($exportedNodesFile, $serializedNodes) + ? strlen($serializedNodes[$exportedNodesFile]) + : $cachedExportedNodes->getLength($exportedNodesFile); + $index[] = [$transformer->relativizePath($exportedNodesFile), $length]; + $size += $length; + } + + $this->writeValueFrame($handle, $file, 'exportedNodesIndex', $index); + $this->writeToHandle($handle, $file, 'exportedNodes# ' . $size . "\n"); + + // The files keep their order from one cache file to the next, so the cached nodes come in + // long runs stored one after another in the old file - each run is copied in a few large + // reads instead of one per file. + $runOffset = null; + $runLength = 0; + foreach (array_keys($files) as $exportedNodesFile) { + if (array_key_exists($exportedNodesFile, $serializedNodes)) { + $this->copyCachedExportedNodesRun($handle, $file, $cachedExportedNodes, $runOffset, $runLength); + $runOffset = null; + $runLength = 0; + $this->writeToHandle($handle, $file, $serializedNodes[$exportedNodesFile]); + continue; + } + + $offset = $cachedExportedNodes->getOffset($exportedNodesFile); + $length = $cachedExportedNodes->getLength($exportedNodesFile); + if ($runOffset !== null && $runOffset + $runLength === $offset) { + $runLength += $length; + continue; + } + + $this->copyCachedExportedNodesRun($handle, $file, $cachedExportedNodes, $runOffset, $runLength); + $runOffset = $offset; + $runLength = $length; + } + + $this->copyCachedExportedNodesRun($handle, $file, $cachedExportedNodes, $runOffset, $runLength); + } + + /** + * @param resource $handle + */ + private function copyCachedExportedNodesRun($handle, string $file, CachedExportedNodes $cachedExportedNodes, ?int $offset, int $length): void + { + if ($offset === null) { + return; + } + + $chunkSize = 8 * 1024 * 1024; + for ($copied = 0; $copied < $length; $copied += $chunkSize) { + $chunkLength = min($chunkSize, $length - $copied); + if ($chunkLength <= 0) { + break; + } + + $this->writeToHandle($handle, $file, $cachedExportedNodes->readRange($offset + $copied, $chunkLength)); + } + } + /** * @param resource $handle */ @@ -1809,6 +1948,39 @@ private function readCacheFile(string $cacheFilePath): ?array } [$name, $size] = $parts; + if (str_ends_with($name, '#')) { + // payloads stored back to back, their paths and lengths in the index frame before them + $name = substr($name, 0, -1); + $size = (int) $size; + $offset = ftell($handle); + $index = $data[$name . 'Index'] ?? null; + if ($offset === false || !is_array($index)) { + throw new RuntimeException(sprintf('Section "%s" has no index.', $name)); + } + + $locations = []; + $position = $offset; + $count = count($index); + foreach ($index as $i => [$indexedFile, $length]) { + if (!is_string($indexedFile) || !is_int($length) || $length <= 0) { + throw new RuntimeException(sprintf('Section "%s" has a malformed index.', $name)); + } + + $locations[$indexedFile] = [$position, $length]; + $position += $length; + if ($position > $fileSize) { + throw new RuntimeException(sprintf('Section "%s" is truncated at entry %d of %d.', $name, $i, $count)); + } + } + if ($position !== $offset + $size || fseek($handle, $size, SEEK_CUR) !== 0) { + throw new RuntimeException(sprintf('Section "%s" does not match its index.', $name)); + } + + unset($data[$name . 'Index']); + $data[$name . 'Locations'] = $locations; + continue; + } + if (!str_ends_with($name, '*')) { $data[$name] = $this->readFrame($handle, (int) $size); @@ -1844,6 +2016,7 @@ private function readCacheFile(string $cacheFilePath): ?array $data[$name . 'Callback'] = static fn (): array => []; } + $data['cacheFileHandle'] = $handle; $closeHandle = false; return $data; @@ -2183,7 +2356,7 @@ private function hasTraitNode(array $exportedNodes): bool * @param array>|null $usedTraitDependencies * @return array> */ - private function addNonAnalysedExportedNodes(array $exportedNodes, ?array $dependencies, ?array $usedTraitDependencies): array + private function addNonAnalysedExportedNodes(array $exportedNodes, ?array $dependencies, ?array $usedTraitDependencies, CachedExportedNodes $cachedExportedNodes): array { if ($dependencies === null || $usedTraitDependencies === null) { return $exportedNodes; @@ -2196,6 +2369,7 @@ private function addNonAnalysedExportedNodes(array $exportedNodes, ?array $depen if ( array_key_exists($dependencyFile, $dependencies) || array_key_exists($dependencyFile, $exportedNodes) + || $cachedExportedNodes->has($dependencyFile) || !is_file($dependencyFile) ) { continue; diff --git a/tests/PHPStan/Analyser/ResultCache/CachedExportedNodesTest.php b/tests/PHPStan/Analyser/ResultCache/CachedExportedNodesTest.php new file mode 100644 index 00000000000..9edafb75905 --- /dev/null +++ b/tests/PHPStan/Analyser/ResultCache/CachedExportedNodesTest.php @@ -0,0 +1,129 @@ + */ + private array $locations; + + #[Override] + protected function setUp(): void + { + parent::setUp(); + + $file = tempnam(sys_get_temp_dir(), 'phpstan-cached-nodes-'); + $this->assertIsString($file); + $this->file = $file; + + $header = "some other section\n"; + $first = serialize([$this->node('First')]); + $second = serialize([$this->node('Second'), $this->node('Another')]); + file_put_contents($this->file, $header . $first . $second); + $firstLength = strlen($first); + $secondLength = strlen($second); + if ($firstLength === 0 || $secondLength === 0) { + $this->fail('Serialized nodes are never empty.'); + } + + $this->locations = [ + '/project/First.php' => [strlen($header), $firstLength], + '/project/Second.php' => [strlen($header) + $firstLength, $secondLength], + ]; + } + + #[Override] + protected function tearDown(): void + { + @unlink($this->file); + parent::tearDown(); + } + + private function node(string $name): ExportedTraitNode + { + return new ExportedTraitNode($name, null, [], [], [], []); + } + + private function create(): CachedExportedNodes + { + $handle = fopen($this->file, 'r'); + $this->assertNotFalse($handle); + + return CachedExportedNodes::createFromFile($handle, $this->locations); + } + + public function testDecodesOneFileWithoutTheOthers(): void + { + $nodes = $this->create(); + + $this->assertEquals([$this->node('Second'), $this->node('Another')], $nodes->decode('/project/Second.php')); + $this->assertEquals([$this->node('First')], $nodes->decode('/project/First.php')); + $this->assertSame(serialize([$this->node('First')]), $nodes->read('/project/First.php')); + $this->assertSame(strlen(serialize([$this->node('First')])), $nodes->getLength('/project/First.php')); + $this->assertSame( + serialize([$this->node('First')]) . serialize([$this->node('Second'), $this->node('Another')]), + $nodes->readRange($nodes->getOffset('/project/First.php'), $nodes->getLength('/project/First.php') + $nodes->getLength('/project/Second.php')), + ); + } + + public function testOnlyAndWithout(): void + { + $nodes = $this->create(); + + $this->assertSame(['/project/First.php', '/project/Second.php'], $nodes->getFiles()); + $this->assertSame(['/project/Second.php'], $nodes->only(['/project/Second.php' => true, '/project/Unknown.php' => true])->getFiles()); + $this->assertSame(['/project/First.php'], $nodes->without(['/project/Second.php' => true])->getFiles()); + $this->assertFalse($nodes->without(['/project/Second.php' => true])->has('/project/Second.php')); + $this->assertTrue($nodes->has('/project/Second.php')); + // the instances share the file + $this->assertEquals([$this->node('First')], $nodes->only(['/project/First.php' => true])->decode('/project/First.php')); + } + + public function testUnknownFile(): void + { + $this->expectException(CachedExportedNodesUnreadableException::class); + $this->create()->decode('/project/Unknown.php'); + } + + public function testTruncatedFile(): void + { + $nodes = $this->create(); + file_put_contents($this->file, 'too short'); + + $this->expectException(CachedExportedNodesUnreadableException::class); + $nodes->decode('/project/Second.php'); + } + + public function testClosed(): void + { + $nodes = $this->create(); + $nodes->close(); + + $this->expectException(CachedExportedNodesUnreadableException::class); + $nodes->read('/project/First.php'); + } + + public function testEmpty(): void + { + $nodes = CachedExportedNodes::createEmpty(); + + $this->assertSame([], $nodes->getFiles()); + $this->assertFalse($nodes->has('/project/First.php')); + $nodes->close(); + } + +} From 3e2db51cd4ae8c0a628a4e0719e4b64aa01abaf1 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Tue, 29 Sep 2026 10:16:51 +0200 Subject: [PATCH 06/10] Do not parse a changed file when the outcome cannot add anything restore() parses every changed file to compare its exported nodes with the cached ones, which decides whether its dependents, the classes using a trait it declares, and the files with errors are re-analysed too. When all of those files changed themselves and are re-analysed anyway, the answer cannot add anything, so the file is not parsed. After a branch switch or a formatting run that touched most of the project that is most of the changed files. On Drupal core with every file edited, restoring took 9 seconds of parsing in the main process, and the workers forked from it were slower too. The files re-analysed stay the same. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ --- .../ResultCache/ResultCacheManager.php | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/Analyser/ResultCache/ResultCacheManager.php b/src/Analyser/ResultCache/ResultCacheManager.php index bb29cf77fed..9b4dea21d8a 100644 --- a/src/Analyser/ResultCache/ResultCacheManager.php +++ b/src/Analyser/ResultCache/ResultCacheManager.php @@ -721,6 +721,36 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? $filteredErrors[$stubFile] = $errors[$stubFile]; } + // Whether a changed file's exported nodes changed decides what else has to be re-analysed with + // it: its dependents, the classes using a trait it declares, and - when a symbol appeared or + // disappeared - the files with errors. Finding out means parsing the file, serially here in + // the main process. When all of those files are re-analysed anyway, because they changed + // themselves, the answer cannot add anything and the file is not parsed. After a branch switch + // or a formatting run that touched most of the project, that is most of the changed files - + // on Drupal core with every file edited, 9 seconds of parsing, and workers forked from a main + // process bloated by the parsed files. + $changedAnalysedFiles = []; + foreach ($allAnalysedFiles as $analysedFile) { + if ( + array_key_exists($analysedFile, $invertedDependencies) + && $invertedDependencies[$analysedFile]['fileHash'] === $currentFileHashes[$analysedFile] + ) { + continue; + } + + $changedAnalysedFiles[$analysedFile] = true; + } + // only the stub files with errors so far, which are never analysed + $filesWithErrorsAllChanged = $filteredErrors === []; + foreach ($allAnalysedFiles as $analysedFile) { + if (!array_key_exists($analysedFile, $errors) || array_key_exists($analysedFile, $changedAnalysedFiles)) { + continue; + } + + $filesWithErrorsAllChanged = false; + break; + } + foreach ($allAnalysedFiles as $analysedFile) { if (array_key_exists($analysedFile, $errors)) { $filteredErrors[$analysedFile] = $errors[$analysedFile]; @@ -766,6 +796,14 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? } $filesToAnalyse[] = $analysedFile; + if ( + ($filesWithErrorsAllChanged || $newFileAppeared) + && $this->areAllChanged($dependentFiles, $changedAnalysedFiles) + && $this->areAllChanged($usedTraitDependentFiles, $changedAnalysedFiles) + ) { + continue; + } + // A file that declared nothing has no entry at all - save() only writes one for a file with // at least one exported node - and a missing entry is not the same as nothing to propagate: // the file may have gained its first symbol, which is exactly what the files with errors are @@ -1044,6 +1082,21 @@ private function getMetaKeyDifferences(array $cachedMeta, array $currentMeta): a return $diffs; } + /** + * @param list $files + * @param array $changedFiles + */ + private function areAllChanged(array $files, array $changedFiles): bool + { + foreach ($files as $file) { + if (!array_key_exists($file, $changedFiles)) { + return false; + } + } + + return true; + } + /** * @param array $cachedFileExportedNodes * @return bool|null null means nothing changed, true means new root symbol appeared, false means nested node changed From 866f50d33d767a516e28663fcf054debdd3f9bd7 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 30 Sep 2026 13:30:55 +0200 Subject: [PATCH 07/10] Check stat signatures in one place DirectoryWalker and ResultCacheManager each built a stat signature, decided on their own whether it could be trusted - not on Windows, not for a file modified in the second the reading began - and had to take the time before reading anything for that to hold. FileStatSignatures::begin() takes the time, and the reader it returns gives a signature only when it can be trusted, so the callers just compare and keep what they get. The directory listings use the same signature as the files now. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ --- .../ResultCache/ResultCacheManager.php | 36 ++++----- src/File/DirectoryWalker.php | 32 ++++---- src/File/FileStatSignatureReader.php | 55 +++++++++++++ src/File/FileStatSignatures.php | 33 ++++++++ tests/PHPStan/File/DirectoryWalkerTest.php | 20 ++--- .../File/FileStatSignatureReaderTest.php | 81 +++++++++++++++++++ 6 files changed, 209 insertions(+), 48 deletions(-) create mode 100644 src/File/FileStatSignatureReader.php create mode 100644 src/File/FileStatSignatures.php create mode 100644 tests/PHPStan/File/FileStatSignatureReaderTest.php diff --git a/src/Analyser/ResultCache/ResultCacheManager.php b/src/Analyser/ResultCache/ResultCacheManager.php index 9b4dea21d8a..3fa5c4807dc 100644 --- a/src/Analyser/ResultCache/ResultCacheManager.php +++ b/src/Analyser/ResultCache/ResultCacheManager.php @@ -22,6 +22,7 @@ use PHPStan\File\CouldNotWriteFileException; use PHPStan\File\FileFinder; use PHPStan\File\FileHelper; +use PHPStan\File\FileStatSignatures; use PHPStan\Internal\ArrayHelper; use PHPStan\Internal\ComposerHelper; use PHPStan\Php\ComposerPhpVersionFactory; @@ -81,7 +82,6 @@ use function uniqid; use function unlink; use function unserialize; -use const DIRECTORY_SEPARATOR; use const PHP_VERSION_ID; use const SEEK_CUR; @@ -158,7 +158,7 @@ final class ResultCacheManager * * @var array */ - private array $fileStatSignatures = []; + private array $recordedFileStats = []; private ?ResultCachePathTransformer $pathTransformer = null; @@ -223,6 +223,7 @@ public function __construct( private string $anchorDirectory, private PhpVersion $phpVersion, private ComposerPhpVersionFactory $composerPhpVersionFactory, + private FileStatSignatures $fileStatSignatures, ) { } @@ -291,7 +292,7 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ? { $this->restoredCacheUnchanged = false; $this->restoredStubFiles = []; - $this->fileStatSignatures = []; + $this->recordedFileStats = []; $startTime = microtime(true); $analysedFileStats = $this->statAnalysedFiles($allAnalysedFiles); @@ -2511,8 +2512,8 @@ private function createDependencyEntry(string $file, array $currentFileHashes): 'fileHash' => $currentFileHashes[$file] ?? $this->getDependencyFileHash($file), 'dependentFiles' => [], ]; - if (array_key_exists($file, $currentFileHashes) && array_key_exists($file, $this->fileStatSignatures)) { - $entry['fileStat'] = $this->fileStatSignatures[$file]; + if (array_key_exists($file, $currentFileHashes) && array_key_exists($file, $this->recordedFileStats)) { + $entry['fileStat'] = $this->recordedFileStats[$file]; } return $entry; @@ -2540,15 +2541,9 @@ private function statAnalysedFiles(array $allAnalysedFiles): array /** * Hashing every analysed file is the bulk of what a run with nothing to re-analyse costs - on - * Drupal core almost a second for 11k files. A file whose size, mtime, ctime, inode and device - * are what they were when it was last hashed has not been written to since, so the hash the - * cache recorded then is reused - the check git makes against its index. ctime cannot be set - * back the way mtime can (touch, an extracted archive), and a replaced file has a new inode. - * - * A signature is only recorded for a file last modified before the second its hash was taken: - * the timestamps have a one-second granularity, so a file written again within that same second - * would keep a matching signature over different contents. On Windows the ctime PHP reports is - * the creation time, which a file rewritten in place keeps, so nothing is reused there. + * Drupal core almost a second for 11k files. A file whose stat signature is what it was when it + * was last hashed has not been written to since, so the hash the cache recorded then is reused + * - see FileStatSignatures. * * @param array> $analysedFileStats * @param array, usedTraitDependentFiles?: list}>|null $cachedDependencies @@ -2556,14 +2551,13 @@ private function statAnalysedFiles(array $allAnalysedFiles): array */ private function hashAnalysedFiles(array $analysedFileStats, ?array $cachedDependencies): array { - $now = time(); - $trustSignatures = DIRECTORY_SEPARATOR === '/'; + $signatures = $this->fileStatSignatures->begin(); $hashes = []; foreach ($analysedFileStats as $file => $stat) { - $signature = sprintf('%d:%d:%d:%d:%d', $stat['size'], $stat['mtime'], $stat['ctime'], $stat['ino'], $stat['dev']); + $signature = $signatures->fromStat($stat); $cachedEntry = $cachedDependencies[$file] ?? null; if ( - $trustSignatures + $signature !== null && $cachedEntry !== null && ($cachedEntry['fileStat'] ?? null) === $signature && $cachedEntry['fileHash'] !== self::MISSING_FILE_HASH @@ -2576,11 +2570,11 @@ private function hashAnalysedFiles(array $analysedFileStats, ?array $cachedDepen } $hashes[$file] = $hash; - if (!$trustSignatures || $stat['mtime'] >= $now || $stat['ctime'] >= $now) { + if ($signature === null) { continue; } - $this->fileStatSignatures[$file] = $signature; + $this->recordedFileStats[$file] = $signature; } return $hashes; @@ -2594,7 +2588,7 @@ private function hashAnalysedFiles(array $analysedFileStats, ?array $cachedDepen */ private function fileStatSignaturesDiffer(array $cachedDependencies): bool { - foreach ($this->fileStatSignatures as $file => $signature) { + foreach ($this->recordedFileStats as $file => $signature) { if (!array_key_exists($file, $cachedDependencies)) { continue; } diff --git a/src/File/DirectoryWalker.php b/src/File/DirectoryWalker.php index 6e6cfd0566b..6808646295b 100644 --- a/src/File/DirectoryWalker.php +++ b/src/File/DirectoryWalker.php @@ -29,7 +29,6 @@ use function str_contains; use function str_starts_with; use function substr; -use function time; use function unlink; use function unserialize; use const DIRECTORY_SEPARATOR; @@ -49,11 +48,8 @@ * Reading the directories is what a walk costs - on a tree the size of Drupal core (28k directories) * about a second, every run, in the main process and again in the worker that builds the symbol * index. What a directory contains changes only when an entry is added, removed or renamed in it, - * and each of those updates the directory's mtime and ctime, so the listings are kept in tmpDir and - * a directory whose stat still matches is not read again. ctime cannot be set back by touch or by - * extracting an archive, and a directory modified in the very second its listing is read is not - * kept: a second change within that same second would leave its stat unchanged. The same technique - * is behind git's untracked cache. + * so the listings are kept in tmpDir and a directory whose stat signature still matches (see + * FileStatSignatures) is not read again. The same technique is behind git's untracked cache. * * The walk yields what Symfony Finder yields for files()->name()->followLinks() with its default * ignores - dot files and VCS directories left out, symlinks followed, files in readdir order - and @@ -64,7 +60,7 @@ final class DirectoryWalker { - private const LISTINGS_FORMAT = 'directoryListings-v1'; + private const LISTINGS_FORMAT = 'directoryListings-v2'; /** The directories Finder's ignoreVCS() leaves out; the ones starting with a dot are left out anyway. */ private const VCS_DIRECTORIES = ['_svn' => true, 'CVS' => true, '_darcs' => true]; @@ -73,12 +69,12 @@ final class DirectoryWalker private array $cachedWalks = []; /** - * Directory path => [mtime, ctime, inode, entries]. The entries are the directory's names in + * Directory path => [stat signature, entries]. The entries are the directory's names in * readdir order, each prefixed by d (directory), f (anything else - Finder's files() only leaves * out directories, so a broken symlink is a file too) or l (symlink - resolved on every walk, * because its target can change without this directory changing), joined by NUL. * - * @var array|null + * @var array|null */ private ?array $listings = null; @@ -88,6 +84,7 @@ final class DirectoryWalker * @param string $tmpDir where the listings are kept between runs, nowhere when empty */ public function __construct( + private FileStatSignatures $fileStatSignatures, #[AutowiredParameter] private string $tmpDir = '', ) @@ -155,7 +152,7 @@ private function walkWithListings(string $directory, array $fileExtensions): ?ar $files = []; $visited = []; - if (!$this->walkDirectory($directory, Glob::toRegex('*.{' . implode(',', $fileExtensions) . '}'), time(), $files, $visited)) { + if (!$this->walkDirectory($directory, Glob::toRegex('*.{' . implode(',', $fileExtensions) . '}'), $this->fileStatSignatures->begin(), $files, $visited)) { return null; } @@ -168,7 +165,7 @@ private function walkWithListings(string $directory, array $fileExtensions): ?ar * @param list $files * @param array $visited */ - private function walkDirectory(string $directory, string $pattern, int $now, array &$files, array &$visited): bool + private function walkDirectory(string $directory, string $pattern, FileStatSignatureReader $signatures, array &$files, array &$visited): bool { $stat = @stat($directory); if ($stat === false) { @@ -176,17 +173,18 @@ private function walkDirectory(string $directory, string $pattern, int $now, arr } $visited[$directory] = true; + $signature = $signatures->fromStat($stat); $listing = $this->listings[$directory] ?? null; - if ($listing !== null && $listing[0] === $stat['mtime'] && $listing[1] === $stat['ctime'] && $listing[2] === $stat['ino']) { - $entries = $listing[3]; + if ($signature !== null && $listing !== null && $listing[0] === $signature) { + $entries = $listing[1]; } else { $entries = $this->readDirectory($directory); if ($entries === null) { return false; } - if ($stat['mtime'] < $now && $stat['ctime'] < $now) { - $this->listings[$directory] = [$stat['mtime'], $stat['ctime'], $stat['ino'], $entries]; + if ($signature !== null) { + $this->listings[$directory] = [$signature, $entries]; } else { unset($this->listings[$directory]); } @@ -213,7 +211,7 @@ private function walkDirectory(string $directory, string $pattern, int $now, arr } if ($type === 'd') { - if (!$this->walkDirectory($path, $pattern, $now, $files, $visited)) { + if (!$this->walkDirectory($path, $pattern, $signatures, $files, $visited)) { return false; } @@ -297,7 +295,7 @@ private function loadListings(): void return; } - /** @var array $listings */ + /** @var array $listings */ $listings = $data['listings']; $this->listings = $listings; } diff --git a/src/File/FileStatSignatureReader.php b/src/File/FileStatSignatureReader.php new file mode 100644 index 00000000000..0a9a0ed1c3d --- /dev/null +++ b/src/File/FileStatSignatureReader.php @@ -0,0 +1,55 @@ +trusted || str_contains($path, '://')) { + return null; + } + + $stat = @stat($path); + if ($stat === false) { + return null; + } + + return $this->fromStat($stat); + } + + /** + * @param array $stat what stat() returned for the path + */ + public function fromStat(array $stat): ?string + { + if (!$this->trusted || $stat['mtime'] >= $this->startedAt || $stat['ctime'] >= $this->startedAt) { + return null; + } + + return sprintf('%d:%d:%d:%d:%d', $stat['size'], $stat['mtime'], $stat['ctime'], $stat['ino'], $stat['dev']); + } + +} diff --git a/src/File/FileStatSignatures.php b/src/File/FileStatSignatures.php new file mode 100644 index 00000000000..20f677f2953 --- /dev/null +++ b/src/File/FileStatSignatures.php @@ -0,0 +1,33 @@ +walk($this->directory, ['php']); file_put_contents($this->directory . '/second.php', 'walkCached($this->directory, ['php']); file_put_contents($this->directory . '/second.php', 'directory . '/notes.txt', 'x'); $phpFiles = $walker->walkCached($this->directory, ['php']); @@ -80,11 +80,11 @@ public function testWalkYieldsWhatFinderYields(): void $tmpDir = $this->createTree(); $expected = $this->walkWithFinder($this->directory, ['php', 'sh', '']); - $fresh = (new DirectoryWalker($tmpDir))->walk($this->directory, ['php', 'sh', '']); + $fresh = (new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php', 'sh', '']); $this->waitUntilTheTreeIsInThePast(); // stores the listings, now that they are no longer racy - (new DirectoryWalker($tmpDir))->walk($this->directory, ['php', 'sh', '']); - $fromListings = (new DirectoryWalker($tmpDir))->walk($this->directory, ['php', 'sh', '']); + (new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php', 'sh', '']); + $fromListings = (new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php', 'sh', '']); $this->assertNotSame([], $expected); $this->assertSame($expected, $fresh); @@ -96,7 +96,7 @@ public function testListingsSeeAddedAndRemovedFiles(): void { $tmpDir = $this->createTree(); $this->waitUntilTheTreeIsInThePast(); - (new DirectoryWalker($tmpDir))->walk($this->directory, ['php']); + (new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php']); file_put_contents($this->directory . '/src/Added.php', 'directory . '/src/Nested/Deep.php'); @@ -104,7 +104,7 @@ public function testListingsSeeAddedAndRemovedFiles(): void $this->assertSame( $this->walkWithFinder($this->directory, ['php']), - (new DirectoryWalker($tmpDir))->walk($this->directory, ['php']), + (new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php']), ); } @@ -112,7 +112,7 @@ public function testReplacedDirectoryIsReadAgain(): void { $tmpDir = $this->createTree(); $this->waitUntilTheTreeIsInThePast(); - (new DirectoryWalker($tmpDir))->walk($this->directory, ['php']); + (new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php']); unlink($this->directory . '/src/Nested/Deep.php'); rmdir($this->directory . '/src/Nested'); @@ -121,7 +121,7 @@ public function testReplacedDirectoryIsReadAgain(): void $this->assertContains( $this->directory . '/src/Nested/Other.php', - (new DirectoryWalker($tmpDir))->walk($this->directory, ['php']), + (new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php']), ); } diff --git a/tests/PHPStan/File/FileStatSignatureReaderTest.php b/tests/PHPStan/File/FileStatSignatureReaderTest.php new file mode 100644 index 00000000000..747ec17a3da --- /dev/null +++ b/tests/PHPStan/File/FileStatSignatureReaderTest.php @@ -0,0 +1,81 @@ +file = sys_get_temp_dir() . '/' . uniqid('phpstan-stat-', true) . '.php'; + file_put_contents($this->file, 'file); + } + + public function testSignatureStaysWhileTheFileIsUnchanged(): void + { + $reader = new FileStatSignatureReader($this->getLastChange() + 1, true); + + $signature = $reader->get($this->file); + + $this->assertNotNull($signature); + $this->assertSame($signature, $reader->get($this->file)); + } + + public function testSignatureChangesWithTheFile(): void + { + $before = (new FileStatSignatureReader($this->getLastChange() + 1, true))->get($this->file); + file_put_contents($this->file, 'getLastChange() + 1, true))->get($this->file); + + $this->assertNotNull($before); + $this->assertNotNull($after); + $this->assertNotSame($before, $after); + } + + public function testNoSignatureForFileModifiedInTheSecondTheReadingBegan(): void + { + $this->assertNull((new FileStatSignatureReader($this->getLastChange(), true))->get($this->file)); + } + + public function testNoSignatureWhenNotTrusted(): void + { + $this->assertNull((new FileStatSignatureReader($this->getLastChange() + 1, false))->get($this->file)); + } + + public function testNoSignatureForMissingFileOrStreamWrapper(): void + { + $reader = new FileStatSignatureReader($this->getLastChange() + 1, true); + + $this->assertNull($reader->get($this->file . '.missing')); + $this->assertNull($reader->get('file://' . $this->file)); + } + + private function getLastChange(): int + { + $stat = stat($this->file); + $this->assertIsArray($stat); + + return max($stat['mtime'], $stat['ctime']); + } + +} From abdcbedc78bee9b9cf14c9192c78f9a3e7533ff3 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 30 Sep 2026 13:31:33 +0200 Subject: [PATCH 08/10] Keep the symbols found in directories between runs, checked by stat OptimizedDirectorySourceLocatorFactory had two ways of finding the symbols of a directory. Without the turbo extension, or with workers that are not forked, they were cached, and the cache was checked by hashing every file. With the extension and forked workers, every file was scanned natively on every run instead, because hashing cost about as much as the scan - 0.7s on Drupal core, paid by every run that analyses at least one file. There is one way now. The cache entry of a directory keeps each file's stat signature next to its hash (see FileStatSignatures), and a file whose signature matches is not even hashed. When the signature cannot vouch for the file, the hash decides, as before. The arena records that shared the file hashes and the symbol maps between workers that are not forked are gone - the cache entry itself is still shared through the arena by Cache::load(). The cache now also keeps the files that declare no symbols, which used to be scanned again on every run. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ --- .../OptimizedDirectorySourceLocator.php | 121 +----- ...OptimizedDirectorySourceLocatorFactory.php | 401 ++++++------------ .../PreForkDirectorySymbolScanner.php | 22 +- ...mizedDirectorySourceLocatorFactoryTest.php | 140 ++++++ 4 files changed, 268 insertions(+), 416 deletions(-) create mode 100644 tests/PHPStan/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactoryTest.php diff --git a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php index 3dda469450c..de959b6f38c 100644 --- a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php @@ -14,7 +14,6 @@ use PHPStan\BetterReflection\Reflector\Reflector; use PHPStan\BetterReflection\SourceLocator\Ast\Strategy\NodeToReflection; use PHPStan\BetterReflection\SourceLocator\Type\SourceLocator; -use PHPStan\Cache\ArenaCache; use PHPStan\Cache\Cache; use PHPStan\File\CouldNotReadFileException; use PHPStan\File\FileContentHasher; @@ -25,31 +24,13 @@ use function array_key_exists; use function array_values; use function current; -use function is_array; -use function is_string; use function sprintf; use function strtolower; final class OptimizedDirectorySourceLocator implements SourceLocator { - /** @var array */ - private array $arenaClassLookups = []; - - /** @var array|false> */ - private array $arenaFunctionLookups = []; - - /** @var array */ - private array $arenaConstantLookups = []; - - private bool $hydratedFromArena = false; - /** - * With $arenaKeyPrefix set, the maps start empty and names resolve lazily - * from the run's shared arena (published by whichever process built this - * directory's index first), so the worker materializes only the names it - * touches instead of the whole index. - * * @param array $classToFile * @param array> $functionToFiles * @param array $constantToFile @@ -62,14 +43,13 @@ public function __construct( private array $classToFile, private array $functionToFiles, private array $constantToFile, - private ?string $arenaKeyPrefix = null, private bool $awaitingBatchedScan = false, ) { } /** - * Fills in the symbol maps of a locator created for a batched scan. + * Fills in the symbol maps of a locator created ahead of the scan. * * The factory hands these out before the scan that produces their contents * has run, so that one scan can cover every directory at once @@ -248,48 +228,12 @@ private function nodeToReflection(Reflector $reflector, FetchedNode $fetchedNode private function findFileByClass(string $className): ?string { - if (array_key_exists($className, $this->classToFile)) { - return $this->classToFile[$className]; - } - - if ($this->arenaKeyPrefix === null) { - return null; - } - - if (array_key_exists($className, $this->arenaClassLookups)) { - $file = $this->arenaClassLookups[$className]; - } else { - $file = ArenaCache::lookupHash($this->arenaKeyPrefix . '-classes', $className); - if (!is_string($file)) { - $file = false; - } - $this->arenaClassLookups[$className] = $file; - } - - return $file === false ? null : $file; + return $this->classToFile[$className] ?? null; } private function findFileByConstant(string $constantName): ?string { - if (array_key_exists($constantName, $this->constantToFile)) { - return $this->constantToFile[$constantName]; - } - - if ($this->arenaKeyPrefix === null) { - return null; - } - - if (array_key_exists($constantName, $this->arenaConstantLookups)) { - $file = $this->arenaConstantLookups[$constantName]; - } else { - $file = ArenaCache::lookupHash($this->arenaKeyPrefix . '-constants', $constantName); - if (!is_string($file)) { - $file = false; - } - $this->arenaConstantLookups[$constantName] = $file; - } - - return $file === false ? null : $file; + return $this->constantToFile[$constantName] ?? null; } /** @@ -297,62 +241,7 @@ private function findFileByConstant(string $constantName): ?string */ private function findFilesByFunction(string $functionName): array { - if (array_key_exists($functionName, $this->functionToFiles)) { - return $this->functionToFiles[$functionName]; - } - - if ($this->arenaKeyPrefix === null) { - return []; - } - - if (array_key_exists($functionName, $this->arenaFunctionLookups)) { - $files = $this->arenaFunctionLookups[$functionName]; - } else { - /** @var array|mixed $files */ - $files = ArenaCache::lookupHash($this->arenaKeyPrefix . '-functions', $functionName); - if (!is_array($files)) { - $files = false; - } - $this->arenaFunctionLookups[$functionName] = $files; - } - - return $files === false ? [] : $files; - } - - /** - * Enumeration needs the full maps: hydrates them from the arena records - * in their publication order, which equals the insertion order a locally - * built index would have. A null (a corrupt record — impossible with an - * intact arena, the factory gated on all three records) leaves a map - * empty rather than failing the run. - */ - private function hydrateSymbolsFromArena(): void - { - if ($this->arenaKeyPrefix === null || $this->hydratedFromArena) { - return; - } - - $this->hydratedFromArena = true; - - /** @var array|null $classes */ - $classes = ArenaCache::lookupHashAll($this->arenaKeyPrefix . '-classes'); - if ($classes !== null) { - $this->classToFile = $classes; - } - - /** @var array>|null $functions */ - $functions = ArenaCache::lookupHashAll($this->arenaKeyPrefix . '-functions'); - if ($functions !== null) { - $this->functionToFiles = $functions; - } - - /** @var array|null $constants */ - $constants = ArenaCache::lookupHashAll($this->arenaKeyPrefix . '-constants'); - if ($constants === null) { - return; - } - - $this->constantToFile = $constants; + return $this->functionToFiles[$functionName] ?? []; } /** @@ -365,8 +254,6 @@ public function locateIdentifiersByType(Reflector $reflector, IdentifierType $id throw new ShouldNotHappenException('Symbols were looked up in a directory whose batched scan has not been flushed yet.'); } - $this->hydrateSymbolsFromArena(); - $reflections = []; if ($identifierType->isClass()) { foreach ($this->classToFile as $file) { diff --git a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactory.php b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactory.php index a4287665de8..f856e844dec 100644 --- a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactory.php +++ b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactory.php @@ -2,35 +2,37 @@ namespace PHPStan\Reflection\BetterReflection\SourceLocator; -use PHPStan\Cache\ArenaCache; use PHPStan\Cache\Cache; use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\File\FileContentHasher; use PHPStan\File\FileFinder; +use PHPStan\File\FileStatSignatures; use PHPStan\Internal\DirectoryCreator; use PHPStan\Internal\DirectoryCreatorException; -use PHPStan\Parallel\ForkParallelChecker; use PHPStan\Php\PhpVersion; -use PHPStan\Turbo\TurboExtensionEnabler; use function array_key_exists; use function array_keys; -use function array_values; use function fclose; use function flock; use function fopen; use function hrtime; -use function is_array; -use function ksort; -use function serialize; use function sha1; use function sprintf; use function usleep; use const LOCK_EX; use const LOCK_NB; use const LOCK_UN; -use const SORT_STRING; +/** + * Builds the symbol maps of the optimized directory source locators: which file declares which + * class, function and constant. + * + * Finding the symbols means reading every file, so what was found is cached per directory, and a + * file is only scanned again once it changed. A file whose stat signature is what it was when it was + * scanned has not changed (see FileStatSignatures) - checking that costs a fraction of reading the + * file. When the signature cannot vouch for the file, its content hash decides. + */ #[AutowiredService] final class OptimizedDirectorySourceLocatorFactory { @@ -46,19 +48,20 @@ final class OptimizedDirectorySourceLocatorFactory private const SCAN_LOCK_POLL_INTERVAL_MICROSECONDS = 50_000; /** - * The hash lock is polled much finer than the scan lock: every worker - * reaches the same directories in near lockstep at startup, and most - * directories hash in well under the scan lock's 50ms tick, so a coarse - * poll would make lock losers sleep longer than the work they skip. + * Directories collected for a batched scan, null when not batching. + * + * @var list|null */ - private const HASH_LOCK_POLL_INTERVAL_MICROSECONDS = 5_000; + private ?array $batchedScan = null; /** - * Directories collected for a batched scan, null when not batching. + * The files this process already checked or scanned, as the caches keep them: [content hash, + * stat signature, classes, functions, constants]. The directories a process builds locators for + * can overlap, and a file reachable from two of them is then looked at once. * - * @var list|null + * @var array */ - private ?array $batchedScan = null; + private array $checkedFiles = []; public function __construct( private FileNodesFetcher $fileNodesFetcher, @@ -68,7 +71,7 @@ public function __construct( private SymbolFinderInFiles $symbolFinderInFiles, private Cache $cache, private FileContentHasher $fileContentHasher, - private ForkParallelChecker $forkParallelChecker, + private FileStatSignatures $fileStatSignatures, #[AutowiredParameter] private string $tmpDir, ) @@ -77,98 +80,16 @@ public function __construct( public function createByDirectory(string $directory): OptimizedDirectorySourceLocator { - if ($this->scansFresh()) { - return $this->createFreshDirectorySourceLocator($directory); - } - - $cacheKey = sprintf('odsl-%s', $directory); - $hashesRecordKey = 'odsl-filehashes-' . $directory; - - // The walk + hash of a directory is identical in every process of a - // run, and running it once per worker in parallel multiplies both the - // CPU and — on hosts where concurrent open() is expensive — the wall - // cost of the analysis startup. When the run has a shared arena, the - // first process publishes the file-hash map and everyone else reuses - // it. hasRecord() on the analysed-files record (published by the - // master before workers spawn) doubles as the "is an arena active?" - // probe — the seam has no explicit method for that and only grows one - // together with the extension. - $arenaActive = ArenaCache::hasRecord('analysed-files'); - $hashesLock = null; - if ($arenaActive) { - $shared = ArenaCache::lookup($hashesRecordKey); - if (is_array($shared)) { - /** @var array $shared */ - return $this->createCachedDirectorySourceLocator($shared, $cacheKey); - } - - // Single-flight the walk + hash, same pattern as the cold-cache - // scan below: the winner computes and publishes, losers wait and - // re-read the record. A lost lock (timeout, unwritable tmp) just - // means this worker hashes the directory itself. - $hashesLock = $this->acquireDirectoryScanLock('hashes-' . $directory, self::HASH_LOCK_POLL_INTERVAL_MICROSECONDS); - if ($hashesLock !== null) { - $shared = ArenaCache::lookup($hashesRecordKey); - if (is_array($shared)) { - $this->releaseDirectoryScanLock($hashesLock); - - /** @var array $shared */ - return $this->createCachedDirectorySourceLocator($shared, $cacheKey); - } - } - } - - try { - $files = $this->fileFinder->findFiles([$directory])->getFiles(); - $fileHashes = []; - foreach ($files as $file) { - $hash = $this->fileContentHasher->hash($file); - if ($hash === false) { - continue; - } - $fileHashes[$file] = $hash; - } - - if ($arenaActive) { - ArenaCache::publish($hashesRecordKey, $fileHashes); - } - } finally { - if ($hashesLock !== null) { - $this->releaseDirectoryScanLock($hashesLock); - } - } - - return $this->createCachedDirectorySourceLocator($fileHashes, $cacheKey); + return $this->createLocator(sprintf('odsl-%s', $directory), $this->fileFinder->findFiles([$directory])->getFiles()); } /** - * Whether the symbol index is built outright instead of being cached. - * - * Both halves are needed. The native scan is what makes the cache not - * worth its keep, and forking is what keeps the scan from happening once - * per worker: the parent scans before it forks and the children inherit - * the result (see PreForkDirectorySymbolScanner). Where the extension is - * active but workers are spawned rather than forked - Windows, or OPcache - * left on - there is nothing to inherit, so the cache and its scan lock - * stay in charge. - */ - private function scansFresh(): bool - { - return TurboExtensionEnabler::isActive() && $this->forkParallelChecker->isSupported(); - } - - /** - * With the turbo extension the symbol scan is native and costs about what - * hashing the directory to validate a cache costs, so a cache has nothing - * left to save: the directory is walked and scanned outright, with no file - * hashing, no persisted symbol table, no scan lock and no arena record — - * and therefore no cache that can go stale. PreForkDirectoryScanner runs - * this once in the main process before it forks its workers, so every - * worker inherits the finished locators instead of racing to build them. + * @param string[] $files + * @param non-empty-string&literal-string $uniqueCacheIdentifier */ - private function createFreshDirectorySourceLocator(string $directory): OptimizedDirectorySourceLocator + public function createByFiles(array $files, string $uniqueCacheIdentifier): OptimizedDirectorySourceLocator { - return $this->createFreshFileListSourceLocator($this->fileFinder->findFiles([$directory])->getFiles()); + return $this->createLocator($uniqueCacheIdentifier, $files); } /** @@ -194,195 +115,133 @@ public function flushBatchedScan(): void return; } - $allFiles = []; - foreach ($batched as [$files]) { - foreach ($files as $file) { - // a file reachable from two directories is scanned once - $allFiles[$file] = $file; - } - } - - $symbols = $this->symbolFinderInFiles->findSymbols(array_values($allFiles), $this->phpVersion->supportsEnums()); - - foreach ($batched as [$files, $locator]) { - $directorySymbols = []; - foreach ($files as $file) { - if (!array_key_exists($file, $symbols)) { - continue; - } - - $directorySymbols[$file] = $symbols[$file]; - } - - [$classToFile, $functionToFiles, $constantToFile] = $this->changeStructure($directorySymbols); - $locator->fillBatchedScan($classToFile, $functionToFiles, $constantToFile); - } + $this->scan($batched); } /** + * @param non-empty-string $cacheKey * @param string[] $files */ - private function createFreshFileListSourceLocator(array $files): OptimizedDirectorySourceLocator + private function createLocator(string $cacheKey, array $files): OptimizedDirectorySourceLocator { - if ($this->batchedScan !== null) { - $locator = new OptimizedDirectorySourceLocator( - $this->fileNodesFetcher, - $this->cache, - $this->phpVersion, - $this->fileContentHasher, - [], - [], - [], - awaitingBatchedScan: true, - ); - $this->batchedScan[] = [$files, $locator]; - - return $locator; - } - - $symbols = $this->symbolFinderInFiles->findSymbols($files, $this->phpVersion->supportsEnums()); - [$classToFile, $functionToFiles, $constantToFile] = $this->changeStructure($symbols); - - return new OptimizedDirectorySourceLocator( + $locator = new OptimizedDirectorySourceLocator( $this->fileNodesFetcher, $this->cache, $this->phpVersion, $this->fileContentHasher, - $classToFile, - $functionToFiles, - $constantToFile, + [], + [], + [], + awaitingBatchedScan: true, ); + + if ($this->batchedScan !== null) { + $this->batchedScan[] = [$cacheKey, $files, $locator]; + } else { + $this->scan([[$cacheKey, $files, $locator]]); + } + + return $locator; } /** - * @param array $fileHashes - * @param non-empty-string $cacheKey + * Fills in the symbol maps of the locators, each for its files, from their caches and by + * scanning the files that changed since. + * + * @param list $requests */ - private function createCachedDirectorySourceLocator(array $fileHashes, string $cacheKey): OptimizedDirectorySourceLocator + private function scan(array $requests): void { - $variableCacheKey = sprintf('v1-%s', $this->phpVersion->supportsEnums() ? 'enums' : 'no-enums'); - - // The run's shared arena binds the symbol index to the exact content - // fingerprint of the directory: a worker seeing the same file hashes - // reuses the index another process already validated and published — - // no include() of the cache blob, no validation pass, and names are - // materialized lazily one by one. A worker whose view differs (a file - // changed mid-run) misses the fingerprint and builds locally. - $sortedFileHashes = $fileHashes; - ksort($sortedFileHashes, SORT_STRING); - $arenaKeyPrefix = sprintf('odsl-arena-%s', sha1($cacheKey . "\0" . $variableCacheKey . "\0" . serialize($sortedFileHashes))); - if ( - ArenaCache::hasRecord($arenaKeyPrefix . '-classes') - && ArenaCache::hasRecord($arenaKeyPrefix . '-functions') - && ArenaCache::hasRecord($arenaKeyPrefix . '-constants') - ) { - return new OptimizedDirectorySourceLocator( - $this->fileNodesFetcher, - $this->cache, - $this->phpVersion, - $this->fileContentHasher, - [], - [], - [], - $arenaKeyPrefix, - ); - } + $variableCacheKey = sprintf('v2-%s', $this->phpVersion->supportsEnums() ? 'enums' : 'no-enums'); + $signatures = $this->fileStatSignatures->begin(); + $scanLocks = []; - $originalFileHashes = $fileHashes; - - $cached = $this->loadCachedSymbols($cacheKey, $variableCacheKey); - - $scanLock = null; - if ($cached === null) { - // On a cold cache every parallel worker builds the same directory locator at once and would - // scan the same directory redundantly. A scan is not published until it finishes and the save - // is atomic, so these races are wasteful rather than unsafe. The first worker to take the lock - // scans and saves; the rest block until it releases, then re-read the cache it wrote. When the - // re-read hits, the lock has done its job, so release it right away and continue lock-free - - // the validation and any (re)scan below then run exactly as they did before this change. - $scanLock = $this->acquireDirectoryScanLock($cacheKey . $variableCacheKey); - if ($scanLock !== null) { + try { + $cachedEntries = []; + foreach ($requests as $i => [$cacheKey]) { $cached = $this->loadCachedSymbols($cacheKey, $variableCacheKey); - if ($cached !== null) { - $this->releaseDirectoryScanLock($scanLock); - $scanLock = null; + if ($cached === null) { + // On a cold cache every parallel worker that is not forked from a process that did + // the scan already builds the same locators at once, and would scan the same files. + // The first worker to take the lock scans and saves; the rest block until it + // releases, then read the cache it wrote. + $scanLock = $this->acquireDirectoryScanLock($cacheKey . $variableCacheKey); + if ($scanLock !== null) { + $cached = $this->loadCachedSymbols($cacheKey, $variableCacheKey); + if ($cached !== null) { + $this->releaseDirectoryScanLock($scanLock); + } else { + $scanLocks[] = $scanLock; + } + } } + + $cachedEntries[$i] = $cached; } - } - try { - $cacheModified = false; - $findInFiles = []; - if ($cached !== null) { - foreach ($cached as $file => [$hash]) { - if (!array_key_exists($file, $fileHashes)) { - unset($cached[$file]); - $cacheModified = true; + $filesToScan = []; + foreach ($requests as $i => [, $files]) { + $cached = $cachedEntries[$i] ?? []; + foreach ($files as $file) { + if (array_key_exists($file, $this->checkedFiles) || array_key_exists($file, $filesToScan)) { + continue; + } + + $signature = $signatures->get($file); + $cachedFile = $cached[$file] ?? null; + if ($cachedFile !== null && $signature !== null && $cachedFile[1] === $signature) { + $this->checkedFiles[$file] = $cachedFile; continue; } - $newHash = $fileHashes[$file]; - unset($fileHashes[$file]); - if ($hash === $newHash) { + + $hash = $this->fileContentHasher->hash($file); + if ($hash === false) { continue; } - $findInFiles[] = $file; + if ($cachedFile !== null && $cachedFile[0] === $hash) { + $this->checkedFiles[$file] = [$hash, $signature, $cachedFile[2], $cachedFile[3], $cachedFile[4]]; + continue; + } + + $filesToScan[$file] = [$hash, $signature]; } - } else { - // Cold miss: publish the result (even an empty one) so lock losers read it back instead - // of finding the cache still cold and re-scanning the directory themselves. - $cached = []; - $cacheModified = true; } - foreach (array_keys($fileHashes) as $file) { - $findInFiles[] = $file; + if ($filesToScan !== []) { + $foundSymbols = $this->symbolFinderInFiles->findSymbols(array_keys($filesToScan), $this->phpVersion->supportsEnums()); + foreach ($filesToScan as $file => [$hash, $signature]) { + [$classes, $functions, $constants] = $foundSymbols[$file] ?? [[], [], []]; + $this->checkedFiles[$file] = [$hash, $signature, $classes, $functions, $constants]; + } } - if ($findInFiles !== []) { - $cacheModified = true; - foreach ($this->symbolFinderInFiles->findSymbols($findInFiles, $this->phpVersion->supportsEnums()) as $file => [$newClasses, $newFunctions, $newConstants]) { - $newHash = $originalFileHashes[$file]; - $cached[$file] = [$newHash, $newClasses, $newFunctions, $newConstants]; + foreach ($requests as $i => [$cacheKey, $files, $locator]) { + $entry = []; + foreach ($files as $file) { + if (!array_key_exists($file, $this->checkedFiles)) { + continue; + } + + $entry[$file] = $this->checkedFiles[$file]; } - } - // Only write when the cache actually changed. A lock loser re-reads exactly what the winner - // wrote, and a warm run finds every hash unchanged, so both would otherwise re-serialize and - // re-write the identical symbol table - the loser while still holding the lock. - if ($cacheModified) { - $this->cache->save($cacheKey, $variableCacheKey, $cached); + // A warm run finds every file unchanged and does not write anything. A cold miss is + // written even when empty, so that the workers waiting for the lock read it back. + if ($entry !== $cachedEntries[$i]) { + $this->cache->save($cacheKey, $variableCacheKey, $entry); + } + + [$classToFile, $functionToFiles, $constantToFile] = $this->changeStructure($entry); + $locator->fillBatchedScan($classToFile, $functionToFiles, $constantToFile); } } finally { // Release even if scanning or saving throws, so a failing worker cannot leave other // workers blocked on the lock until it exits. - if ($scanLock !== null) { + foreach ($scanLocks as $scanLock) { $this->releaseDirectoryScanLock($scanLock); } } - - $symbols = []; - foreach ($cached as $file => [, $classes, $functions, $constants]) { - $symbols[$file] = [$classes, $functions, $constants]; - } - - [$classToFile, $functionToFiles, $constantToFile] = $this->changeStructure($symbols); - - // Publication order matters: the reader above requires all three - // records, so a partially-published index is never consumed. - ArenaCache::publishHash($arenaKeyPrefix . '-classes', $classToFile); - ArenaCache::publishHash($arenaKeyPrefix . '-functions', $functionToFiles); - ArenaCache::publishHash($arenaKeyPrefix . '-constants', $constantToFile); - - return new OptimizedDirectorySourceLocator( - $this->fileNodesFetcher, - $this->cache, - $this->phpVersion, - $this->fileContentHasher, - $classToFile, - $functionToFiles, - $constantToFile, - ); } /** @@ -393,7 +252,7 @@ private function createCachedDirectorySourceLocator(array $fileHashes, string $c * * @return resource|null */ - private function acquireDirectoryScanLock(string $lockKey, int $pollIntervalMicroseconds = self::SCAN_LOCK_POLL_INTERVAL_MICROSECONDS) + private function acquireDirectoryScanLock(string $lockKey) { $lockDirectory = sprintf('%s/cache/locks', $this->tmpDir); try { @@ -422,7 +281,7 @@ private function acquireDirectoryScanLock(string $lockKey, int $pollIntervalMicr return null; } - usleep($pollIntervalMicroseconds); + usleep(self::SCAN_LOCK_POLL_INTERVAL_MICROSECONDS); } return $lockHandle; @@ -430,11 +289,11 @@ private function acquireDirectoryScanLock(string $lockKey, int $pollIntervalMicr /** * @param non-empty-string $cacheKey - * @return array|null + * @return array|null */ private function loadCachedSymbols(string $cacheKey, string $variableCacheKey): ?array { - /** @var array|null $cached */ + /** @var array|null $cached */ $cached = $this->cache->load($cacheKey, $variableCacheKey); return $cached; @@ -450,37 +309,15 @@ private function releaseDirectoryScanLock($lockHandle): void } /** - * @param string[] $files - * @param non-empty-string&literal-string $uniqueCacheIdentifier - */ - public function createByFiles(array $files, string $uniqueCacheIdentifier): OptimizedDirectorySourceLocator - { - if ($this->scansFresh()) { - return $this->createFreshFileListSourceLocator($files); - } - - $fileHashes = []; - foreach ($files as $file) { - $hash = $this->fileContentHasher->hash($file); - if ($hash === false) { - continue; - } - $fileHashes[$file] = $hash; - } - - return $this->createCachedDirectorySourceLocator($fileHashes, $uniqueCacheIdentifier); - } - - /** - * @param array $symbols + * @param array $entry * @return array{array, array>, array} */ - private function changeStructure(array $symbols): array + private function changeStructure(array $entry): array { $classToFile = []; $constantToFile = []; $functionToFiles = []; - foreach ($symbols as $file => [$classes, $functions, $constants]) { + foreach ($entry as $file => [, , $classes, $functions, $constants]) { foreach ($classes as $classInFile) { $classToFile[$classInFile] = $file; } diff --git a/src/Reflection/BetterReflection/SourceLocator/PreForkDirectorySymbolScanner.php b/src/Reflection/BetterReflection/SourceLocator/PreForkDirectorySymbolScanner.php index 8d2a3629ec4..e99d873d450 100644 --- a/src/Reflection/BetterReflection/SourceLocator/PreForkDirectorySymbolScanner.php +++ b/src/Reflection/BetterReflection/SourceLocator/PreForkDirectorySymbolScanner.php @@ -4,22 +4,16 @@ use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\AutowiredService; -use PHPStan\Turbo\TurboExtensionEnabler; use function array_merge; use function array_unique; use function is_dir; /** - * Scans the Composer classmap directories once in the main process, just - * before it forks its workers, so that every worker inherits the finished - * symbol indexes instead of building its own. - * - * With the turbo extension the scan is native and no longer worth caching - * (see OptimizedDirectorySourceLocatorFactory), which removes the disk cache, - * the scan lock and the arena records that used to keep parallel workers from - * duplicating the work. Forking replaces all three: the memoized locators in - * OptimizedDirectorySourceLocatorRepository are copy-on-write shared with - * every child. + * Builds the directory locators once in the main process, just before it + * forks its workers, so that every worker inherits the finished symbol maps + * instead of checking the cache and scanning the changed files on its own: + * the memoized locators in OptimizedDirectorySourceLocatorRepository are + * copy-on-write shared with every child. * * This adds no work that was not already being done. Both sets of directory * locators - the analysed and scanned directories, and the Composer classmap @@ -66,12 +60,6 @@ public function __construct( public function scanBeforeFork(): void { - if (!TurboExtensionEnabler::isActive()) { - // without the extension the cache and the scan lock are still in - // place and already keep the workers from duplicating the scan - return; - } - $directories = []; foreach (array_merge($this->analysedPaths, $this->analysedPathsFromConfig) as $analysedPath) { if (!is_dir($analysedPath)) { diff --git a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactoryTest.php b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactoryTest.php new file mode 100644 index 00000000000..34ba8a9f0f5 --- /dev/null +++ b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactoryTest.php @@ -0,0 +1,140 @@ +directory = sys_get_temp_dir() . '/' . uniqid('phpstan-odsl-', true); + mkdir($this->directory); + $this->cache = new Cache(new MemoryCacheStorage()); + } + + public function testChangedFileIsScannedAgain(): void + { + file_put_contents($this->directory . '/a.php', 'assertTrue($this->hasClass('OdslFactoryTest\\First')); + + // the same second, so the signature cannot tell - the content hash does + file_put_contents($this->directory . '/a.php', 'assertFalse($this->hasClass('OdslFactoryTest\\First')); + $this->assertTrue($this->hasClass('OdslFactoryTest\\Second')); + } + + public function testUnchangedFileIsTrustedByItsSignature(): void + { + if (DIRECTORY_SEPARATOR !== '/') { + $this->markTestSkipped('Signatures are not trusted on Windows.'); + } + + file_put_contents($this->directory . '/a.php', 'waitUntilInThePast($this->directory . '/a.php'); + $this->assertTrue($this->hasClass('OdslFactoryTest\\First')); + + $entry = $this->cache->load(self::CACHE_KEY, $this->getVariableCacheKey()); + $this->assertIsArray($entry); + $this->assertNotNull($entry[$this->directory . '/a.php'][1]); + + // a hash that cannot match: the entry is only reused because the signature does + $entry[$this->directory . '/a.php'][0] = 'not-the-hash'; + $this->cache->save(self::CACHE_KEY, $this->getVariableCacheKey(), $entry); + $this->assertTrue($this->hasClass('OdslFactoryTest\\First')); + $this->assertSame($entry, $this->cache->load(self::CACHE_KEY, $this->getVariableCacheKey())); + + file_put_contents($this->directory . '/a.php', 'assertFalse($this->hasClass('OdslFactoryTest\\First')); + $this->assertTrue($this->hasClass('OdslFactoryTest\\SecondWithLongerName')); + } + + public function testBatchedScanFillsEveryLocator(): void + { + mkdir($this->directory . '/sub'); + file_put_contents($this->directory . '/a.php', 'directory . '/sub/b.php', 'createFactory(); + $factory->beginBatchedScan(); + $all = $factory->createByFiles([$this->directory . '/a.php', $this->directory . '/sub/b.php'], 'odsl-factory-test-all'); + $sub = $factory->createByFiles([$this->directory . '/sub/b.php'], 'odsl-factory-test-sub'); + $factory->flushBatchedScan(); + + $this->assertCount(2, $all->locateIdentifiersByType(new DefaultReflector($all), new IdentifierType(IdentifierType::IDENTIFIER_CLASS))); + $this->assertCount(1, $sub->locateIdentifiersByType(new DefaultReflector($sub), new IdentifierType(IdentifierType::IDENTIFIER_CLASS))); + } + + private function hasClass(string $className): bool + { + // a new factory is a new run: nothing but the cache is shared + $locator = $this->createFactory()->createByFiles([$this->directory . '/a.php'], self::CACHE_KEY); + + return $locator->locateIdentifier(new DefaultReflector($locator), new Identifier($className, new IdentifierType(IdentifierType::IDENTIFIER_CLASS))) !== null; + } + + private function createFactory(): OptimizedDirectorySourceLocatorFactory + { + $container = self::getContainer(); + + return new OptimizedDirectorySourceLocatorFactory( + $container->getByType(FileNodesFetcher::class), + $container->getService('fileFinderScan'), + $container->getByType(PhpVersion::class), + $container->getByType(SymbolFinderInFiles::class), + $this->cache, + new FileContentHasher(), + new FileStatSignatures(), + $this->directory . '-tmp', + ); + } + + private function getVariableCacheKey(): string + { + return sprintf('v2-%s', self::getContainer()->getByType(PhpVersion::class)->supportsEnums() ? 'enums' : 'no-enums'); + } + + private function waitUntilInThePast(string $file): void + { + clearstatcache(); + $stat = stat($file); + $this->assertIsArray($stat); + $latest = max($stat['mtime'], $stat['ctime']); + while (time() <= $latest) { + usleep(50_000); + } + } + +} From 43f9c1bdd2ca9ef052daf26fede593fd2b63c9b0 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 30 Sep 2026 13:31:38 +0200 Subject: [PATCH 09/10] Scan the directory locators of the lazy source locator in one batch Before forking, PreForkDirectorySymbolScanner collects every directory locator and scans them in one go. With a single worker the locators are created lazily in that worker instead, where each directory was scanned on its own. The lazy initializer now batches them the same way. beginBatchedScan() and flushBatchedScan() switched the factory into a collecting mode that everything calling it in between took part in. createBatch() returns an object instead: the locators added to it are scanned by its scan(), and the repository and the Composer locator maker add to it when they are given one. A batch can hold two locators with the same cache key (odsl-installed-files of two Composer projects), and taking the scan lock for the second one does not wait for the lock the same process holds for the first. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ --- .../BetterReflectionSourceLocatorFactory.php | 35 ++++--- ...JsonAndInstalledJsonSourceLocatorMaker.php | 12 ++- .../OptimizedDirectorySourceLocator.php | 20 ++-- .../OptimizedDirectorySourceLocatorBatch.php | 74 ++++++++++++++ ...OptimizedDirectorySourceLocatorFactory.php | 96 +++++++------------ ...imizedDirectorySourceLocatorRepository.php | 8 +- .../PreForkDirectorySymbolScanner.php | 14 ++- ...mizedDirectorySourceLocatorFactoryTest.php | 13 +-- 8 files changed, 170 insertions(+), 102 deletions(-) create mode 100644 src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorBatch.php diff --git a/src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php b/src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php index 41914c4870f..4c9d522ee1e 100644 --- a/src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php +++ b/src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php @@ -23,6 +23,7 @@ use PHPStan\Reflection\BetterReflection\SourceLocator\ComposerJsonAndInstalledJsonSourceLocatorMaker; use PHPStan\Reflection\BetterReflection\SourceLocator\FileNodesFetcher; use PHPStan\Reflection\BetterReflection\SourceLocator\LazySourceLocator; +use PHPStan\Reflection\BetterReflection\SourceLocator\OptimizedDirectorySourceLocatorFactory; use PHPStan\Reflection\BetterReflection\SourceLocator\OptimizedDirectorySourceLocatorRepository; use PHPStan\Reflection\BetterReflection\SourceLocator\OptimizedPsrAutoloaderLocatorFactory; use PHPStan\Reflection\BetterReflection\SourceLocator\OptimizedSingleFileSourceLocatorRepository; @@ -64,6 +65,7 @@ public function __construct( private ReflectionSourceStubber $reflectionSourceStubber, private OptimizedSingleFileSourceLocatorRepository $optimizedSingleFileSourceLocatorRepository, private OptimizedDirectorySourceLocatorRepository $optimizedDirectorySourceLocatorRepository, + private OptimizedDirectorySourceLocatorFactory $optimizedDirectorySourceLocatorFactory, private ComposerJsonAndInstalledJsonSourceLocatorMaker $composerJsonAndInstalledJsonSourceLocatorMaker, private OptimizedPsrAutoloaderLocatorFactory $optimizedPsrAutoloaderLocatorFactory, private FileNodesFetcher $fileNodesFetcher, @@ -131,23 +133,32 @@ public function create(): SourceLocator $fileLocators[] = $this->optimizedSingleFileSourceLocatorRepository->getOrCreate($analysedFile); } - $directories = array_unique(array_merge($analysedDirectories, $this->scanDirectories)); - foreach ($directories as $directory) { - $fileLocators[] = $this->optimizedDirectorySourceLocatorRepository->getOrCreate($directory); - } - - $astPhp8Locator = new Locator($this->php8Parser); + // The directory locators - the analysed and scanned directories here, the Composer classmap + // paths below - are scanned together once they are all known, the way + // PreForkDirectorySymbolScanner does it before forking: a file two of them reach is looked + // at once. + $batch = $this->optimizedDirectorySourceLocatorFactory->createBatch(); + try { + $directories = array_unique(array_merge($analysedDirectories, $this->scanDirectories)); + foreach ($directories as $directory) { + $fileLocators[] = $this->optimizedDirectorySourceLocatorRepository->getOrCreate($directory, $batch); + } - $composerLocators = []; + $composerLocators = []; - foreach ($this->composerAutoloaderProjectPaths as $composerAutoloaderProjectPath) { - $locator = $this->composerJsonAndInstalledJsonSourceLocatorMaker->create($composerAutoloaderProjectPath); - if ($locator === null) { - continue; + foreach ($this->composerAutoloaderProjectPaths as $composerAutoloaderProjectPath) { + $locator = $this->composerJsonAndInstalledJsonSourceLocatorMaker->create($composerAutoloaderProjectPath, $batch); + if ($locator === null) { + continue; + } + $composerLocators[] = $locator; } - $composerLocators[] = $locator; + } finally { + $batch->scan(); } + $astPhp8Locator = new Locator($this->php8Parser); + if (count($composerLocators) > 0) { $fileLocators[] = new SkipPolyfillSourceLocator(new AggregateSourceLocator($composerLocators), $this->phpVersion); } diff --git a/src/Reflection/BetterReflection/SourceLocator/ComposerJsonAndInstalledJsonSourceLocatorMaker.php b/src/Reflection/BetterReflection/SourceLocator/ComposerJsonAndInstalledJsonSourceLocatorMaker.php index 58637b2b11f..5e202ddbf65 100644 --- a/src/Reflection/BetterReflection/SourceLocator/ComposerJsonAndInstalledJsonSourceLocatorMaker.php +++ b/src/Reflection/BetterReflection/SourceLocator/ComposerJsonAndInstalledJsonSourceLocatorMaker.php @@ -41,7 +41,11 @@ public function __construct( { } - public function create(string $projectInstallationPath): ?SourceLocator + /** + * With a batch, the directory locators are added to it, and the returned locator cannot be used + * before the batch is scanned. + */ + public function create(string $projectInstallationPath, ?OptimizedDirectorySourceLocatorBatch $batch = null): ?SourceLocator { $composer = ComposerHelper::getComposerConfig($projectInstallationPath); @@ -115,7 +119,7 @@ public function create(string $projectInstallationPath): ?SourceLocator $files = []; foreach ($classMapPaths as $classMapPath) { if (is_dir($classMapPath)) { - $locators[] = $this->optimizedDirectorySourceLocatorRepository->getOrCreate($classMapPath); + $locators[] = $this->optimizedDirectorySourceLocatorRepository->getOrCreate($classMapPath, $batch); continue; } if (!is_file($classMapPath)) { @@ -131,7 +135,9 @@ public function create(string $projectInstallationPath): ?SourceLocator } if (count($files) > 0) { - $locators[] = $this->optimizedDirectorySourceLocatorFactory->createByFiles($files, 'odsl-installed-files'); + $locators[] = $batch !== null + ? $batch->createByFiles($files, 'odsl-installed-files') + : $this->optimizedDirectorySourceLocatorFactory->createByFiles($files, 'odsl-installed-files'); } $binDir = ComposerHelper::getBinDirFromComposerConfig($projectInstallationPath, $composer); diff --git a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php index de959b6f38c..76f0f58acfe 100644 --- a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php +++ b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php @@ -43,7 +43,7 @@ public function __construct( private array $classToFile, private array $functionToFiles, private array $constantToFile, - private bool $awaitingBatchedScan = false, + private bool $awaitingScan = false, ) { } @@ -53,21 +53,21 @@ public function __construct( * * The factory hands these out before the scan that produces their contents * has run, so that one scan can cover every directory at once - * (see OptimizedDirectorySourceLocatorFactory::flushBatchedScan()). Nothing - * may look a symbol up in between - the maps are empty, so a lookup would - * quietly answer "not found" - which is what the flag guards. + * (see OptimizedDirectorySourceLocatorBatch). Nothing may look a symbol up + * in between - the maps are empty, so a lookup would quietly answer "not + * found" - which is what the flag guards. * * @param array $classToFile * @param array> $functionToFiles * @param array $constantToFile * @internal */ - public function fillBatchedScan(array $classToFile, array $functionToFiles, array $constantToFile): void + public function fillScanned(array $classToFile, array $functionToFiles, array $constantToFile): void { $this->classToFile = $classToFile; $this->functionToFiles = $functionToFiles; $this->constantToFile = $constantToFile; - $this->awaitingBatchedScan = false; + $this->awaitingScan = false; } /** @@ -89,8 +89,8 @@ private function getCacheKeys(string $file, Identifier $identifier): array #[Override] public function locateIdentifier(Reflector $reflector, Identifier $identifier): ?Reflection { - if ($this->awaitingBatchedScan) { - throw new ShouldNotHappenException('Symbols were looked up in a directory whose batched scan has not been flushed yet.'); + if ($this->awaitingScan) { + throw new ShouldNotHappenException('Symbols were looked up in a directory whose batch has not been scanned yet.'); } if ($identifier->isClass()) { @@ -250,8 +250,8 @@ private function findFilesByFunction(string $functionName): array #[Override] public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array { - if ($this->awaitingBatchedScan) { - throw new ShouldNotHappenException('Symbols were looked up in a directory whose batched scan has not been flushed yet.'); + if ($this->awaitingScan) { + throw new ShouldNotHappenException('Symbols were looked up in a directory whose batch has not been scanned yet.'); } $reflections = []; diff --git a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorBatch.php b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorBatch.php new file mode 100644 index 00000000000..9d0b828ae2b --- /dev/null +++ b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorBatch.php @@ -0,0 +1,74 @@ + */ + private array $requests = []; + + /** + * @param Closure(string): string[] $findFiles + * @param Closure(): OptimizedDirectorySourceLocator $createLocator + * @param Closure(list): void $scan + */ + public function __construct( + private Closure $findFiles, + private Closure $createLocator, + private Closure $scan, + ) + { + } + + public function createByDirectory(string $directory): OptimizedDirectorySourceLocator + { + return $this->add(sprintf('odsl-%s', $directory), ($this->findFiles)($directory)); + } + + /** + * @param string[] $files + * @param non-empty-string&literal-string $uniqueCacheIdentifier + */ + public function createByFiles(array $files, string $uniqueCacheIdentifier): OptimizedDirectorySourceLocator + { + return $this->add($uniqueCacheIdentifier, $files); + } + + /** + * Fills in every locator created since the last scan. + */ + public function scan(): void + { + $requests = $this->requests; + $this->requests = []; + if ($requests === []) { + return; + } + + ($this->scan)($requests); + } + + /** + * @param non-empty-string $cacheKey + * @param string[] $files + */ + private function add(string $cacheKey, array $files): OptimizedDirectorySourceLocator + { + $locator = ($this->createLocator)(); + $this->requests[] = [$cacheKey, $files, $locator]; + + return $locator; + } + +} diff --git a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactory.php b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactory.php index f856e844dec..fdb20040868 100644 --- a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactory.php +++ b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactory.php @@ -47,13 +47,6 @@ final class OptimizedDirectorySourceLocatorFactory private const SCAN_LOCK_POLL_INTERVAL_MICROSECONDS = 50_000; - /** - * Directories collected for a batched scan, null when not batching. - * - * @var list|null - */ - private ?array $batchedScan = null; - /** * The files this process already checked or scanned, as the caches keep them: [content hash, * stat signature, classes, functions, constants]. The directories a process builds locators for @@ -80,7 +73,11 @@ public function __construct( public function createByDirectory(string $directory): OptimizedDirectorySourceLocator { - return $this->createLocator(sprintf('odsl-%s', $directory), $this->fileFinder->findFiles([$directory])->getFiles()); + $batch = $this->createBatch(); + $locator = $batch->createByDirectory($directory); + $batch->scan(); + + return $locator; } /** @@ -89,59 +86,34 @@ public function createByDirectory(string $directory): OptimizedDirectorySourceLo */ public function createByFiles(array $files, string $uniqueCacheIdentifier): OptimizedDirectorySourceLocator { - return $this->createLocator($uniqueCacheIdentifier, $files); - } - - /** - * Starts collecting the directories asked for instead of scanning each one - * as it comes, so that flushBatchedScan() can cover all of them in a single - * scan: a file reachable from two directories is read once rather than - * twice, and the per-call costs are paid once instead of per directory. - */ - public function beginBatchedScan(): void - { - $this->batchedScan = []; - } - - /** - * Scans everything collected since beginBatchedScan() at once and fills in - * the locators handed out in the meantime. - */ - public function flushBatchedScan(): void - { - $batched = $this->batchedScan; - $this->batchedScan = null; - if ($batched === null || $batched === []) { - return; - } + $batch = $this->createBatch(); + $locator = $batch->createByFiles($files, $uniqueCacheIdentifier); + $batch->scan(); - $this->scan($batched); + return $locator; } /** - * @param non-empty-string $cacheKey - * @param string[] $files + * For creating several locators and scanning them in one go. */ - private function createLocator(string $cacheKey, array $files): OptimizedDirectorySourceLocator + public function createBatch(): OptimizedDirectorySourceLocatorBatch { - $locator = new OptimizedDirectorySourceLocator( - $this->fileNodesFetcher, - $this->cache, - $this->phpVersion, - $this->fileContentHasher, - [], - [], - [], - awaitingBatchedScan: true, + return new OptimizedDirectorySourceLocatorBatch( + fn (string $directory): array => $this->fileFinder->findFiles([$directory])->getFiles(), + fn (): OptimizedDirectorySourceLocator => new OptimizedDirectorySourceLocator( + $this->fileNodesFetcher, + $this->cache, + $this->phpVersion, + $this->fileContentHasher, + [], + [], + [], + awaitingScan: true, + ), + function (array $requests): void { + $this->scan($requests); + }, ); - - if ($this->batchedScan !== null) { - $this->batchedScan[] = [$cacheKey, $files, $locator]; - } else { - $this->scan([[$cacheKey, $files, $locator]]); - } - - return $locator; } /** @@ -160,18 +132,20 @@ private function scan(array $requests): void $cachedEntries = []; foreach ($requests as $i => [$cacheKey]) { $cached = $this->loadCachedSymbols($cacheKey, $variableCacheKey); - if ($cached === null) { - // On a cold cache every parallel worker that is not forked from a process that did - // the scan already builds the same locators at once, and would scan the same files. - // The first worker to take the lock scans and saves; the rest block until it - // releases, then read the cache it wrote. + // On a cold cache every parallel worker that is not forked from a process that did + // the scan already builds the same locators at once, and would scan the same files. + // The first worker to take the lock scans and saves; the rest block until it + // releases, then read the cache it wrote. The locators of a batch can share a cache + // key (odsl-installed-files of two Composer projects), and a second lock of the same + // file would wait for this very process. + if ($cached === null && !array_key_exists($cacheKey, $scanLocks)) { $scanLock = $this->acquireDirectoryScanLock($cacheKey . $variableCacheKey); if ($scanLock !== null) { $cached = $this->loadCachedSymbols($cacheKey, $variableCacheKey); if ($cached !== null) { $this->releaseDirectoryScanLock($scanLock); } else { - $scanLocks[] = $scanLock; + $scanLocks[$cacheKey] = $scanLock; } } } @@ -233,7 +207,7 @@ private function scan(array $requests): void } [$classToFile, $functionToFiles, $constantToFile] = $this->changeStructure($entry); - $locator->fillBatchedScan($classToFile, $functionToFiles, $constantToFile); + $locator->fillScanned($classToFile, $functionToFiles, $constantToFile); } } finally { // Release even if scanning or saving throws, so a failing worker cannot leave other diff --git a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorRepository.php b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorRepository.php index e0404ad6297..c6386018441 100644 --- a/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorRepository.php +++ b/src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorRepository.php @@ -16,13 +16,17 @@ public function __construct(private OptimizedDirectorySourceLocatorFactory $fact { } - public function getOrCreate(string $directory): OptimizedDirectorySourceLocator + /** + * With a batch, a locator not created yet is added to it, and cannot be used before the batch + * is scanned. + */ + public function getOrCreate(string $directory, ?OptimizedDirectorySourceLocatorBatch $batch = null): OptimizedDirectorySourceLocator { if (array_key_exists($directory, $this->locators)) { return $this->locators[$directory]; } - $this->locators[$directory] = $this->factory->createByDirectory($directory); + $this->locators[$directory] = $batch !== null ? $batch->createByDirectory($directory) : $this->factory->createByDirectory($directory); return $this->locators[$directory]; } diff --git a/src/Reflection/BetterReflection/SourceLocator/PreForkDirectorySymbolScanner.php b/src/Reflection/BetterReflection/SourceLocator/PreForkDirectorySymbolScanner.php index e99d873d450..d31b1ce251b 100644 --- a/src/Reflection/BetterReflection/SourceLocator/PreForkDirectorySymbolScanner.php +++ b/src/Reflection/BetterReflection/SourceLocator/PreForkDirectorySymbolScanner.php @@ -73,25 +73,23 @@ public function scanBeforeFork(): void // two directories both reach is then read once instead of twice, and // the scan pays its per-call costs once instead of per directory: // measured over this repository's tree, 0.32s -> 0.16s. - $this->optimizedDirectorySourceLocatorFactory->beginBatchedScan(); + $batch = $this->optimizedDirectorySourceLocatorFactory->createBatch(); try { foreach (array_unique(array_merge($directories, $this->scanDirectories)) as $directory) { - $this->optimizedDirectorySourceLocatorRepository->getOrCreate($directory); + $this->optimizedDirectorySourceLocatorRepository->getOrCreate($directory, $batch); } foreach ($this->composerAutoloaderProjectPaths as $composerAutoloaderProjectPath) { // the aggregate locator is thrown away - what matters is that the // directory locators it builds land in the repository's memo, // which the forked children inherit - $this->composerJsonAndInstalledJsonSourceLocatorMaker->create($composerAutoloaderProjectPath); + $this->composerJsonAndInstalledJsonSourceLocatorMaker->create($composerAutoloaderProjectPath, $batch); } - - $this->optimizedDirectorySourceLocatorFactory->flushBatchedScan(); } finally { - // a throw must not leave the factory collecting into a batch that - // nobody will flush - $this->optimizedDirectorySourceLocatorFactory->flushBatchedScan(); + // the locators already in the repository's memo must not be left + // unscanned by a throw + $batch->scan(); } } diff --git a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactoryTest.php b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactoryTest.php index 34ba8a9f0f5..4e74e34b99f 100644 --- a/tests/PHPStan/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactoryTest.php +++ b/tests/PHPStan/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactoryTest.php @@ -81,20 +81,21 @@ public function testUnchangedFileIsTrustedByItsSignature(): void $this->assertTrue($this->hasClass('OdslFactoryTest\\SecondWithLongerName')); } - public function testBatchedScanFillsEveryLocator(): void + public function testBatchFillsEveryLocator(): void { mkdir($this->directory . '/sub'); file_put_contents($this->directory . '/a.php', 'directory . '/sub/b.php', 'createFactory(); - $factory->beginBatchedScan(); - $all = $factory->createByFiles([$this->directory . '/a.php', $this->directory . '/sub/b.php'], 'odsl-factory-test-all'); - $sub = $factory->createByFiles([$this->directory . '/sub/b.php'], 'odsl-factory-test-sub'); - $factory->flushBatchedScan(); + $batch = $this->createFactory()->createBatch(); + $all = $batch->createByFiles([$this->directory . '/a.php', $this->directory . '/sub/b.php'], 'odsl-factory-test-all'); + $sub = $batch->createByFiles([$this->directory . '/sub/b.php'], 'odsl-factory-test-sub'); + $sameKey = $batch->createByFiles([$this->directory . '/a.php'], 'odsl-factory-test-sub'); + $batch->scan(); $this->assertCount(2, $all->locateIdentifiersByType(new DefaultReflector($all), new IdentifierType(IdentifierType::IDENTIFIER_CLASS))); $this->assertCount(1, $sub->locateIdentifiersByType(new DefaultReflector($sub), new IdentifierType(IdentifierType::IDENTIFIER_CLASS))); + $this->assertCount(1, $sameKey->locateIdentifiersByType(new DefaultReflector($sameKey), new IdentifierType(IdentifierType::IDENTIFIER_CLASS))); } private function hasClass(string $className): bool From ec0d4b6b0bad6f0d9d1c730779892030555dc415 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Wed, 30 Sep 2026 13:31:43 +0200 Subject: [PATCH 10/10] Bump expected turbo version --- src/Turbo/TurboExtensionEnabler.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Turbo/TurboExtensionEnabler.php b/src/Turbo/TurboExtensionEnabler.php index cc46e2c4ec1..2b54632598c 100644 --- a/src/Turbo/TurboExtensionEnabler.php +++ b/src/Turbo/TurboExtensionEnabler.php @@ -33,7 +33,7 @@ final class TurboExtensionEnabler { - public const EXPECTED_EXTENSION_VERSION = '005b513'; + public const EXPECTED_EXTENSION_VERSION = '63cd54c'; private static bool $active = false;