Skip to content

Commit 866f50d

Browse files
ondrejmirtesclaude
andcommitted
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ
1 parent 3e2db51 commit 866f50d

6 files changed

Lines changed: 209 additions & 48 deletions

File tree

‎src/Analyser/ResultCache/ResultCacheManager.php‎

Lines changed: 15 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
use PHPStan\File\CouldNotWriteFileException;
2323
use PHPStan\File\FileFinder;
2424
use PHPStan\File\FileHelper;
25+
use PHPStan\File\FileStatSignatures;
2526
use PHPStan\Internal\ArrayHelper;
2627
use PHPStan\Internal\ComposerHelper;
2728
use PHPStan\Php\ComposerPhpVersionFactory;
@@ -81,7 +82,6 @@
8182
use function uniqid;
8283
use function unlink;
8384
use function unserialize;
84-
use const DIRECTORY_SEPARATOR;
8585
use const PHP_VERSION_ID;
8686
use const SEEK_CUR;
8787

@@ -158,7 +158,7 @@ final class ResultCacheManager
158158
*
159159
* @var array<string, string>
160160
*/
161-
private array $fileStatSignatures = [];
161+
private array $recordedFileStats = [];
162162

163163
private ?ResultCachePathTransformer $pathTransformer = null;
164164

@@ -223,6 +223,7 @@ public function __construct(
223223
private string $anchorDirectory,
224224
private PhpVersion $phpVersion,
225225
private ComposerPhpVersionFactory $composerPhpVersionFactory,
226+
private FileStatSignatures $fileStatSignatures,
226227
)
227228
{
228229
}
@@ -291,7 +292,7 @@ public function restore(array $allAnalysedFiles, bool $debug, bool $onlyFiles, ?
291292
{
292293
$this->restoredCacheUnchanged = false;
293294
$this->restoredStubFiles = [];
294-
$this->fileStatSignatures = [];
295+
$this->recordedFileStats = [];
295296

296297
$startTime = microtime(true);
297298
$analysedFileStats = $this->statAnalysedFiles($allAnalysedFiles);
@@ -2511,8 +2512,8 @@ private function createDependencyEntry(string $file, array $currentFileHashes):
25112512
'fileHash' => $currentFileHashes[$file] ?? $this->getDependencyFileHash($file),
25122513
'dependentFiles' => [],
25132514
];
2514-
if (array_key_exists($file, $currentFileHashes) && array_key_exists($file, $this->fileStatSignatures)) {
2515-
$entry['fileStat'] = $this->fileStatSignatures[$file];
2515+
if (array_key_exists($file, $currentFileHashes) && array_key_exists($file, $this->recordedFileStats)) {
2516+
$entry['fileStat'] = $this->recordedFileStats[$file];
25162517
}
25172518

25182519
return $entry;
@@ -2540,30 +2541,23 @@ private function statAnalysedFiles(array $allAnalysedFiles): array
25402541

25412542
/**
25422543
* Hashing every analysed file is the bulk of what a run with nothing to re-analyse costs - on
2543-
* Drupal core almost a second for 11k files. A file whose size, mtime, ctime, inode and device
2544-
* are what they were when it was last hashed has not been written to since, so the hash the
2545-
* cache recorded then is reused - the check git makes against its index. ctime cannot be set
2546-
* back the way mtime can (touch, an extracted archive), and a replaced file has a new inode.
2547-
*
2548-
* A signature is only recorded for a file last modified before the second its hash was taken:
2549-
* the timestamps have a one-second granularity, so a file written again within that same second
2550-
* would keep a matching signature over different contents. On Windows the ctime PHP reports is
2551-
* the creation time, which a file rewritten in place keeps, so nothing is reused there.
2544+
* Drupal core almost a second for 11k files. A file whose stat signature is what it was when it
2545+
* was last hashed has not been written to since, so the hash the cache recorded then is reused
2546+
* - see FileStatSignatures.
25522547
*
25532548
* @param array<string, array<int|string, int>> $analysedFileStats
25542549
* @param array<string, array{fileHash: string, fileStat?: string, dependentFiles: list<string>, usedTraitDependentFiles?: list<string>}>|null $cachedDependencies
25552550
* @return array<string, string>
25562551
*/
25572552
private function hashAnalysedFiles(array $analysedFileStats, ?array $cachedDependencies): array
25582553
{
2559-
$now = time();
2560-
$trustSignatures = DIRECTORY_SEPARATOR === '/';
2554+
$signatures = $this->fileStatSignatures->begin();
25612555
$hashes = [];
25622556
foreach ($analysedFileStats as $file => $stat) {
2563-
$signature = sprintf('%d:%d:%d:%d:%d', $stat['size'], $stat['mtime'], $stat['ctime'], $stat['ino'], $stat['dev']);
2557+
$signature = $signatures->fromStat($stat);
25642558
$cachedEntry = $cachedDependencies[$file] ?? null;
25652559
if (
2566-
$trustSignatures
2560+
$signature !== null
25672561
&& $cachedEntry !== null
25682562
&& ($cachedEntry['fileStat'] ?? null) === $signature
25692563
&& $cachedEntry['fileHash'] !== self::MISSING_FILE_HASH
@@ -2576,11 +2570,11 @@ private function hashAnalysedFiles(array $analysedFileStats, ?array $cachedDepen
25762570
}
25772571

25782572
$hashes[$file] = $hash;
2579-
if (!$trustSignatures || $stat['mtime'] >= $now || $stat['ctime'] >= $now) {
2573+
if ($signature === null) {
25802574
continue;
25812575
}
25822576

2583-
$this->fileStatSignatures[$file] = $signature;
2577+
$this->recordedFileStats[$file] = $signature;
25842578
}
25852579

25862580
return $hashes;
@@ -2594,7 +2588,7 @@ private function hashAnalysedFiles(array $analysedFileStats, ?array $cachedDepen
25942588
*/
25952589
private function fileStatSignaturesDiffer(array $cachedDependencies): bool
25962590
{
2597-
foreach ($this->fileStatSignatures as $file => $signature) {
2591+
foreach ($this->recordedFileStats as $file => $signature) {
25982592
if (!array_key_exists($file, $cachedDependencies)) {
25992593
continue;
26002594
}

‎src/File/DirectoryWalker.php‎

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
use function str_contains;
3030
use function str_starts_with;
3131
use function substr;
32-
use function time;
3332
use function unlink;
3433
use function unserialize;
3534
use const DIRECTORY_SEPARATOR;
@@ -49,11 +48,8 @@
4948
* Reading the directories is what a walk costs - on a tree the size of Drupal core (28k directories)
5049
* about a second, every run, in the main process and again in the worker that builds the symbol
5150
* index. What a directory contains changes only when an entry is added, removed or renamed in it,
52-
* and each of those updates the directory's mtime and ctime, so the listings are kept in tmpDir and
53-
* a directory whose stat still matches is not read again. ctime cannot be set back by touch or by
54-
* extracting an archive, and a directory modified in the very second its listing is read is not
55-
* kept: a second change within that same second would leave its stat unchanged. The same technique
56-
* is behind git's untracked cache.
51+
* so the listings are kept in tmpDir and a directory whose stat signature still matches (see
52+
* FileStatSignatures) is not read again. The same technique is behind git's untracked cache.
5753
*
5854
* The walk yields what Symfony Finder yields for files()->name()->followLinks() with its default
5955
* ignores - dot files and VCS directories left out, symlinks followed, files in readdir order - and
@@ -64,7 +60,7 @@
6460
final class DirectoryWalker
6561
{
6662

67-
private const LISTINGS_FORMAT = 'directoryListings-v1';
63+
private const LISTINGS_FORMAT = 'directoryListings-v2';
6864

6965
/** The directories Finder's ignoreVCS() leaves out; the ones starting with a dot are left out anyway. */
7066
private const VCS_DIRECTORIES = ['_svn' => true, 'CVS' => true, '_darcs' => true];
@@ -73,12 +69,12 @@ final class DirectoryWalker
7369
private array $cachedWalks = [];
7470

7571
/**
76-
* Directory path => [mtime, ctime, inode, entries]. The entries are the directory's names in
72+
* Directory path => [stat signature, entries]. The entries are the directory's names in
7773
* readdir order, each prefixed by d (directory), f (anything else - Finder's files() only leaves
7874
* out directories, so a broken symlink is a file too) or l (symlink - resolved on every walk,
7975
* because its target can change without this directory changing), joined by NUL.
8076
*
81-
* @var array<string, array{int, int, int, string}>|null
77+
* @var array<string, array{string, string}>|null
8278
*/
8379
private ?array $listings = null;
8480

@@ -88,6 +84,7 @@ final class DirectoryWalker
8884
* @param string $tmpDir where the listings are kept between runs, nowhere when empty
8985
*/
9086
public function __construct(
87+
private FileStatSignatures $fileStatSignatures,
9188
#[AutowiredParameter]
9289
private string $tmpDir = '',
9390
)
@@ -155,7 +152,7 @@ private function walkWithListings(string $directory, array $fileExtensions): ?ar
155152

156153
$files = [];
157154
$visited = [];
158-
if (!$this->walkDirectory($directory, Glob::toRegex('*.{' . implode(',', $fileExtensions) . '}'), time(), $files, $visited)) {
155+
if (!$this->walkDirectory($directory, Glob::toRegex('*.{' . implode(',', $fileExtensions) . '}'), $this->fileStatSignatures->begin(), $files, $visited)) {
159156
return null;
160157
}
161158

@@ -168,25 +165,26 @@ private function walkWithListings(string $directory, array $fileExtensions): ?ar
168165
* @param list<string> $files
169166
* @param array<string, true> $visited
170167
*/
171-
private function walkDirectory(string $directory, string $pattern, int $now, array &$files, array &$visited): bool
168+
private function walkDirectory(string $directory, string $pattern, FileStatSignatureReader $signatures, array &$files, array &$visited): bool
172169
{
173170
$stat = @stat($directory);
174171
if ($stat === false) {
175172
return false;
176173
}
177174

178175
$visited[$directory] = true;
176+
$signature = $signatures->fromStat($stat);
179177
$listing = $this->listings[$directory] ?? null;
180-
if ($listing !== null && $listing[0] === $stat['mtime'] && $listing[1] === $stat['ctime'] && $listing[2] === $stat['ino']) {
181-
$entries = $listing[3];
178+
if ($signature !== null && $listing !== null && $listing[0] === $signature) {
179+
$entries = $listing[1];
182180
} else {
183181
$entries = $this->readDirectory($directory);
184182
if ($entries === null) {
185183
return false;
186184
}
187185

188-
if ($stat['mtime'] < $now && $stat['ctime'] < $now) {
189-
$this->listings[$directory] = [$stat['mtime'], $stat['ctime'], $stat['ino'], $entries];
186+
if ($signature !== null) {
187+
$this->listings[$directory] = [$signature, $entries];
190188
} else {
191189
unset($this->listings[$directory]);
192190
}
@@ -213,7 +211,7 @@ private function walkDirectory(string $directory, string $pattern, int $now, arr
213211
}
214212

215213
if ($type === 'd') {
216-
if (!$this->walkDirectory($path, $pattern, $now, $files, $visited)) {
214+
if (!$this->walkDirectory($path, $pattern, $signatures, $files, $visited)) {
217215
return false;
218216
}
219217

@@ -297,7 +295,7 @@ private function loadListings(): void
297295
return;
298296
}
299297

300-
/** @var array<string, array{int, int, int, string}> $listings */
298+
/** @var array<string, array{string, string}> $listings */
301299
$listings = $data['listings'];
302300
$this->listings = $listings;
303301
}
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\File;
4+
5+
use function sprintf;
6+
use function stat;
7+
use function str_contains;
8+
9+
/**
10+
* Signatures of files and directories read after FileStatSignatures::begin(). A signature is null
11+
* when the stat cannot vouch for the contents: the caller then neither reuses what it derived from
12+
* them before nor keeps the signature for next time.
13+
*
14+
* That includes a file or directory modified in the second the reading began or later. Timestamps
15+
* have a one-second granularity, so it could be written again within that second, after it was
16+
* read, and keep its signature over different contents. Everything modified before that second
17+
* was read afterwards, so any later change moves its timestamps.
18+
*/
19+
final class FileStatSignatureReader
20+
{
21+
22+
public function __construct(
23+
private int $startedAt,
24+
private bool $trusted,
25+
)
26+
{
27+
}
28+
29+
public function get(string $path): ?string
30+
{
31+
if (!$this->trusted || str_contains($path, '://')) {
32+
return null;
33+
}
34+
35+
$stat = @stat($path);
36+
if ($stat === false) {
37+
return null;
38+
}
39+
40+
return $this->fromStat($stat);
41+
}
42+
43+
/**
44+
* @param array<int|string, int> $stat what stat() returned for the path
45+
*/
46+
public function fromStat(array $stat): ?string
47+
{
48+
if (!$this->trusted || $stat['mtime'] >= $this->startedAt || $stat['ctime'] >= $this->startedAt) {
49+
return null;
50+
}
51+
52+
return sprintf('%d:%d:%d:%d:%d', $stat['size'], $stat['mtime'], $stat['ctime'], $stat['ino'], $stat['dev']);
53+
}
54+
55+
}

‎src/File/FileStatSignatures.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\File;
4+
5+
use PHPStan\DependencyInjection\AutowiredService;
6+
use function time;
7+
use const DIRECTORY_SEPARATOR;
8+
9+
/**
10+
* Tells from its stat alone that a file or directory has not been written to since it was last
11+
* read, the way git's index does: its size, mtime, ctime, inode and device are what they were.
12+
* ctime cannot be set back the way mtime can (touch, an extracted archive), and a replaced file
13+
* has a new inode. Adding, removing or renaming an entry of a directory updates its mtime and
14+
* ctime, so a directory whose signature matches still has the entries it had.
15+
*
16+
* A caller keeps the signature next to what it derived from the contents, and next time reuses
17+
* that as long as the signature it gets is the same - see FileStatSignatureReader.
18+
*/
19+
#[AutowiredService]
20+
final class FileStatSignatures
21+
{
22+
23+
/**
24+
* To be called before reading the contents the signatures are going to vouch for.
25+
*/
26+
public function begin(): FileStatSignatureReader
27+
{
28+
// On Windows, the ctime PHP reports is the creation time, which a file rewritten in place
29+
// keeps, and the mtime alone can be set back - nothing can be vouched for there.
30+
return new FileStatSignatureReader(time(), DIRECTORY_SEPARATOR === '/');
31+
}
32+
33+
}

‎tests/PHPStan/File/DirectoryWalkerTest.php‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ protected function setUp(): void
3636

3737
public function testWalkAlwaysSeesTheCurrentState(): void
3838
{
39-
$walker = new DirectoryWalker();
39+
$walker = new DirectoryWalker(new FileStatSignatures());
4040

4141
$beforeAdding = $walker->walk($this->directory, ['php']);
4242
file_put_contents($this->directory . '/second.php', '<?php');
@@ -48,7 +48,7 @@ public function testWalkAlwaysSeesTheCurrentState(): void
4848

4949
public function testWalkCachedIsSharedBetweenCalls(): void
5050
{
51-
$walker = new DirectoryWalker();
51+
$walker = new DirectoryWalker(new FileStatSignatures());
5252

5353
$firstWalk = $walker->walkCached($this->directory, ['php']);
5454
file_put_contents($this->directory . '/second.php', '<?php');
@@ -65,7 +65,7 @@ public function testWalkCachedIsSharedBetweenCalls(): void
6565

6666
public function testCachedWalksAreKeyedByExtensions(): void
6767
{
68-
$walker = new DirectoryWalker();
68+
$walker = new DirectoryWalker(new FileStatSignatures());
6969
file_put_contents($this->directory . '/notes.txt', 'x');
7070

7171
$phpFiles = $walker->walkCached($this->directory, ['php']);
@@ -80,11 +80,11 @@ public function testWalkYieldsWhatFinderYields(): void
8080
$tmpDir = $this->createTree();
8181

8282
$expected = $this->walkWithFinder($this->directory, ['php', 'sh', '']);
83-
$fresh = (new DirectoryWalker($tmpDir))->walk($this->directory, ['php', 'sh', '']);
83+
$fresh = (new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php', 'sh', '']);
8484
$this->waitUntilTheTreeIsInThePast();
8585
// stores the listings, now that they are no longer racy
86-
(new DirectoryWalker($tmpDir))->walk($this->directory, ['php', 'sh', '']);
87-
$fromListings = (new DirectoryWalker($tmpDir))->walk($this->directory, ['php', 'sh', '']);
86+
(new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php', 'sh', '']);
87+
$fromListings = (new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php', 'sh', '']);
8888

8989
$this->assertNotSame([], $expected);
9090
$this->assertSame($expected, $fresh);
@@ -96,23 +96,23 @@ public function testListingsSeeAddedAndRemovedFiles(): void
9696
{
9797
$tmpDir = $this->createTree();
9898
$this->waitUntilTheTreeIsInThePast();
99-
(new DirectoryWalker($tmpDir))->walk($this->directory, ['php']);
99+
(new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php']);
100100

101101
file_put_contents($this->directory . '/src/Added.php', '<?php');
102102
unlink($this->directory . '/src/Nested/Deep.php');
103103
rename($this->directory . '/src/Foo.php', $this->directory . '/src/Renamed.php');
104104

105105
$this->assertSame(
106106
$this->walkWithFinder($this->directory, ['php']),
107-
(new DirectoryWalker($tmpDir))->walk($this->directory, ['php']),
107+
(new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php']),
108108
);
109109
}
110110

111111
public function testReplacedDirectoryIsReadAgain(): void
112112
{
113113
$tmpDir = $this->createTree();
114114
$this->waitUntilTheTreeIsInThePast();
115-
(new DirectoryWalker($tmpDir))->walk($this->directory, ['php']);
115+
(new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php']);
116116

117117
unlink($this->directory . '/src/Nested/Deep.php');
118118
rmdir($this->directory . '/src/Nested');
@@ -121,7 +121,7 @@ public function testReplacedDirectoryIsReadAgain(): void
121121

122122
$this->assertContains(
123123
$this->directory . '/src/Nested/Other.php',
124-
(new DirectoryWalker($tmpDir))->walk($this->directory, ['php']),
124+
(new DirectoryWalker(new FileStatSignatures(), $tmpDir))->walk($this->directory, ['php']),
125125
);
126126
}
127127

0 commit comments

Comments
 (0)