From b0665981c2e80578b6692da8604edcda7b2093ab Mon Sep 17 00:00:00 2001 From: Jordi Boggiano Date: Thu, 17 Mar 2022 14:52:14 +0100 Subject: [PATCH] Parallellize the branch comparisons to speed up bootstrapping/version guessing when on a feature branch (#10632) * Parallellize the branch comparisons to speed up bootstrapping/version guessing when on a feature branch, fixes #10568 * Allow ProcessExecutorMock to function with async calls --- .../Package/Version/VersionGuesser.php | 36 +++++++++++++------ src/Composer/Util/ProcessExecutor.php | 12 ++++++- .../Test/Mock/ProcessExecutorMock.php | 28 +++++++++++++-- .../Package/Loader/RootPackageLoaderTest.php | 6 +++- .../Test/Repository/Vcs/GitHubDriverTest.php | 2 +- tests/Composer/Test/TestCase.php | 3 +- 6 files changed, 69 insertions(+), 18 deletions(-) diff --git a/src/Composer/Package/Version/VersionGuesser.php b/src/Composer/Package/Version/VersionGuesser.php index 7e7a0cae1..f3b857375 100644 --- a/src/Composer/Package/Version/VersionGuesser.php +++ b/src/Composer/Package/Version/VersionGuesser.php @@ -21,6 +21,8 @@ use Composer\Util\Git as GitUtil; use Composer\Util\HttpDownloader; use Composer\Util\ProcessExecutor; use Composer\Util\Svn as SvnUtil; +use React\Promise\CancellablePromiseInterface; +use Symfony\Component\Process\Process; /** * Try to guess the current version number based on different VCS configuration. @@ -307,6 +309,8 @@ class VersionGuesser return strnatcasecmp($b, $a); }); + $promises = []; + $this->process->setMaxJobs(30); foreach ($branches as $candidate) { $candidateVersion = Preg::replace('{^remotes/\S+/}', '', $candidate); @@ -316,19 +320,29 @@ class VersionGuesser } $cmdLine = str_replace(array('%candidate%', '%branch%'), array($candidate, $branch), $scmCmdline); - if (0 !== $this->process->execute($cmdLine, $output, $path)) { - continue; - } - - if (strlen($output) < $length) { - $length = strlen($output); - $version = $this->versionParser->normalizeBranch($candidateVersion); - $prettyVersion = 'dev-' . $candidateVersion; - if ($length === 0) { - break; + $promises[] = $this->process->executeAsync($cmdLine, $path)->then(function (Process $process) use (&$length, &$version, &$prettyVersion, $candidateVersion, &$promises): void { + if (!$process->isSuccessful()) { + return; } - } + + $output = $process->getOutput(); + if (strlen($output) < $length) { + $length = strlen($output); + $version = $this->versionParser->normalizeBranch($candidateVersion); + $prettyVersion = 'dev-' . $candidateVersion; + if ($length === 0) { + foreach ($promises as $promise) { + if ($promise instanceof CancellablePromiseInterface) { + $promise->cancel(); + } + } + } + } + }); } + + $this->process->wait(); + $this->process->resetMaxJobs(); } return array('version' => $version, 'pretty_version' => $prettyVersion); diff --git a/src/Composer/Util/ProcessExecutor.php b/src/Composer/Util/ProcessExecutor.php index d2572a02e..a4c707cf9 100644 --- a/src/Composer/Util/ProcessExecutor.php +++ b/src/Composer/Util/ProcessExecutor.php @@ -271,6 +271,16 @@ class ProcessExecutor } } + public function setMaxJobs(int $maxJobs): void + { + $this->maxJobs = $maxJobs; + } + + public function resetMaxJobs(): void + { + $this->maxJobs = 10; + } + /** * @param ?int $index job id * @return void @@ -278,7 +288,7 @@ class ProcessExecutor public function wait($index = null): void { while (true) { - if (!$this->countActiveJobs($index)) { + if (0 === $this->countActiveJobs($index)) { return; } diff --git a/tests/Composer/Test/Mock/ProcessExecutorMock.php b/tests/Composer/Test/Mock/ProcessExecutorMock.php index b8948e6ae..253f1a129 100644 --- a/tests/Composer/Test/Mock/ProcessExecutorMock.php +++ b/tests/Composer/Test/Mock/ProcessExecutorMock.php @@ -12,6 +12,8 @@ namespace Composer\Test\Mock; +use Composer\Test\TestCase; +use PHPUnit\Framework\MockObject\MockBuilder; use React\Promise\PromiseInterface; use Composer\Util\ProcessExecutor; use Composer\Util\Platform; @@ -41,6 +43,19 @@ class ProcessExecutorMock extends ProcessExecutor * @var string[] */ private $log = array(); + /** + * @var MockBuilder + */ + private $processMockBuilder; + + /** + * @param MockBuilder $processMockBuilder + */ + public function __construct(MockBuilder $processMockBuilder) + { + parent::__construct(); + $this->processMockBuilder = $processMockBuilder->disableOriginalConstructor(); + } /** * @param array, return?: int, stdout?: string, stderr?: string, callback?: callable}> $expectations @@ -180,9 +195,16 @@ class ProcessExecutorMock extends ProcessExecutor public function executeAsync($command, ?string $cwd = null): PromiseInterface { - $resolver = function ($resolve, $reject): void { - // TODO strictly speaking this should resolve with a mock Process instance here - $resolve(); + $cwd = $cwd ?? Platform::getCwd(); + + $resolver = function ($resolve, $reject) use ($command, $cwd): void { + $result = $this->doExecute($command, $cwd, false, $output); + $procMock = $this->processMockBuilder->getMock(); + $procMock->method('getOutput')->willReturn($output); + $procMock->method('isSuccessful')->willReturn($result === 0); + $procMock->method('getExitCode')->willReturn($result); + + $resolve($procMock); }; $canceler = function (): void { diff --git a/tests/Composer/Test/Package/Loader/RootPackageLoaderTest.php b/tests/Composer/Test/Package/Loader/RootPackageLoaderTest.php index 7f8872fc1..6d0cc3d34 100644 --- a/tests/Composer/Test/Package/Loader/RootPackageLoaderTest.php +++ b/tests/Composer/Test/Package/Loader/RootPackageLoaderTest.php @@ -20,6 +20,7 @@ use Composer\Package\RootPackage; use Composer\Package\Version\VersionGuesser; use Composer\Semver\VersionParser; use Composer\Test\TestCase; +use Composer\Util\ProcessExecutor; class RootPackageLoaderTest extends TestCase { @@ -36,8 +37,11 @@ class RootPackageLoaderTest extends TestCase $config = new Config; $config->merge(array('repositories' => array('packagist' => false))); + $processExecutor = new ProcessExecutor(); + $processExecutor->enableAsync(); + $guesser = new VersionGuesser($config, $processExecutor, new VersionParser()); - $loader = new RootPackageLoader($manager, $config); + $loader = new RootPackageLoader($manager, $config, null, $guesser); return $loader->load($data); } diff --git a/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php b/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php index bcacb53a6..c4e607566 100644 --- a/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php +++ b/tests/Composer/Test/Repository/Vcs/GitHubDriverTest.php @@ -320,7 +320,7 @@ class GitHubDriverTest extends TestCase ->setConstructorArgs(array($io, $this->config)) ->getMock(); - $gitHubDriver = new GitHubDriver($repoConfig, $io, $this->config, $httpDownloader, new ProcessExecutorMock); + $gitHubDriver = new GitHubDriver($repoConfig, $io, $this->config, $httpDownloader, $this->getProcessExecutorMock()); $gitHubDriver->initialize(); } diff --git a/tests/Composer/Test/TestCase.php b/tests/Composer/Test/TestCase.php index a26dbfb6e..ff04e79d1 100644 --- a/tests/Composer/Test/TestCase.php +++ b/tests/Composer/Test/TestCase.php @@ -32,6 +32,7 @@ use Composer\Package\RootAliasPackage; use Composer\Package\CompletePackage; use Composer\Package\CompleteAliasPackage; use Composer\Package\Package; +use Symfony\Component\Process\Process; abstract class TestCase extends \PHPUnit\Framework\TestCase { @@ -256,7 +257,7 @@ abstract class TestCase extends \PHPUnit\Framework\TestCase protected function getProcessExecutorMock(): ProcessExecutorMock { - $this->processExecutorMocks[] = $mock = new ProcessExecutorMock(); + $this->processExecutorMocks[] = $mock = new ProcessExecutorMock($this->getMockBuilder(Process::class)); return $mock; }