Skip to content

Commit

Permalink
Refactoring and cosmetic changes
Browse files Browse the repository at this point in the history
* syntax sugar
* sequential option - no shortcut, as we do not use shortcut for other any other option
* explicitly mark `ProcessFactory::getCommandArgs` as `@private`, to indicate it shall not be called from outside of this class, ref #7777 (comment)
* `WorkerCommand` options written as single-liners, so unified as all other commands and easier to copy-paste
* add explicit comment for hardcoded option
* remove unused var
* rename `ParallelRunnerConfigInterface` to `ParallelAwareConfigInterface`
* make `ParallelConfig` only DTO and externalize factory methods to dedicated class
* `Runner` - don't reevaluate `maxFixerPerProcess` in each loop
* explicitly call generator, avoid simple 'job' as too similar to job-id
* rename variables related to file chunks
  • Loading branch information
keradus committed May 7, 2024
1 parent dc15acf commit 762c5da
Show file tree
Hide file tree
Showing 18 changed files with 159 additions and 116 deletions.
4 changes: 2 additions & 2 deletions .php-cs-fixer.dist.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@

use PhpCsFixer\Config;
use PhpCsFixer\Finder;
use PhpCsFixer\Runner\Parallel\ParallelConfig;
use PhpCsFixer\Runner\Parallel\ParallelConfigFactory;

return (new Config())
->setParallelConfig(ParallelConfig::detect())
->setParallelConfig(ParallelConfigFactory::detect())
->setRiskyAllowed(true)
->setRules([
'@PHP74Migration' => true,
Expand Down
4 changes: 2 additions & 2 deletions doc/usage.rst
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ If you do not have config file, you can run following command to fix non-hidden,
php php-cs-fixer.phar fix .
You can also fix files in parallel, utilising more CPU cores. You can do this by using config class that implements ``PhpCsFixer\Runner\Parallel\ParallelConfig\ParallelRunnerConfigInterface``, and use ``setParallelConfig()`` method. Recommended way is to utilise auto-detecting parallel configuration:
You can also fix files in parallel, utilising more CPU cores. You can do this by using config class that implements ``PhpCsFixer\Runner\Parallel\ParallelConfig\ParallelAwareConfigInterface``, and use ``setParallelConfig()`` method. Recommended way is to utilise auto-detecting parallel configuration:

.. code-block:: php
<?php
return (new PhpCsFixer\Config())
->setParallelConfig(ParallelConfig::detect())
->setParallelConfig(ParallelConfigFactory::detect())
;
However, in some case you may want to fine-tune parallelisation with explicit values (e.g. in environments where auto-detection does not work properly and suggests more cores than it should):
Expand Down
11 changes: 7 additions & 4 deletions src/Config.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@

use PhpCsFixer\Fixer\FixerInterface;
use PhpCsFixer\Runner\Parallel\ParallelConfig;
use PhpCsFixer\Runner\Parallel\ParallelConfigFactory;

/**
* @author Fabien Potencier <fabien@symfony.com>
* @author Katsuhiro Ogawa <ko.fivestar@gmail.com>
* @author Dariusz Rumiński <dariusz.ruminski@gmail.com>
*/
class Config implements ConfigInterface, ParallelRunnerConfigInterface
class Config implements ConfigInterface, ParallelAwareConfigInterface
{
private string $cacheFile = '.php-cs-fixer.cache';

Expand All @@ -48,7 +49,7 @@ class Config implements ConfigInterface, ParallelRunnerConfigInterface

private string $name;

private ?ParallelConfig $parallelRunnerConfig;
private ?ParallelConfig $parallelConfig;

/**
* @var null|string
Expand Down Expand Up @@ -123,7 +124,9 @@ public function getName(): string

public function getParallelConfig(): ParallelConfig
{
return $this->parallelRunnerConfig ?? ParallelConfig::sequential();
$this->parallelConfig ??= ParallelConfigFactory::sequential();

return $this->parallelConfig;
}

public function getPhpExecutable(): ?string
Expand Down Expand Up @@ -199,7 +202,7 @@ public function setLineEnding(string $lineEnding): ConfigInterface

public function setParallelConfig(ParallelConfig $config): ConfigInterface
{
$this->parallelRunnerConfig = $config;
$this->parallelConfig = $config;

return $this;
}
Expand Down
2 changes: 1 addition & 1 deletion src/Console/Command/FixCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ protected function configure(): void
new InputOption('format', '', InputOption::VALUE_REQUIRED, 'To output results in other formats.'),
new InputOption('stop-on-violation', '', InputOption::VALUE_NONE, 'Stop execution on first violation.'),
new InputOption('show-progress', '', InputOption::VALUE_REQUIRED, 'Type of progress indicator (none, dots).'),
new InputOption('sequential', 's', InputOption::VALUE_NONE, 'Enforce sequential analysis.'),
new InputOption('sequential', '', InputOption::VALUE_NONE, 'Enforce sequential analysis.'),
]
);
}
Expand Down
50 changes: 10 additions & 40 deletions src/Console/Command/WorkerCommand.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
use PhpCsFixer\Error\ErrorsManager;
use PhpCsFixer\FixerFileProcessedEvent;
use PhpCsFixer\Runner\Parallel\ParallelAction;
use PhpCsFixer\Runner\Parallel\ParallelConfig;
use PhpCsFixer\Runner\Parallel\ParallelConfigFactory;
use PhpCsFixer\Runner\Parallel\ParallelisationException;
use PhpCsFixer\Runner\Parallel\ReadonlyCacheManager;
use PhpCsFixer\Runner\Runner;
Expand Down Expand Up @@ -77,43 +77,13 @@ protected function configure(): void
{
$this->setDefinition(
[
new InputOption(
'port',
null,
InputOption::VALUE_REQUIRED,
'Specifies parallelisation server\'s port.'
),
new InputOption(
'identifier',
null,
InputOption::VALUE_REQUIRED,
'Specifies parallelisation process\' identifier.'
),
new InputOption(
'allow-risky',
'',
InputOption::VALUE_REQUIRED,
'Are risky fixers allowed (can be `yes` or `no`).'
),
new InputOption('port', null, InputOption::VALUE_REQUIRED, 'Specifies parallelisation server\'s port.'),
new InputOption('identifier', null, InputOption::VALUE_REQUIRED, 'Specifies parallelisation process\' identifier.'),
new InputOption('allow-risky', '', InputOption::VALUE_REQUIRED, 'Are risky fixers allowed (can be `yes` or `no`).'),
new InputOption('config', '', InputOption::VALUE_REQUIRED, 'The path to a config file.'),
new InputOption(
'dry-run',
'',
InputOption::VALUE_NONE,
'Only shows which files would have been modified.'
),
new InputOption(
'rules',
'',
InputOption::VALUE_REQUIRED,
'List of rules that should be run against configured paths.'
),
new InputOption(
'using-cache',
'',
InputOption::VALUE_REQUIRED,
'Should cache be used (can be `yes` or `no`).'
),
new InputOption('dry-run', '', InputOption::VALUE_NONE, 'Only shows which files would have been modified.'),
new InputOption('rules', '', InputOption::VALUE_REQUIRED, 'List of rules that should be run against configured paths.'),
new InputOption('using-cache', '', InputOption::VALUE_REQUIRED, 'Should cache be used (can be `yes` or `no`).'),
new InputOption('cache-file', '', InputOption::VALUE_REQUIRED, 'The path to the cache file.'),
new InputOption('diff', '', InputOption::VALUE_NONE, 'Prints diff for each file.'),
]
Expand Down Expand Up @@ -182,7 +152,7 @@ function (ConnectionInterface $connection) use ($loop, $runner, $identifier): vo
/** @var iterable<int, string> $files */
$files = $json['files'];

foreach ($files as $i => $absolutePath) {
foreach ($files as $absolutePath) {
$relativePath = $this->configurationResolver->getDirectory()->getRelativePathTo($absolutePath);

// Reset events because we want to collect only those coming from analysed files chunk
Expand Down Expand Up @@ -237,7 +207,7 @@ private function createRunner(InputInterface $input): Runner
'dry-run' => $input->getOption('dry-run'),
'rules' => $passedRules,
'path' => [],
'path-mode' => ConfigurationResolver::PATH_MODE_OVERRIDE,
'path-mode' => ConfigurationResolver::PATH_MODE_OVERRIDE, // IMPORTANT! WorkerCommand is called with file that already passed filtering, so here we can rely on PATH_MODE_OVERRIDE.
'using-cache' => $input->getOption('using-cache'),
'cache-file' => $input->getOption('cache-file'),
'diff' => $input->getOption('diff'),
Expand All @@ -258,7 +228,7 @@ private function createRunner(InputInterface $input): Runner
new ReadonlyCacheManager($this->configurationResolver->getCacheManager()),
$this->configurationResolver->getDirectory(),
$this->configurationResolver->shouldStopOnViolation(),
ParallelConfig::sequential(), // IMPORTANT! Worker must run in sequential mode
ParallelConfigFactory::sequential(), // IMPORTANT! Worker must run in sequential mode.
null,
$this->configurationResolver->getConfigFile()
);
Expand Down
7 changes: 4 additions & 3 deletions src/Console/ConfigurationResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,11 @@
use PhpCsFixer\FixerFactory;
use PhpCsFixer\Linter\Linter;
use PhpCsFixer\Linter\LinterInterface;
use PhpCsFixer\ParallelRunnerConfigInterface;
use PhpCsFixer\ParallelAwareConfigInterface;
use PhpCsFixer\RuleSet\RuleSet;
use PhpCsFixer\RuleSet\RuleSetInterface;
use PhpCsFixer\Runner\Parallel\ParallelConfig;
use PhpCsFixer\Runner\Parallel\ParallelConfigFactory;
use PhpCsFixer\StdinFileInfo;
use PhpCsFixer\ToolInfoInterface;
use PhpCsFixer\Utils;
Expand Down Expand Up @@ -281,9 +282,9 @@ public function getParallelConfig(): ParallelConfig
{
$config = $this->getConfig();

return true !== $this->options['sequential'] && $config instanceof ParallelRunnerConfigInterface
return true !== $this->options['sequential'] && $config instanceof ParallelAwareConfigInterface
? $config->getParallelConfig()
: ParallelConfig::sequential();
: ParallelConfigFactory::sequential();
}

public function getConfigFile(): ?string
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
/**
* @author Greg Korba <greg@codito.dev>
*
* @TODO 4.0 Include parallel runner config in main config
* @TODO 4.0 Include parallel runner config in main ConfigInterface
*/
interface ParallelRunnerConfigInterface extends ConfigInterface
interface ParallelAwareConfigInterface extends ConfigInterface
{
public function getParallelConfig(): ParallelConfig;

Expand Down
25 changes: 0 additions & 25 deletions src/Runner/Parallel/ParallelConfig.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@

namespace PhpCsFixer\Runner\Parallel;

use Fidry\CpuCoreCounter\CpuCoreCounter;
use Fidry\CpuCoreCounter\Finder\DummyCpuCoreFinder;
use Fidry\CpuCoreCounter\Finder\FinderRegistry;

/**
* @author Greg Korba <greg@codito.dev>
*/
Expand Down Expand Up @@ -63,25 +59,4 @@ public function getProcessTimeout(): int
{
return $this->processTimeout;
}

public static function sequential(): self
{
return new self(1);
}

/**
* @param positive-int $filesPerProcess
* @param positive-int $processTimeout
*/
public static function detect(
int $filesPerProcess = self::DEFAULT_FILES_PER_PROCESS,
int $processTimeout = self::DEFAULT_PROCESS_TIMEOUT
): self {
$counter = new CpuCoreCounter([
...FinderRegistry::getDefaultLogicalFinders(),
new DummyCpuCoreFinder(1),
]);

return new self($counter->getCount(), $filesPerProcess, $processTimeout);
}
}
53 changes: 53 additions & 0 deletions src/Runner/Parallel/ParallelConfigFactory.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
<?php

declare(strict_types=1);

/*
* This file is part of PHP CS Fixer.
*
* (c) Fabien Potencier <fabien@symfony.com>
* Dariusz Rumiński <dariusz.ruminski@gmail.com>
*
* This source file is subject to the MIT license that is bundled
* with this source code in the file LICENSE.
*/

namespace PhpCsFixer\Runner\Parallel;

use Fidry\CpuCoreCounter\CpuCoreCounter;
use Fidry\CpuCoreCounter\Finder\DummyCpuCoreFinder;
use Fidry\CpuCoreCounter\Finder\FinderRegistry;

/**
* @author Dariusz Rumiński <dariusz.ruminski@gmail.com>
*/
final class ParallelConfigFactory
{
private function __construct() {}

public static function sequential(): ParallelConfig
{
return new ParallelConfig(1);
}

/**
* @param null|positive-int $filesPerProcess
* @param null|positive-int $processTimeout
*/
public static function detect(
?int $filesPerProcess = null,
?int $processTimeout = null
): ParallelConfig {
$counter = new CpuCoreCounter([
...FinderRegistry::getDefaultLogicalFinders(),
new DummyCpuCoreFinder(1),
]);

return new ParallelConfig(
...array_filter(
[$counter->getCount(), $filesPerProcess, $processTimeout],
static fn ($value): bool => null !== $value
)
);
}
}
2 changes: 2 additions & 0 deletions src/Runner/Parallel/ProcessFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ public function create(
}

/**
* @private
*
* @return list<string>
*/
public function getCommandArgs(int $serverPort, ProcessIdentifier $identifier, RunnerConfig $runnerConfig): array
Expand Down

0 comments on commit 762c5da

Please sign in to comment.