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
Changes from 7 commits
2afbcd1
8c73bbb
1a3a437
0e15e7a
7603b4b
5de0f5e
c27932a
791b05d
f21e1c1
6b42442
b02a0f1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,24 @@ | ||||||
<?php | ||||||
|
||||||
namespace PhpCsFixer\Linter; | ||||||
|
||||||
use Amp\Parallel\Worker\Environment; | ||||||
use Amp\Parallel\Worker\Task; | ||||||
use PhpCsFixer\Linter\TokenizerLinter; | ||||||
|
||||||
class PooledLintFileTask implements Task { | ||||||
private $path; | ||||||
|
||||||
public function __construct($path) | ||||||
{ | ||||||
$this->path = $path; | ||||||
} | ||||||
|
||||||
/** | ||||||
* @inheritDoc | ||||||
*/ | ||||||
public function run(Environment $environment) { | ||||||
$linter = new TokenizerLinter(); | ||||||
return $linter->lintFile($this->path); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,25 @@ | ||||||
<?php | ||||||
|
||||||
namespace PhpCsFixer\Linter; | ||||||
|
||||||
use Amp\Parallel\Worker\Environment; | ||||||
use Amp\Parallel\Worker\Task; | ||||||
use PhpCsFixer\Linter\TokenizerLinter; | ||||||
|
||||||
class PooledLintSourceTask implements Task { | ||||||
private $source; | ||||||
|
||||||
public function __construct($source) | ||||||
{ | ||||||
$this->source = $source; | ||||||
} | ||||||
|
||||||
/** | ||||||
* @inheritDoc | ||||||
*/ | ||||||
public function run(Environment $environment) { | ||||||
$linter = new TokenizerLinter(); | ||||||
return $linter->lintSource($this->source); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
} | ||||||
}; | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,65 @@ | ||
<?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; | ||
|
||
class PooledLinter implements LinterInterface | ||
{ | ||
private $processPool; | ||
|
||
public function __construct() | ||
{ | ||
$this->processPool = new DefaultPool(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would allow a |
||
} | ||
|
||
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); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,57 @@ | ||||||||
<?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\Promise; | ||||||||
use Symfony\Component\Process\Process; | ||||||||
|
||||||||
/** | ||||||||
* @internal | ||||||||
*/ | ||||||||
final class PooledLintingResult implements LintingResultInterface | ||||||||
{ | ||||||||
/** | ||||||||
* @var bool | ||||||||
*/ | ||||||||
private $isSuccessful; | ||||||||
|
||||||||
/** | ||||||||
* @var Promise | ||||||||
*/ | ||||||||
private $promise; | ||||||||
|
||||||||
/** | ||||||||
* @param Promise<TokenizerLintingResult> $promise | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
*/ | ||||||||
public function __construct(Promise $promise) | ||||||||
{ | ||||||||
$this->promise = $promise; | ||||||||
} | ||||||||
|
||||||||
/** | ||||||||
* {@inheritdoc} | ||||||||
*/ | ||||||||
public function check() | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should return :void |
||||||||
{ | ||||||||
if (null === $this->isSuccessful) { | ||||||||
/** | ||||||||
* @var TokenizerLintingResult | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
*/ | ||||||||
$result = \Amp\Promise\wait($this->promise); | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||
$this->isSuccessful = $result->check(); | ||||||||
} | ||||||||
|
||||||||
return $this->isSuccessful; | ||||||||
} | ||||||||
} |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had the theory that tokenized linting in parallel process is faster then doing it in a single process