Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parallel execution - minor changes #5

Merged
merged 25 commits into from
May 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
4f70787
minor: sugar syntax
keradus May 5, 2024
7f099c5
minor: sequential option - no shortcut, as we do not use shortcut for…
keradus May 5, 2024
1bb6fa9
minor: explicitly mark ProcessFactory::getCommandArgs as minor, to in…
keradus May 5, 2024
c8982bf
minor: WorkerCommand options written as single-liners, so unified as …
keradus May 5, 2024
1cdba6d
minor: add explicit comment for hardcoded option
keradus May 5, 2024
d85cd53
minor: remove unused var
keradus May 5, 2024
891df50
minor: ErrorsManager - no need for forPath, but we need getAll instead
keradus May 5, 2024
cd2adad
minor - fix allergy for type casting
keradus May 5, 2024
7f600b4
minor - ParallelRunnerConfigInterface - rename
keradus May 5, 2024
1e52202
minor: make ParallelConfig only DTO and externalize factory methods t…
keradus May 5, 2024
2ccc642
minor: Runner - don't reevaluate maxFixerPerProcess in each loop
keradus May 5, 2024
3444af8
minor - explicitly call generator, avoid simple 'job' as too similar …
keradus May 5, 2024
569f43a
minor: make ParallelConfig only DTO and externalize factory methods t…
keradus May 5, 2024
41e24e3
minor: adjust test
keradus May 5, 2024
8eb7892
minor - explicitly call generator, avoid simple 'job' as too similar …
keradus May 5, 2024
121ddd1
minor: fix tests
keradus May 5, 2024
315f603
test: don't count comment after class as another classy element
keradus May 5, 2024
32fc3fe
dx: allow to enfore cache mechanism by env var
keradus May 5, 2024
c3d52ba
re-use hasher
keradus May 5, 2024
1a7905c
minor: extra comment
keradus May 5, 2024
56e4a70
rename filesChunkGenerator
keradus May 7, 2024
f45cdee
rename fileChunkForJob
keradus May 7, 2024
31acd48
Update src/Runner/Parallel/ParallelConfigFactory.php
keradus May 7, 2024
0220bf7
Revert "minor: ErrorsManager - no need for forPath, but we need getAl…
Wirone May 7, 2024
11fe596
Revert "minor - fix allergy for type casting"
Wirone May 7, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
4 changes: 3 additions & 1 deletion src/Cache/FileCacheManager.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@

namespace PhpCsFixer\Cache;

use PhpCsFixer\Tokenizer\CodeHasher;

/**
* Class supports caching information about state of fixing files.
*
Expand Down Expand Up @@ -136,6 +138,6 @@ private function writeCache(): void

private function calcHash(string $content): string
{
return md5($content);
return CodeHasher::calculateCodeHash($content);
}
}
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 <[email protected]>
* @author Katsuhiro Ogawa <[email protected]>
* @author Dariusz Rumiński <[email protected]>
*/
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 @@ -125,7 +126,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 @@ -201,7 +204,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 @@
{
$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 @@
/** @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 @@ -231,13 +201,13 @@

$this->configurationResolver = new ConfigurationResolver(
new Config(),
[

Check warning on line 204 in src/Console/Command/WorkerCommand.php

View workflow job for this annotation

GitHub Actions / PHP 8.3 mutation tests

Escaped Mutant for Mutator "ArrayItemRemoval": --- Original +++ New @@ @@ $this->configurationResolver = new ConfigurationResolver( new Config(), [ - 'allow-risky' => $input->getOption('allow-risky'), 'config' => $passedConfig, 'dry-run' => $input->getOption('dry-run'), 'rules' => $passedRules,
'allow-risky' => $input->getOption('allow-risky'),
'config' => $passedConfig,
'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 @@
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
10 changes: 6 additions & 4 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 @@
{
$config = $this->getConfig();

return true !== $this->options['sequential'] && $config instanceof ParallelRunnerConfigInterface
return true !== $this->options['sequential'] && $config instanceof ParallelAwareConfigInterface

Check warning on line 285 in src/Console/ConfigurationResolver.php

View workflow job for this annotation

GitHub Actions / PHP 8.3 mutation tests

Escaped Mutant for Mutator "InstanceOf_": --- Original +++ New @@ @@ public function getParallelConfig() : ParallelConfig { $config = $this->getConfig(); - return true !== $this->options['sequential'] && $config instanceof ParallelAwareConfigInterface ? $config->getParallelConfig() : ParallelConfigFactory::sequential(); + return true !== $this->options['sequential'] && true ? $config->getParallelConfig() : ParallelConfigFactory::sequential(); } public function getConfigFile() : ?string {
? $config->getParallelConfig()
: ParallelConfig::sequential();
: ParallelConfigFactory::sequential();
}

public function getConfigFile(): ?string
Expand Down Expand Up @@ -966,6 +967,7 @@
{
return $this->toolInfo->isInstalledAsPhar()
|| $this->toolInfo->isInstalledByComposer()
|| $this->toolInfo->isRunInsideDocker();
|| $this->toolInfo->isRunInsideDocker()
|| filter_var(getenv('PHP_CS_FIXER_ENFORCE_CACHE'), FILTER_VALIDATE_BOOL);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@
/**
* @author Greg Korba <[email protected]>
*
* @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
Wirone marked this conversation as resolved.
Show resolved Hide resolved
{
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 <[email protected]>
*/
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
Wirone marked this conversation as resolved.
Show resolved Hide resolved
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 <[email protected]>
* Dariusz Rumiński <[email protected]>
*
* 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 <[email protected]>
*/
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([

Check warning on line 41 in src/Runner/Parallel/ParallelConfigFactory.php

View workflow job for this annotation

GitHub Actions / PHP 8.3 mutation tests

Escaped Mutant for Mutator "ArrayItemRemoval": --- Original +++ New @@ @@ */ public static function detect(?int $filesPerProcess = null, ?int $processTimeout = null) : ParallelConfig { - $counter = new CpuCoreCounter([...FinderRegistry::getDefaultLogicalFinders(), new DummyCpuCoreFinder(1)]); + $counter = new CpuCoreCounter([new DummyCpuCoreFinder(1)]); return new ParallelConfig(...array_filter([$counter->getCount(), $filesPerProcess, $processTimeout], static fn($value): bool => null !== $value)); } }
...FinderRegistry::getDefaultLogicalFinders(),

Check warning on line 42 in src/Runner/Parallel/ParallelConfigFactory.php

View workflow job for this annotation

GitHub Actions / PHP 8.3 mutation tests

Escaped Mutant for Mutator "SpreadOneItem": --- Original +++ New @@ @@ */ public static function detect(?int $filesPerProcess = null, ?int $processTimeout = null) : ParallelConfig { - $counter = new CpuCoreCounter([...FinderRegistry::getDefaultLogicalFinders(), new DummyCpuCoreFinder(1)]); + $counter = new CpuCoreCounter([[...FinderRegistry::getDefaultLogicalFinders()][0], new DummyCpuCoreFinder(1)]); return new ParallelConfig(...array_filter([$counter->getCount(), $filesPerProcess, $processTimeout], static fn($value): bool => null !== $value)); } }
new DummyCpuCoreFinder(1),

Check warning on line 43 in src/Runner/Parallel/ParallelConfigFactory.php

View workflow job for this annotation

GitHub Actions / PHP 8.3 mutation tests

Escaped Mutant for Mutator "DecrementInteger": --- Original +++ New @@ @@ */ public static function detect(?int $filesPerProcess = null, ?int $processTimeout = null) : ParallelConfig { - $counter = new CpuCoreCounter([...FinderRegistry::getDefaultLogicalFinders(), new DummyCpuCoreFinder(1)]); + $counter = new CpuCoreCounter([...FinderRegistry::getDefaultLogicalFinders(), new DummyCpuCoreFinder(0)]); return new ParallelConfig(...array_filter([$counter->getCount(), $filesPerProcess, $processTimeout], static fn($value): bool => null !== $value)); } }

Check warning on line 43 in src/Runner/Parallel/ParallelConfigFactory.php

View workflow job for this annotation

GitHub Actions / PHP 8.3 mutation tests

Escaped Mutant for Mutator "IncrementInteger": --- Original +++ New @@ @@ */ public static function detect(?int $filesPerProcess = null, ?int $processTimeout = null) : ParallelConfig { - $counter = new CpuCoreCounter([...FinderRegistry::getDefaultLogicalFinders(), new DummyCpuCoreFinder(1)]); + $counter = new CpuCoreCounter([...FinderRegistry::getDefaultLogicalFinders(), new DummyCpuCoreFinder(2)]); return new ParallelConfig(...array_filter([$counter->getCount(), $filesPerProcess, $processTimeout], static fn($value): bool => null !== $value)); } }
]);

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
Loading