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

WIP: Try to parallelize linting with amphp #4838

Closed
wants to merge 11 commits into from
3 changes: 2 additions & 1 deletion composer.json
Expand Up @@ -14,9 +14,10 @@
}
],
"require": {
"php": "^5.6 || ^7.0",
"php": "^7.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will work on how the compat with old php can/should look like, after I get it working on current php versions

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to see #4810.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't want PHP 5, then why do we even need a parallel linter? We can use the tokenising linter on PHP 7?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we even need a parallel linter? We can use the tokenising linter on PHP 7?

I had the theory that tokenized linting in parallel process is faster then doing it in a single process

"ext-json": "*",
"ext-tokenizer": "*",
"amphp/parallel": "^1.3",
"composer/semver": "^1.4",
"composer/xdebug-handler": "^1.2",
"doctrine/annotations": "^1.2",
Expand Down
5 changes: 4 additions & 1 deletion src/Linter/Linter.php
Expand Up @@ -32,10 +32,13 @@ final class Linter implements LinterInterface
public function __construct($executable = null)
{
try {
$this->sublinter = new TokenizerLinter();
// XXX feature detect the pooled linter
$this->sublinter = new PooledLinter();
// $this->sublinter = new TokenizerLinter();
} catch (UnavailableLinterException $e) {
$this->sublinter = new ProcessLinter($executable);
}
var_dump(get_class($this->sublinter));
}

/**
Expand Down
35 changes: 35 additions & 0 deletions src/Linter/PooledLintFileTask.php
@@ -0,0 +1,35 @@
<?php

namespace PhpCsFixer\Linter;

use Amp\Parallel\Worker\Environment;
use Amp\Parallel\Worker\Task;
use PhpCsFixer\Linter\TokenizerLinter;

class PooledLintFileTask implements Task {
private $path;

/**
* @param string $path
*/
public function __construct($path)
{
$this->path = $path;
}

/**
* @inheritDoc
*/
public function run(Environment $environment) {
$linter = new TokenizerLinter();

try {
$linter->lintFile($this->path)->check();
return true;
} catch( LintingException $e) {
// its not easy to handle a exception thrown within a task on the caller site.
// therefore we transform it to a string a re-create the exception later on.
return $e->getMessage();
}
}
}
36 changes: 36 additions & 0 deletions src/Linter/PooledLintSourceTask.php
@@ -0,0 +1,36 @@
<?php

namespace PhpCsFixer\Linter;

use Amp\Parallel\Worker\Environment;
use Amp\Parallel\Worker\Task;
use PhpCsFixer\Linter\TokenizerLinter;

class PooledLintSourceTask implements Task {
private $source;

/**
* @param string $source
*/
public function __construct($source)
{
$this->source = $source;
}

/**
* @inheritDoc
*/
public function run(Environment $environment) {
$linter = new TokenizerLinter();

try {
$linter->lintSource($this->source)->check();
return true;
} catch( LintingException $e) {
// its not easy to handle a exception thrown within a task on the caller site.
// therefore we transform it to a string a re-create the exception later on.
return $e->getMessage();
}
}
};

69 changes: 69 additions & 0 deletions src/Linter/PooledLinter.php
@@ -0,0 +1,69 @@
<?php

/*
* 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\Linter;

use Amp\Parallel\Worker\DefaultPool;
use Amp\Parallel\Worker\Environment;
use Amp\Parallel\Worker\Task;
use Amp\Parallel\Worker\Pool;

class PooledLinter implements LinterInterface
{
/**
* @var Pool
*/
private $processPool;

public function __construct(Pool $pool = null)
{
$this->processPool = $pool ?? \Amp\Parallel\Worker\pool();
}

public function __destruct()
{
\Amp\Promise\wait($this->processPool->shutdown());
}

/**
* {@inheritdoc}
*/
public function isAsync()
{
return true;
}

/**
* {@inheritdoc}
*/
public function lintFile($path)
{
return new PooledLintingResult($this->processPool->enqueue($this->createTaskForFile($path)));
}

/**
* {@inheritdoc}
*/
public function lintSource($source)
{
return new PooledLintingResult($this->processPool->enqueue($this->createTaskForSource($source)));
}

private function createTaskForFile($path) {
return new PooledLintFileTask($path);
}

private function createTaskForSource($source) {
return new PooledLintSourceTask($source);
}
}

56 changes: 56 additions & 0 deletions src/Linter/PooledLintingResult.php
@@ -0,0 +1,56 @@
<?php

/*
* 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\Linter;

use Amp\Parallel\Worker\Task;
use Amp\Parallel\Worker\TaskFailureException;
use Amp\Promise;
use Symfony\Component\Process\Process;

/**
* @internal
*/
final class PooledLintingResult implements LintingResultInterface
{
/**
* @var string|bool|null
*/
private $result;

/**
* @var Promise
*/
private $promise;

/**
* @param Promise<TokenizerLintingResult> $promise
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* @param Promise<TokenizerLintingResult> $promise
* @param Promise $promise

*/
public function __construct(Promise $promise)
{
$this->promise = $promise;
}

/**
* {@inheritdoc}
*/
public function check()
Copy link
Contributor Author

@staabm staabm Feb 23, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should return :void

{
if (null === $this->result) {
$this->result = \Amp\Promise\wait($this->promise);
}

if (is_string($this->result)) {
throw new LintingException($this->result);
}
}
}