From b487f05b8280af9759bc3bd4dcc9ee7d1e8b4f86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Bok?= Date: Sun, 14 May 2023 07:52:41 +0200 Subject: [PATCH 1/3] [cache] Cache unchanged files on --dry-run --- .../ValueObject/ProcessFileResult.php | 33 +++++++++++++++++++ packages/Parallel/WorkerRunner.php | 13 +++++--- 2 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 packages/Parallel/ValueObject/ProcessFileResult.php diff --git a/packages/Parallel/ValueObject/ProcessFileResult.php b/packages/Parallel/ValueObject/ProcessFileResult.php new file mode 100644 index 00000000000..6a9ba97a2b0 --- /dev/null +++ b/packages/Parallel/ValueObject/ProcessFileResult.php @@ -0,0 +1,33 @@ +isFileChanged; + } + + /** + * @return array{system_errors: SystemError[], file_diffs: FileDiff[]} + */ + public function getErrorAndFileDiffs(): array + { + return $this->errorAndFileDiffs; + } +} diff --git a/packages/Parallel/WorkerRunner.php b/packages/Parallel/WorkerRunner.php index 2f9a7a97ddd..21838729aac 100644 --- a/packages/Parallel/WorkerRunner.php +++ b/packages/Parallel/WorkerRunner.php @@ -20,6 +20,7 @@ use Rector\Core\ValueObject\Error\SystemError; use Rector\Core\ValueObject\Reporting\FileDiff; use Rector\Parallel\ValueObject\Bridge; +use Rector\Parallel\ValueObject\ProcessFileResult; use Symplify\EasyParallel\Enum\Action; use Symplify\EasyParallel\Enum\ReactCommand; use Symplify\EasyParallel\Enum\ReactEvent; @@ -93,11 +94,12 @@ public function run(Encoder $encoder, Decoder $decoder, Configuration $configura $file = new File($filePath, FileSystem::read($filePath)); $this->currentFileProvider->setFile($file); - $errorAndFileDiffs = $this->processFile($file, $configuration, $errorAndFileDiffs); + $processFileResult = $this->processFile($file, $configuration, $errorAndFileDiffs); + $errorAndFileDiffs = $processFileResult->getErrorAndFileDiffs(); if ($errorAndFileDiffs[Bridge::SYSTEM_ERRORS] !== []) { $this->invalidateFile($file); - } elseif (! $configuration->isDryRun()) { + } elseif (! $configuration->isDryRun() || $processFileResult->isFileChanged()) { $this->changedFilesDetector->cacheFileWithDependencies($file->getFilePath()); } } catch (Throwable $throwable) { @@ -129,23 +131,24 @@ public function run(Encoder $encoder, Decoder $decoder, Configuration $configura /** * @param array{system_errors: SystemError[], file_diffs: FileDiff[]}|mixed[] $errorAndFileDiffs - * @return array{system_errors: SystemError[], file_diffs: FileDiff[]} */ - private function processFile(File $file, Configuration $configuration, array $errorAndFileDiffs): array + private function processFile(File $file, Configuration $configuration, array $errorAndFileDiffs): ProcessFileResult { + $wasFileChanged = false; foreach ($this->fileProcessors as $fileProcessor) { if (! $fileProcessor->supports($file, $configuration)) { continue; } $currentErrorsAndFileDiffs = $fileProcessor->process($file, $configuration); + $wasFileChanged = $wasFileChanged || $currentErrorsAndFileDiffs[Bridge::FILE_DIFFS] !== []; $errorAndFileDiffs = $this->arrayParametersMerger->merge( $errorAndFileDiffs, $currentErrorsAndFileDiffs ); } - return $errorAndFileDiffs; + return new ProcessFileResult($wasFileChanged, $errorAndFileDiffs); } /** From 60b85e9b1f28745693e02af1a322c687f2d60541 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Bok?= Date: Sun, 14 May 2023 10:48:40 +0200 Subject: [PATCH 2/3] [cache] Simplify implementation --- .../ValueObject/ProcessFileResult.php | 33 ------------------- packages/Parallel/WorkerRunner.php | 13 +++----- 2 files changed, 5 insertions(+), 41 deletions(-) delete mode 100644 packages/Parallel/ValueObject/ProcessFileResult.php diff --git a/packages/Parallel/ValueObject/ProcessFileResult.php b/packages/Parallel/ValueObject/ProcessFileResult.php deleted file mode 100644 index 6a9ba97a2b0..00000000000 --- a/packages/Parallel/ValueObject/ProcessFileResult.php +++ /dev/null @@ -1,33 +0,0 @@ -isFileChanged; - } - - /** - * @return array{system_errors: SystemError[], file_diffs: FileDiff[]} - */ - public function getErrorAndFileDiffs(): array - { - return $this->errorAndFileDiffs; - } -} diff --git a/packages/Parallel/WorkerRunner.php b/packages/Parallel/WorkerRunner.php index 21838729aac..4359229d9f7 100644 --- a/packages/Parallel/WorkerRunner.php +++ b/packages/Parallel/WorkerRunner.php @@ -20,7 +20,6 @@ use Rector\Core\ValueObject\Error\SystemError; use Rector\Core\ValueObject\Reporting\FileDiff; use Rector\Parallel\ValueObject\Bridge; -use Rector\Parallel\ValueObject\ProcessFileResult; use Symplify\EasyParallel\Enum\Action; use Symplify\EasyParallel\Enum\ReactCommand; use Symplify\EasyParallel\Enum\ReactEvent; @@ -94,12 +93,11 @@ public function run(Encoder $encoder, Decoder $decoder, Configuration $configura $file = new File($filePath, FileSystem::read($filePath)); $this->currentFileProvider->setFile($file); - $processFileResult = $this->processFile($file, $configuration, $errorAndFileDiffs); - $errorAndFileDiffs = $processFileResult->getErrorAndFileDiffs(); + $errorAndFileDiffs = $this->processFile($file, $configuration, $errorAndFileDiffs); if ($errorAndFileDiffs[Bridge::SYSTEM_ERRORS] !== []) { $this->invalidateFile($file); - } elseif (! $configuration->isDryRun() || $processFileResult->isFileChanged()) { + } else { $this->changedFilesDetector->cacheFileWithDependencies($file->getFilePath()); } } catch (Throwable $throwable) { @@ -131,24 +129,23 @@ public function run(Encoder $encoder, Decoder $decoder, Configuration $configura /** * @param array{system_errors: SystemError[], file_diffs: FileDiff[]}|mixed[] $errorAndFileDiffs + * @return array{system_errors: SystemError[], file_diffs: FileDiff[]} */ - private function processFile(File $file, Configuration $configuration, array $errorAndFileDiffs): ProcessFileResult + private function processFile(File $file, Configuration $configuration, array $errorAndFileDiffs): array { - $wasFileChanged = false; foreach ($this->fileProcessors as $fileProcessor) { if (! $fileProcessor->supports($file, $configuration)) { continue; } $currentErrorsAndFileDiffs = $fileProcessor->process($file, $configuration); - $wasFileChanged = $wasFileChanged || $currentErrorsAndFileDiffs[Bridge::FILE_DIFFS] !== []; $errorAndFileDiffs = $this->arrayParametersMerger->merge( $errorAndFileDiffs, $currentErrorsAndFileDiffs ); } - return new ProcessFileResult($wasFileChanged, $errorAndFileDiffs); + return $errorAndFileDiffs; } /** From 7d71fe54b26821d6133bdf4788ad1f733746c499 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Bok?= Date: Sun, 14 May 2023 21:27:35 +0200 Subject: [PATCH 3/3] [cache] Clear files cached by worker process request if that request has timed out --- .../Application/ParallelFileProcessor.php | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/packages/Parallel/Application/ParallelFileProcessor.php b/packages/Parallel/Application/ParallelFileProcessor.php index 5f50e9a8754..d6681e23846 100644 --- a/packages/Parallel/Application/ParallelFileProcessor.php +++ b/packages/Parallel/Application/ParallelFileProcessor.php @@ -10,6 +10,7 @@ use React\EventLoop\StreamSelectLoop; use React\Socket\ConnectionInterface; use React\Socket\TcpServer; +use Rector\Caching\Detector\ChangedFilesDetector; use Rector\Core\Configuration\Option; use Rector\Core\Configuration\Parameter\ParameterProvider; use Rector\Core\Console\Command\ProcessCommand; @@ -24,6 +25,7 @@ use Symplify\EasyParallel\Enum\Content; use Symplify\EasyParallel\Enum\ReactCommand; use Symplify\EasyParallel\Enum\ReactEvent; +use Symplify\EasyParallel\ValueObject\ChildProcessTimeoutException; use Symplify\EasyParallel\ValueObject\ParallelProcess; use Symplify\EasyParallel\ValueObject\ProcessPool; use Symplify\EasyParallel\ValueObject\Schedule; @@ -46,7 +48,8 @@ final class ParallelFileProcessor public function __construct( private readonly WorkerCommandLineFactory $workerCommandLineFactory, - private readonly ParameterProvider $parameterProvider + private readonly ParameterProvider $parameterProvider, + private readonly ChangedFilesDetector $changedFilesDetector, ) { } @@ -125,6 +128,18 @@ public function process( ++$systemErrorsCount; $reachedSystemErrorsCountLimit = true; $this->processPool->quitAll(); + + // This sleep has to be here, because event though we have called $this->processPool->quitAll(), + // it takes some time for the child processes to actually die, and if we would delete the offending cache + // files right away, they could still write them "back" before they die + sleep(1); + if ($throwable instanceof ChildProcessTimeoutException) { + $context = $throwable->getContext(); + + foreach ($context[Bridge::FILES] as $file) { + $this->changedFilesDetector->invalidateFile($file); + } + } }; $timeoutInSeconds = $this->parameterProvider->provideIntParameter(Option::PARALLEL_JOB_TIMEOUT_IN_SECONDS);