Skip to content

Commit e441809

Browse files
ondrejmirtesclaude
ondrejmirtes
and
claude
committed
Hand out a batch object for scanning several directory locators at once
beginBatchedScan() and flushBatchedScan() switched the factory into a collecting mode that everything calling it in between took part in, and had to count nested batches. 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 no longer waits 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 204180d commit e441809

8 files changed

Lines changed: 149 additions & 107 deletions

‎src/Reflection/BetterReflection/BetterReflectionSourceLocatorFactory.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,24 +137,24 @@ public function create(): SourceLocator
137137
// paths below - are scanned together once they are all known, the way
138138
// PreForkDirectorySymbolScanner does it before forking: a file two of them reach is looked
139139
// at once.
140-
$this->optimizedDirectorySourceLocatorFactory->beginBatchedScan();
140+
$batch = $this->optimizedDirectorySourceLocatorFactory->createBatch();
141141
try {
142142
$directories = array_unique(array_merge($analysedDirectories, $this->scanDirectories));
143143
foreach ($directories as $directory) {
144-
$fileLocators[] = $this->optimizedDirectorySourceLocatorRepository->getOrCreate($directory);
144+
$fileLocators[] = $this->optimizedDirectorySourceLocatorRepository->getOrCreate($directory, $batch);
145145
}
146146

147147
$composerLocators = [];
148148

149149
foreach ($this->composerAutoloaderProjectPaths as $composerAutoloaderProjectPath) {
150-
$locator = $this->composerJsonAndInstalledJsonSourceLocatorMaker->create($composerAutoloaderProjectPath);
150+
$locator = $this->composerJsonAndInstalledJsonSourceLocatorMaker->create($composerAutoloaderProjectPath, $batch);
151151
if ($locator === null) {
152152
continue;
153153
}
154154
$composerLocators[] = $locator;
155155
}
156156
} finally {
157-
$this->optimizedDirectorySourceLocatorFactory->flushBatchedScan();
157+
$batch->scan();
158158
}
159159

160160
$astPhp8Locator = new Locator($this->php8Parser);

‎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: 33 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -47,15 +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-
57-
private int $nestedBatchedScans = 0;
58-
5950
/**
6051
* The files this process already checked or scanned, as the caches keep them: [content hash,
6152
* stat signature, classes, functions, constants]. The directories a process builds locators for
@@ -82,7 +73,11 @@ public function __construct(
8273

8374
public function createByDirectory(string $directory): OptimizedDirectorySourceLocator
8475
{
85-
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;
8681
}
8782

8883
/**
@@ -91,70 +86,32 @@ public function createByDirectory(string $directory): OptimizedDirectorySourceLo
9186
*/
9287
public function createByFiles(array $files, string $uniqueCacheIdentifier): OptimizedDirectorySourceLocator
9388
{
94-
return $this->createLocator($uniqueCacheIdentifier, $files);
95-
}
96-
97-
/**
98-
* Starts collecting the directories asked for instead of scanning each one
99-
* as it comes, so that flushBatchedScan() can cover all of them in a single
100-
* scan: a file reachable from two directories is read once rather than
101-
* twice, and the per-call costs are paid once instead of per directory.
102-
*/
103-
public function beginBatchedScan(): void
104-
{
105-
if ($this->batchedScan !== null) {
106-
// nested in another batch, which the outermost flush covers
107-
$this->nestedBatchedScans++;
108-
return;
109-
}
110-
111-
$this->batchedScan = [];
112-
}
89+
$batch = $this->createBatch();
90+
$locator = $batch->createByFiles($files, $uniqueCacheIdentifier);
91+
$batch->scan();
11392

114-
/**
115-
* Scans everything collected since beginBatchedScan() at once and fills in
116-
* the locators handed out in the meantime.
117-
*/
118-
public function flushBatchedScan(): void
119-
{
120-
if ($this->nestedBatchedScans > 0) {
121-
$this->nestedBatchedScans--;
122-
return;
123-
}
124-
125-
$batched = $this->batchedScan;
126-
$this->batchedScan = null;
127-
if ($batched === null || $batched === []) {
128-
return;
129-
}
130-
131-
$this->scan($batched);
93+
return $locator;
13294
}
13395

13496
/**
135-
* @param non-empty-string $cacheKey
136-
* @param string[] $files
97+
* For creating several locators and scanning them in one go.
13798
*/
138-
private function createLocator(string $cacheKey, array $files): OptimizedDirectorySourceLocator
99+
public function createBatch(): OptimizedDirectorySourceLocatorBatch
139100
{
140-
$locator = new OptimizedDirectorySourceLocator(
141-
$this->fileNodesFetcher,
142-
$this->cache,
143-
$this->phpVersion,
144-
$this->fileContentHasher,
145-
[],
146-
[],
147-
[],
148-
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+
$this->scan(...),
149114
);
150-
151-
if ($this->batchedScan !== null) {
152-
$this->batchedScan[] = [$cacheKey, $files, $locator];
153-
} else {
154-
$this->scan([[$cacheKey, $files, $locator]]);
155-
}
156-
157-
return $locator;
158115
}
159116

160117
/**
@@ -173,18 +130,20 @@ private function scan(array $requests): void
173130
$cachedEntries = [];
174131
foreach ($requests as $i => [$cacheKey]) {
175132
$cached = $this->loadCachedSymbols($cacheKey, $variableCacheKey);
176-
if ($cached === null) {
177-
// On a cold cache every parallel worker that is not forked from a process that did
178-
// the scan already builds the same locators at once, and would scan the same files.
179-
// The first worker to take the lock scans and saves; the rest block until it
180-
// releases, then read the cache it wrote.
133+
// On a cold cache every parallel worker that is not forked from a process that did
134+
// the scan already builds the same locators at once, and would scan the same files.
135+
// The first worker to take the lock scans and saves; the rest block until it
136+
// releases, then read the cache it wrote. The locators of a batch can share a cache
137+
// key (odsl-installed-files of two Composer projects), and a second lock of the same
138+
// file would wait for this very process.
139+
if ($cached === null && !array_key_exists($cacheKey, $scanLocks)) {
181140
$scanLock = $this->acquireDirectoryScanLock($cacheKey . $variableCacheKey);
182141
if ($scanLock !== null) {
183142
$cached = $this->loadCachedSymbols($cacheKey, $variableCacheKey);
184143
if ($cached !== null) {
185144
$this->releaseDirectoryScanLock($scanLock);
186145
} else {
187-
$scanLocks[] = $scanLock;
146+
$scanLocks[$cacheKey] = $scanLock;
188147
}
189148
}
190149
}
@@ -246,7 +205,7 @@ private function scan(array $requests): void
246205
}
247206

248207
[$classToFile, $functionToFiles, $constantToFile] = $this->changeStructure($entry);
249-
$locator->fillBatchedScan($classToFile, $functionToFiles, $constantToFile);
208+
$locator->fillScanned($classToFile, $functionToFiles, $constantToFile);
250209
}
251210
} finally {
252211
// 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)