Skip to content

Commit 43f9c1b

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GdwtgiEHwkF5BTz73jgHQQ
1 parent abdcbed commit 43f9c1b

8 files changed

Lines changed: 170 additions & 102 deletions

‎src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php‎

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
use PHPStan\Reflection\BetterReflection\SourceLocator\ComposerJsonAndInstalledJsonSourceLocatorMaker;
2424
use PHPStan\Reflection\BetterReflection\SourceLocator\FileNodesFetcher;
2525
use PHPStan\Reflection\BetterReflection\SourceLocator\LazySourceLocator;
26+
use PHPStan\Reflection\BetterReflection\SourceLocator\OptimizedDirectorySourceLocatorFactory;
2627
use PHPStan\Reflection\BetterReflection\SourceLocator\OptimizedDirectorySourceLocatorRepository;
2728
use PHPStan\Reflection\BetterReflection\SourceLocator\OptimizedPsrAutoloaderLocatorFactory;
2829
use PHPStan\Reflection\BetterReflection\SourceLocator\OptimizedSingleFileSourceLocatorRepository;
@@ -64,6 +65,7 @@ public function __construct(
6465
private ReflectionSourceStubber $reflectionSourceStubber,
6566
private OptimizedSingleFileSourceLocatorRepository $optimizedSingleFileSourceLocatorRepository,
6667
private OptimizedDirectorySourceLocatorRepository $optimizedDirectorySourceLocatorRepository,
68+
private OptimizedDirectorySourceLocatorFactory $optimizedDirectorySourceLocatorFactory,
6769
private ComposerJsonAndInstalledJsonSourceLocatorMaker $composerJsonAndInstalledJsonSourceLocatorMaker,
6870
private OptimizedPsrAutoloaderLocatorFactory $optimizedPsrAutoloaderLocatorFactory,
6971
private FileNodesFetcher $fileNodesFetcher,
@@ -131,23 +133,32 @@ public function create(): SourceLocator
131133
$fileLocators[] = $this->optimizedSingleFileSourceLocatorRepository->getOrCreate($analysedFile);
132134
}
133135

134-
$directories = array_unique(array_merge($analysedDirectories, $this->scanDirectories));
135-
foreach ($directories as $directory) {
136-
$fileLocators[] = $this->optimizedDirectorySourceLocatorRepository->getOrCreate($directory);
137-
}
138-
139-
$astPhp8Locator = new Locator($this->php8Parser);
136+
// The directory locators - the analysed and scanned directories here, the Composer classmap
137+
// paths below - are scanned together once they are all known, the way
138+
// PreForkDirectorySymbolScanner does it before forking: a file two of them reach is looked
139+
// at once.
140+
$batch = $this->optimizedDirectorySourceLocatorFactory->createBatch();
141+
try {
142+
$directories = array_unique(array_merge($analysedDirectories, $this->scanDirectories));
143+
foreach ($directories as $directory) {
144+
$fileLocators[] = $this->optimizedDirectorySourceLocatorRepository->getOrCreate($directory, $batch);
145+
}
140146

141-
$composerLocators = [];
147+
$composerLocators = [];
142148

143-
foreach ($this->composerAutoloaderProjectPaths as $composerAutoloaderProjectPath) {
144-
$locator = $this->composerJsonAndInstalledJsonSourceLocatorMaker->create($composerAutoloaderProjectPath);
145-
if ($locator === null) {
146-
continue;
149+
foreach ($this->composerAutoloaderProjectPaths as $composerAutoloaderProjectPath) {
150+
$locator = $this->composerJsonAndInstalledJsonSourceLocatorMaker->create($composerAutoloaderProjectPath, $batch);
151+
if ($locator === null) {
152+
continue;
153+
}
154+
$composerLocators[] = $locator;
147155
}
148-
$composerLocators[] = $locator;
156+
} finally {
157+
$batch->scan();
149158
}
150159

160+
$astPhp8Locator = new Locator($this->php8Parser);
161+
151162
if (count($composerLocators) > 0) {
152163
$fileLocators[] = new SkipPolyfillSourceLocator(new AggregateSourceLocator($composerLocators), $this->phpVersion);
153164
}

‎src/Reflection/BetterReflection/SourceLocator/ComposerJsonAndInstalledJsonSourceLocatorMaker.php‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,11 @@ public function __construct(
4141
{
4242
}
4343

44-
public function create(string $projectInstallationPath): ?SourceLocator
44+
/**
45+
* With a batch, the directory locators are added to it, and the returned locator cannot be used
46+
* before the batch is scanned.
47+
*/
48+
public function create(string $projectInstallationPath, ?OptimizedDirectorySourceLocatorBatch $batch = null): ?SourceLocator
4549
{
4650
$composer = ComposerHelper::getComposerConfig($projectInstallationPath);
4751

@@ -115,7 +119,7 @@ public function create(string $projectInstallationPath): ?SourceLocator
115119
$files = [];
116120
foreach ($classMapPaths as $classMapPath) {
117121
if (is_dir($classMapPath)) {
118-
$locators[] = $this->optimizedDirectorySourceLocatorRepository->getOrCreate($classMapPath);
122+
$locators[] = $this->optimizedDirectorySourceLocatorRepository->getOrCreate($classMapPath, $batch);
119123
continue;
120124
}
121125
if (!is_file($classMapPath)) {
@@ -131,7 +135,9 @@ public function create(string $projectInstallationPath): ?SourceLocator
131135
}
132136

133137
if (count($files) > 0) {
134-
$locators[] = $this->optimizedDirectorySourceLocatorFactory->createByFiles($files, 'odsl-installed-files');
138+
$locators[] = $batch !== null
139+
? $batch->createByFiles($files, 'odsl-installed-files')
140+
: $this->optimizedDirectorySourceLocatorFactory->createByFiles($files, 'odsl-installed-files');
135141
}
136142

137143
$binDir = ComposerHelper::getBinDirFromComposerConfig($projectInstallationPath, $composer);

‎src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocator.php‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ public function __construct(
4343
private array $classToFile,
4444
private array $functionToFiles,
4545
private array $constantToFile,
46-
private bool $awaitingBatchedScan = false,
46+
private bool $awaitingScan = false,
4747
)
4848
{
4949
}
@@ -53,21 +53,21 @@ public function __construct(
5353
*
5454
* The factory hands these out before the scan that produces their contents
5555
* has run, so that one scan can cover every directory at once
56-
* (see OptimizedDirectorySourceLocatorFactory::flushBatchedScan()). Nothing
57-
* may look a symbol up in between - the maps are empty, so a lookup would
58-
* quietly answer "not found" - which is what the flag guards.
56+
* (see OptimizedDirectorySourceLocatorBatch). Nothing may look a symbol up
57+
* in between - the maps are empty, so a lookup would quietly answer "not
58+
* found" - which is what the flag guards.
5959
*
6060
* @param array<string, string> $classToFile
6161
* @param array<string, array<int, string>> $functionToFiles
6262
* @param array<string, string> $constantToFile
6363
* @internal
6464
*/
65-
public function fillBatchedScan(array $classToFile, array $functionToFiles, array $constantToFile): void
65+
public function fillScanned(array $classToFile, array $functionToFiles, array $constantToFile): void
6666
{
6767
$this->classToFile = $classToFile;
6868
$this->functionToFiles = $functionToFiles;
6969
$this->constantToFile = $constantToFile;
70-
$this->awaitingBatchedScan = false;
70+
$this->awaitingScan = false;
7171
}
7272

7373
/**
@@ -89,8 +89,8 @@ private function getCacheKeys(string $file, Identifier $identifier): array
8989
#[Override]
9090
public function locateIdentifier(Reflector $reflector, Identifier $identifier): ?Reflection
9191
{
92-
if ($this->awaitingBatchedScan) {
93-
throw new ShouldNotHappenException('Symbols were looked up in a directory whose batched scan has not been flushed yet.');
92+
if ($this->awaitingScan) {
93+
throw new ShouldNotHappenException('Symbols were looked up in a directory whose batch has not been scanned yet.');
9494
}
9595

9696
if ($identifier->isClass()) {
@@ -250,8 +250,8 @@ private function findFilesByFunction(string $functionName): array
250250
#[Override]
251251
public function locateIdentifiersByType(Reflector $reflector, IdentifierType $identifierType): array
252252
{
253-
if ($this->awaitingBatchedScan) {
254-
throw new ShouldNotHappenException('Symbols were looked up in a directory whose batched scan has not been flushed yet.');
253+
if ($this->awaitingScan) {
254+
throw new ShouldNotHappenException('Symbols were looked up in a directory whose batch has not been scanned yet.');
255255
}
256256

257257
$reflections = [];

‎src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorBatch.php‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Reflection\BetterReflection\SourceLocator;
4+
5+
use Closure;
6+
use function sprintf;
7+
8+
/**
9+
* Directory locators created together and filled in by one scan once they are all known: a file
10+
* reachable from two of them is looked at once, and the cost of the scan is paid once instead of
11+
* per directory. The locators cannot be asked anything until scan() has run.
12+
*
13+
* Created by OptimizedDirectorySourceLocatorFactory::createBatch().
14+
*/
15+
final class OptimizedDirectorySourceLocatorBatch
16+
{
17+
18+
/** @var list<array{non-empty-string, string[], OptimizedDirectorySourceLocator}> */
19+
private array $requests = [];
20+
21+
/**
22+
* @param Closure(string): string[] $findFiles
23+
* @param Closure(): OptimizedDirectorySourceLocator $createLocator
24+
* @param Closure(list<array{non-empty-string, string[], OptimizedDirectorySourceLocator}>): void $scan
25+
*/
26+
public function __construct(
27+
private Closure $findFiles,
28+
private Closure $createLocator,
29+
private Closure $scan,
30+
)
31+
{
32+
}
33+
34+
public function createByDirectory(string $directory): OptimizedDirectorySourceLocator
35+
{
36+
return $this->add(sprintf('odsl-%s', $directory), ($this->findFiles)($directory));
37+
}
38+
39+
/**
40+
* @param string[] $files
41+
* @param non-empty-string&literal-string $uniqueCacheIdentifier
42+
*/
43+
public function createByFiles(array $files, string $uniqueCacheIdentifier): OptimizedDirectorySourceLocator
44+
{
45+
return $this->add($uniqueCacheIdentifier, $files);
46+
}
47+
48+
/**
49+
* Fills in every locator created since the last scan.
50+
*/
51+
public function scan(): void
52+
{
53+
$requests = $this->requests;
54+
$this->requests = [];
55+
if ($requests === []) {
56+
return;
57+
}
58+
59+
($this->scan)($requests);
60+
}
61+
62+
/**
63+
* @param non-empty-string $cacheKey
64+
* @param string[] $files
65+
*/
66+
private function add(string $cacheKey, array $files): OptimizedDirectorySourceLocator
67+
{
68+
$locator = ($this->createLocator)();
69+
$this->requests[] = [$cacheKey, $files, $locator];
70+
71+
return $locator;
72+
}
73+
74+
}

‎src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorFactory.php‎

Lines changed: 35 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -47,13 +47,6 @@ final class OptimizedDirectorySourceLocatorFactory
4747

4848
private const SCAN_LOCK_POLL_INTERVAL_MICROSECONDS = 50_000;
4949

50-
/**
51-
* Directories collected for a batched scan, null when not batching.
52-
*
53-
* @var list<array{non-empty-string, string[], OptimizedDirectorySourceLocator}>|null
54-
*/
55-
private ?array $batchedScan = null;
56-
5750
/**
5851
* The files this process already checked or scanned, as the caches keep them: [content hash,
5952
* stat signature, classes, functions, constants]. The directories a process builds locators for
@@ -80,7 +73,11 @@ public function __construct(
8073

8174
public function createByDirectory(string $directory): OptimizedDirectorySourceLocator
8275
{
83-
return $this->createLocator(sprintf('odsl-%s', $directory), $this->fileFinder->findFiles([$directory])->getFiles());
76+
$batch = $this->createBatch();
77+
$locator = $batch->createByDirectory($directory);
78+
$batch->scan();
79+
80+
return $locator;
8481
}
8582

8683
/**
@@ -89,59 +86,34 @@ public function createByDirectory(string $directory): OptimizedDirectorySourceLo
8986
*/
9087
public function createByFiles(array $files, string $uniqueCacheIdentifier): OptimizedDirectorySourceLocator
9188
{
92-
return $this->createLocator($uniqueCacheIdentifier, $files);
93-
}
94-
95-
/**
96-
* Starts collecting the directories asked for instead of scanning each one
97-
* as it comes, so that flushBatchedScan() can cover all of them in a single
98-
* scan: a file reachable from two directories is read once rather than
99-
* twice, and the per-call costs are paid once instead of per directory.
100-
*/
101-
public function beginBatchedScan(): void
102-
{
103-
$this->batchedScan = [];
104-
}
105-
106-
/**
107-
* Scans everything collected since beginBatchedScan() at once and fills in
108-
* the locators handed out in the meantime.
109-
*/
110-
public function flushBatchedScan(): void
111-
{
112-
$batched = $this->batchedScan;
113-
$this->batchedScan = null;
114-
if ($batched === null || $batched === []) {
115-
return;
116-
}
89+
$batch = $this->createBatch();
90+
$locator = $batch->createByFiles($files, $uniqueCacheIdentifier);
91+
$batch->scan();
11792

118-
$this->scan($batched);
93+
return $locator;
11994
}
12095

12196
/**
122-
* @param non-empty-string $cacheKey
123-
* @param string[] $files
97+
* For creating several locators and scanning them in one go.
12498
*/
125-
private function createLocator(string $cacheKey, array $files): OptimizedDirectorySourceLocator
99+
public function createBatch(): OptimizedDirectorySourceLocatorBatch
126100
{
127-
$locator = new OptimizedDirectorySourceLocator(
128-
$this->fileNodesFetcher,
129-
$this->cache,
130-
$this->phpVersion,
131-
$this->fileContentHasher,
132-
[],
133-
[],
134-
[],
135-
awaitingBatchedScan: true,
101+
return new OptimizedDirectorySourceLocatorBatch(
102+
fn (string $directory): array => $this->fileFinder->findFiles([$directory])->getFiles(),
103+
fn (): OptimizedDirectorySourceLocator => new OptimizedDirectorySourceLocator(
104+
$this->fileNodesFetcher,
105+
$this->cache,
106+
$this->phpVersion,
107+
$this->fileContentHasher,
108+
[],
109+
[],
110+
[],
111+
awaitingScan: true,
112+
),
113+
function (array $requests): void {
114+
$this->scan($requests);
115+
},
136116
);
137-
138-
if ($this->batchedScan !== null) {
139-
$this->batchedScan[] = [$cacheKey, $files, $locator];
140-
} else {
141-
$this->scan([[$cacheKey, $files, $locator]]);
142-
}
143-
144-
return $locator;
145117
}
146118

147119
/**
@@ -160,18 +132,20 @@ private function scan(array $requests): void
160132
$cachedEntries = [];
161133
foreach ($requests as $i => [$cacheKey]) {
162134
$cached = $this->loadCachedSymbols($cacheKey, $variableCacheKey);
163-
if ($cached === null) {
164-
// On a cold cache every parallel worker that is not forked from a process that did
165-
// the scan already builds the same locators at once, and would scan the same files.
166-
// The first worker to take the lock scans and saves; the rest block until it
167-
// releases, then read the cache it wrote.
135+
// On a cold cache every parallel worker that is not forked from a process that did
136+
// the scan already builds the same locators at once, and would scan the same files.
137+
// The first worker to take the lock scans and saves; the rest block until it
138+
// releases, then read the cache it wrote. The locators of a batch can share a cache
139+
// key (odsl-installed-files of two Composer projects), and a second lock of the same
140+
// file would wait for this very process.
141+
if ($cached === null && !array_key_exists($cacheKey, $scanLocks)) {
168142
$scanLock = $this->acquireDirectoryScanLock($cacheKey . $variableCacheKey);
169143
if ($scanLock !== null) {
170144
$cached = $this->loadCachedSymbols($cacheKey, $variableCacheKey);
171145
if ($cached !== null) {
172146
$this->releaseDirectoryScanLock($scanLock);
173147
} else {
174-
$scanLocks[] = $scanLock;
148+
$scanLocks[$cacheKey] = $scanLock;
175149
}
176150
}
177151
}
@@ -233,7 +207,7 @@ private function scan(array $requests): void
233207
}
234208

235209
[$classToFile, $functionToFiles, $constantToFile] = $this->changeStructure($entry);
236-
$locator->fillBatchedScan($classToFile, $functionToFiles, $constantToFile);
210+
$locator->fillScanned($classToFile, $functionToFiles, $constantToFile);
237211
}
238212
} finally {
239213
// Release even if scanning or saving throws, so a failing worker cannot leave other

‎src/Reflection/BetterReflection/SourceLocator/OptimizedDirectorySourceLocatorRepository.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,17 @@ public function __construct(private OptimizedDirectorySourceLocatorFactory $fact
1616
{
1717
}
1818

19-
public function getOrCreate(string $directory): OptimizedDirectorySourceLocator
19+
/**
20+
* With a batch, a locator not created yet is added to it, and cannot be used before the batch
21+
* is scanned.
22+
*/
23+
public function getOrCreate(string $directory, ?OptimizedDirectorySourceLocatorBatch $batch = null): OptimizedDirectorySourceLocator
2024
{
2125
if (array_key_exists($directory, $this->locators)) {
2226
return $this->locators[$directory];
2327
}
2428

25-
$this->locators[$directory] = $this->factory->createByDirectory($directory);
29+
$this->locators[$directory] = $batch !== null ? $batch->createByDirectory($directory) : $this->factory->createByDirectory($directory);
2630

2731
return $this->locators[$directory];
2832
}

0 commit comments

Comments
 (0)