PHPStan Pro: analyse the files changed while an analysis runs once it ends

Files were not watched during an analysis, so an edit made while it ran
was noticed only after it ended, by the next poll.

They are watched during an analysis now, and the code shown follows them
right away. Once they settle, the analysis in progress is left to finish
- what it saves to the result cache makes the next one cheaper - and one
more starts when it ends, for everything that changed meanwhile. One
worker at a time writes the result cache.

Stopping the analysis in progress instead was tried, on the Laravel
framework with edits overlapping analyses: about the same time until the
final result, but 35-40 % more CPU, because each stopped analysis is
thrown away. Only while the API of a widely used class kept changing was
stopping faster, by 3 s of 14 s. Cancelling the worker never stopped it
anyway - terminating the process terminated the shell running its
command - and is removed.

Amp-Thread-ID: https://ampcode.com/threads/T-01a1a533-a2c5-7581-b58e-fb459c136c67

Author: ondrejmirtes

src/Command/FixerApplication.php

@@ -24,7 +24,6 @@
 use PHPStan\Internal\DirectoryCreatorException;
 use PHPStan\Internal\HttpClientFactory;
 use PHPStan\PhpDoc\StubFilesProvider;
-use PHPStan\Process\ProcessCanceledException;
 use PHPStan\Process\ProcessCrashedException;
 use PHPStan\Process\ProcessHelper;
 use PHPStan\Process\SpawnedProcessPromise;
@@ -73,6 +72,9 @@ final class FixerApplication
 	/** @var PromiseInterface<string>|null  */
 	private PromiseInterface|null $processInProgress = null;
 
+	/** Files changed during the analysis in progress, another one starts once it ends. */
+	private bool $analysisRequested = false;
+
 	private bool $fileMonitorActive = true;
 
 	/** @var string[] watched files besides the analysed ones and the config files */
@@ -188,9 +190,11 @@ public function run(
 					'paths' => $changes->getChangedFiles(),
 				]]);
 			}, function () use ($loop, $mainScript, $projectConfigFile, $input, $encoder): void {
+				// The analysis in progress is not stopped: what it saves to the result cache makes the next
+				// one cheaper, and that one starts once it ends, for everything that changed meanwhile.
 				if ($this->processInProgress !== null) {
-					$this->processInProgress->cancel();
-					$this->processInProgress = null;
+					$this->analysisRequested = true;
+					return;
 				}
 
 				$this->analyse(
@@ -424,10 +428,6 @@ private function monitorFileChanges(LoopInterface $loop, callable $hasChangesCal
 				$loop->addTimer($interval, $callback);
 				return;
 			}
-			if ($this->processInProgress !== null) {
-				$loop->addTimer($interval, $callback);
-				return;
-			}
 			$changes = $fileMonitor->getChanges();
 			$now = microtime(true);
 
@@ -506,17 +506,11 @@ private function analyse(
 		));
 		$this->processInProgress = $process->run();
 
-		$this->processInProgress->then(function () use ($server): void {
-			$this->processInProgress = null;
+		$this->processInProgress->then(static function () use ($server): void {
 			$server->close();
-		}, function (Throwable $e) use ($server, $phpstanFixerEncoder): void {
-			$this->processInProgress = null;
+		}, static function (Throwable $e) use ($server, $phpstanFixerEncoder): void {
 			$server->close();
 
-			if ($e instanceof ProcessCanceledException) {
-				return;
-			}
-
 			if ($e instanceof ProcessCrashedException) {
 				$message = 'Analysis crashed';
 				$traceAsString = $e->getMessage();
@@ -535,6 +529,14 @@ private function analyse(
 					shouldReportBug: false,
 				)],
 			]]);
+		})->finally(function () use ($loop, $mainScript, $projectConfigFile, $input, $phpstanFixerEncoder): void {
+			$this->processInProgress = null;
+			if (!$this->analysisRequested) {
+				return;
+			}
+
+			$this->analysisRequested = false;
+			$this->analyse($loop, $mainScript, $projectConfigFile, $input, $phpstanFixerEncoder);
 		});
 	}
 

src/Process/ProcessCanceledException.php

@@ -14,24 +14,17 @@
 
 /**
  * A freshly spawned PHP process (react/child-process) as a promise that resolves
- * with its output on success and rejects with ProcessCrashedException or
- * ProcessCanceledException otherwise.
+ * with its output on success and rejects with ProcessCrashedException otherwise.
  */
 final class SpawnedProcessPromise
 {
 
 	/** @var Deferred<string> */
 	private Deferred $deferred;
 
-	private ?Process $process = null;
-
-	private bool $canceled = false;
-
 	public function __construct(private LoopInterface $loop, private string $command)
 	{
-		$this->deferred = new Deferred(function (): void {
-			$this->cancel();
-		});
+		$this->deferred = new Deferred();
 	}
 
 	/**
@@ -48,18 +41,13 @@ public function run(): PromiseInterface
 			throw new ShouldNotHappenException('Failed creating temp file for stderr.');
 		}
 
-		$this->process = new Process($this->command, fds: [
+		$process = new Process($this->command, fds: [
 			1 => $tmpStdOutResource,
 			2 => $tmpStdErrResource,
 		]);
-		$this->process->start($this->loop);
+		$process->start($this->loop);
 
-		$this->process->on('exit', function ($exitCode) use ($tmpStdOutResource, $tmpStdErrResource): void {
-			if ($this->canceled) {
-				fclose($tmpStdOutResource);
-				fclose($tmpStdErrResource);
-				return;
-			}
+		$process->on('exit', function ($exitCode) use ($tmpStdOutResource, $tmpStdErrResource): void {
 			rewind($tmpStdOutResource);
 			$stdOut = stream_get_contents($tmpStdOutResource);
 			fclose($tmpStdOutResource);
@@ -84,14 +72,4 @@ public function run(): PromiseInterface
 		return $this->deferred->promise();
 	}
 
-	private function cancel(): void
-	{
-		if ($this->process === null) {
-			throw new ShouldNotHappenException('Cancelling process before running');
-		}
-		$this->canceled = true;
-		$this->process->terminate();
-		$this->deferred->reject(new ProcessCanceledException());
-	}
-
 }