mirror of
https://we.phorge.it/source/phorge.git
synced 2024-11-10 08:52:39 +01:00
Use futures to improve clustered repository main page performance
Summary: Ref T11954. In cluster configurations, we get repository information by making HTTP calls over Conduit. These are slower than local calls, so clustering imposes a performance penalty. However, we can use futures and parallelize them so that clustering actually improves overall performance. When not running in clustered mode, this just makes us run stuff inline. Test Plan: - Browsed Git, Mercurial and Subversion repositories. - Locally, saw a 700ms wall time page drop to 200ms. Reviewers: chad Reviewed By: chad Maniphest Tasks: T11954 Differential Revision: https://secure.phabricator.com/D17009
This commit is contained in:
parent
4950926130
commit
5f26dd9b66
5 changed files with 158 additions and 52 deletions
|
@ -42,6 +42,10 @@ final class PhabricatorConduitToken
|
|||
return null;
|
||||
}
|
||||
|
||||
if ($user->hasConduitClusterToken()) {
|
||||
return $user->getConduitClusterToken();
|
||||
}
|
||||
|
||||
$tokens = id(new PhabricatorConduitTokenQuery())
|
||||
->setViewer($user)
|
||||
->withObjectPHIDs(array($user->getPHID()))
|
||||
|
@ -55,23 +59,28 @@ final class PhabricatorConduitToken
|
|||
$now = PhabricatorTime::getNow();
|
||||
$must_expire_after = $now + phutil_units('5 minutes in seconds');
|
||||
|
||||
$valid_token = null;
|
||||
foreach ($tokens as $token) {
|
||||
if ($token->getExpires() > $must_expire_after) {
|
||||
return $token;
|
||||
$valid_token = $token;
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
// We didn't find any existing tokens (or the existing tokens are all about
|
||||
// to expire) so generate a new token.
|
||||
if (!$valid_token) {
|
||||
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
|
||||
$valid_token = self::initializeNewToken(
|
||||
$user->getPHID(),
|
||||
self::TYPE_CLUSTER);
|
||||
$valid_token->save();
|
||||
unset($unguarded);
|
||||
}
|
||||
|
||||
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
|
||||
$token = self::initializeNewToken(
|
||||
$user->getPHID(),
|
||||
self::TYPE_CLUSTER);
|
||||
$token->save();
|
||||
unset($unguarded);
|
||||
$user->attachConduitClusterToken($valid_token);
|
||||
|
||||
return $token;
|
||||
return $valid_token;
|
||||
}
|
||||
|
||||
public static function initializeNewToken($object_phid, $token_type) {
|
||||
|
|
|
@ -239,6 +239,18 @@ abstract class DiffusionController extends PhabricatorController {
|
|||
$params);
|
||||
}
|
||||
|
||||
protected function callConduitMethod($method, array $params = array()) {
|
||||
$user = $this->getViewer();
|
||||
$drequest = $this->getDiffusionRequest();
|
||||
|
||||
return DiffusionQuery::callConduitWithDiffusionRequest(
|
||||
$user,
|
||||
$drequest,
|
||||
$method,
|
||||
$params,
|
||||
true);
|
||||
}
|
||||
|
||||
protected function getRepositoryControllerURI(
|
||||
PhabricatorRepository $repository,
|
||||
$path) {
|
||||
|
|
|
@ -2,6 +2,11 @@
|
|||
|
||||
final class DiffusionRepositoryController extends DiffusionController {
|
||||
|
||||
private $historyFuture;
|
||||
private $browseFuture;
|
||||
private $tagFuture;
|
||||
private $branchFuture;
|
||||
|
||||
public function shouldAllowPublic() {
|
||||
return true;
|
||||
}
|
||||
|
@ -106,18 +111,67 @@ final class DiffusionRepositoryController extends DiffusionController {
|
|||
$request = $this->getRequest();
|
||||
$repository = $drequest->getRepository();
|
||||
|
||||
$commit = $drequest->getCommit();
|
||||
$path = $drequest->getPath();
|
||||
|
||||
$this->historyFuture = $this->callConduitMethod(
|
||||
'diffusion.historyquery',
|
||||
array(
|
||||
'commit' => $commit,
|
||||
'path' => $path,
|
||||
'offset' => 0,
|
||||
'limit' => 15,
|
||||
));
|
||||
|
||||
$browse_pager = id(new PHUIPagerView())
|
||||
->readFromRequest($request);
|
||||
|
||||
$this->browseFuture = $this->callConduitMethod(
|
||||
'diffusion.browsequery',
|
||||
array(
|
||||
'commit' => $commit,
|
||||
'path' => $path,
|
||||
'limit' => $browse_pager->getPageSize() + 1,
|
||||
));
|
||||
|
||||
if ($this->needTagFuture()) {
|
||||
$tag_limit = $this->getTagLimit();
|
||||
$this->tagFuture = $this->callConduitMethod(
|
||||
'diffusion.tagsquery',
|
||||
array(
|
||||
// On the home page, we want to find tags on any branch.
|
||||
'commit' => null,
|
||||
'limit' => $tag_limit + 1,
|
||||
));
|
||||
}
|
||||
|
||||
if ($this->needBranchFuture()) {
|
||||
$branch_limit = $this->getBranchLimit();
|
||||
$this->branchFuture = $this->callConduitMethod(
|
||||
'diffusion.branchquery',
|
||||
array(
|
||||
'closed' => false,
|
||||
'limit' => $branch_limit + 1,
|
||||
));
|
||||
}
|
||||
|
||||
$futures = array(
|
||||
$this->historyFuture,
|
||||
$this->browseFuture,
|
||||
$this->tagFuture,
|
||||
$this->branchFuture,
|
||||
);
|
||||
$futures = array_filter($futures);
|
||||
$futures = new FutureIterator($futures);
|
||||
foreach ($futures as $future) {
|
||||
// Just resolve all the futures before continuing.
|
||||
}
|
||||
|
||||
$phids = array();
|
||||
$content = array();
|
||||
|
||||
try {
|
||||
$history_results = $this->callConduitWithDiffusionRequest(
|
||||
'diffusion.historyquery',
|
||||
array(
|
||||
'commit' => $drequest->getCommit(),
|
||||
'path' => $drequest->getPath(),
|
||||
'offset' => 0,
|
||||
'limit' => 15,
|
||||
));
|
||||
$history_results = $this->historyFuture->resolve();
|
||||
$history = DiffusionPathChange::newFromConduit(
|
||||
$history_results['pathChanges']);
|
||||
|
||||
|
@ -139,18 +193,11 @@ final class DiffusionRepositoryController extends DiffusionController {
|
|||
$history_exception = $ex;
|
||||
}
|
||||
|
||||
$browse_pager = id(new PHUIPagerView())
|
||||
->readFromRequest($request);
|
||||
|
||||
try {
|
||||
$browse_results = $this->browseFuture->resolve();
|
||||
$browse_results = DiffusionBrowseResultSet::newFromConduit(
|
||||
$this->callConduitWithDiffusionRequest(
|
||||
'diffusion.browsequery',
|
||||
array(
|
||||
'path' => $drequest->getPath(),
|
||||
'commit' => $drequest->getCommit(),
|
||||
'limit' => $browse_pager->getPageSize() + 1,
|
||||
)));
|
||||
$browse_results);
|
||||
|
||||
$browse_paths = $browse_results->getPaths();
|
||||
$browse_paths = $browse_pager->sliceResults($browse_paths);
|
||||
|
||||
|
@ -366,22 +413,16 @@ final class DiffusionRepositoryController extends DiffusionController {
|
|||
private function buildBranchListTable(DiffusionRequest $drequest) {
|
||||
$viewer = $this->getViewer();
|
||||
|
||||
if ($drequest->getBranch() === null) {
|
||||
if (!$this->needBranchFuture()) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$limit = 15;
|
||||
|
||||
$branches = $this->callConduitWithDiffusionRequest(
|
||||
'diffusion.branchquery',
|
||||
array(
|
||||
'closed' => false,
|
||||
'limit' => $limit + 1,
|
||||
));
|
||||
$branches = $this->branchFuture->resolve();
|
||||
if (!$branches) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$limit = $this->getBranchLimit();
|
||||
$more_branches = (count($branches) > $limit);
|
||||
$branches = array_slice($branches, 0, $limit);
|
||||
|
||||
|
@ -428,26 +469,17 @@ final class DiffusionRepositoryController extends DiffusionController {
|
|||
$viewer = $this->getViewer();
|
||||
$repository = $drequest->getRepository();
|
||||
|
||||
switch ($repository->getVersionControlSystem()) {
|
||||
case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
|
||||
// no tags in SVN
|
||||
return null;
|
||||
if (!$this->needTagFuture()) {
|
||||
return null;
|
||||
}
|
||||
$tag_limit = 15;
|
||||
$tags = array();
|
||||
$tags = DiffusionRepositoryTag::newFromConduit(
|
||||
$this->callConduitWithDiffusionRequest(
|
||||
'diffusion.tagsquery',
|
||||
array(
|
||||
// On the home page, we want to find tags on any branch.
|
||||
'commit' => null,
|
||||
'limit' => $tag_limit + 1,
|
||||
)));
|
||||
|
||||
$tags = $this->tagFuture->resolve();
|
||||
$tags = DiffusionRepositoryTag::newFromConduit($tags);
|
||||
if (!$tags) {
|
||||
return null;
|
||||
}
|
||||
|
||||
$tag_limit = $this->getTagLimit();
|
||||
$more_tags = (count($tags) > $tag_limit);
|
||||
$tags = array_slice($tags, 0, $tag_limit);
|
||||
|
||||
|
@ -688,4 +720,35 @@ final class DiffusionRepositoryController extends DiffusionController {
|
|||
->setDisplayURI($display);
|
||||
}
|
||||
|
||||
private function needTagFuture() {
|
||||
$drequest = $this->getDiffusionRequest();
|
||||
$repository = $drequest->getRepository();
|
||||
|
||||
switch ($repository->getVersionControlSystem()) {
|
||||
case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN:
|
||||
// No tags in SVN.
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
private function getTagLimit() {
|
||||
return 15;
|
||||
}
|
||||
|
||||
private function needBranchFuture() {
|
||||
$drequest = $this->getDiffusionRequest();
|
||||
|
||||
if ($drequest->getBranch() === null) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
private function getBranchLimit() {
|
||||
return 15;
|
||||
}
|
||||
|
||||
}
|
||||
|
|
|
@ -48,7 +48,8 @@ abstract class DiffusionQuery extends PhabricatorQuery {
|
|||
PhabricatorUser $user,
|
||||
DiffusionRequest $drequest,
|
||||
$method,
|
||||
array $params = array()) {
|
||||
array $params = array(),
|
||||
$return_future = false) {
|
||||
|
||||
$repository = $drequest->getRepository();
|
||||
|
||||
|
@ -76,12 +77,19 @@ abstract class DiffusionQuery extends PhabricatorQuery {
|
|||
$user,
|
||||
$drequest->getIsClusterRequest());
|
||||
if (!$client) {
|
||||
return id(new ConduitCall($method, $params))
|
||||
$result = id(new ConduitCall($method, $params))
|
||||
->setUser($user)
|
||||
->execute();
|
||||
$future = new ImmediateFuture($result);
|
||||
} else {
|
||||
return $client->callMethodSynchronous($method, $params);
|
||||
$future = $client->callMethod($method, $params);
|
||||
}
|
||||
|
||||
if (!$return_future) {
|
||||
return $future->resolve();
|
||||
}
|
||||
|
||||
return $future;
|
||||
}
|
||||
|
||||
public function execute() {
|
||||
|
|
|
@ -64,6 +64,7 @@ final class PhabricatorUser
|
|||
private $settingCacheKeys = array();
|
||||
private $settingCache = array();
|
||||
private $allowInlineCacheGeneration;
|
||||
private $conduitClusterToken = self::ATTACHABLE;
|
||||
|
||||
protected function readField($field) {
|
||||
switch ($field) {
|
||||
|
@ -937,6 +938,19 @@ final class PhabricatorUser
|
|||
return $this->authorities;
|
||||
}
|
||||
|
||||
public function hasConduitClusterToken() {
|
||||
return ($this->conduitClusterToken !== self::ATTACHABLE);
|
||||
}
|
||||
|
||||
public function attachConduitClusterToken(PhabricatorConduitToken $token) {
|
||||
$this->conduitClusterToken = $token;
|
||||
return $this;
|
||||
}
|
||||
|
||||
public function getConduitClusterToken() {
|
||||
return $this->assertAttached($this->conduitClusterToken);
|
||||
}
|
||||
|
||||
|
||||
/* -( Availability )------------------------------------------------------- */
|
||||
|
||||
|
|
Loading…
Reference in a new issue