revi-archive/phorge-phorge
1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-18 19:40:55 +01:00
Code Issues Releases Wiki Activity

Improve Differential query plans

Browse source 
Summary:
Ref T8575. We run a big "(A) UNION (B)" query on the home page and on the main Differential page.

"A" can always be improved by using `%Ls`, so it can use the second half of the `(authorPHID, status)` key.

"B" can sometimes be improved if the fraction of open revisions is smaller than the fraction of revisions you are reviewing. This is true for me on secure.phabricator.com (I'm a reviewer, either directly or via 'Blessed Reviewers', on about 80% of revisions, but <5% are open). In these cases, a `(status, phid)` key is more efficient.

Test Plan: Tweaked queries and added keys on this server, saw less derpy query plans and performance.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8575

Differential Revision: https://secure.phabricator.com/D13325
This commit is contained in:
epriestley 2015-06-17 11:25:01 -07:00
parent 984976ce20
commit b3ae48d8ca
2 changed files with 18 additions and 6 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

16
src/applications/differential/query/DifferentialRevisionQuery.php
View file

@ -771,19 +771,23 @@ final class DifferentialRevisionQuery
$this->updatedEpochMax); $this->updatedEpochMax);
} }
// NOTE: Although the status constants are integers in PHP, the column is a
// string column in MySQL, and MySQL won't use keys on string columns if
// you put integers in the query.
switch ($this->status) { switch ($this->status) {
case self::STATUS_ANY: case self::STATUS_ANY:
break; break;
case self::STATUS_OPEN: case self::STATUS_OPEN:
$where[] = qsprintf( $where[] = qsprintf(
$conn_r, $conn_r,
'r.status IN (%Ld)', 'r.status IN (%Ls)',
DifferentialRevisionStatus::getOpenStatuses()); DifferentialRevisionStatus::getOpenStatuses());
break; break;
case self::STATUS_NEEDS_REVIEW: case self::STATUS_NEEDS_REVIEW:
$where[] = qsprintf( $where[] = qsprintf(
$conn_r, $conn_r,
'r.status IN (%Ld)', 'r.status IN (%Ls)',
array( array(
ArcanistDifferentialRevisionStatus::NEEDS_REVIEW, ArcanistDifferentialRevisionStatus::NEEDS_REVIEW,
)); ));
@ -791,7 +795,7 @@ final class DifferentialRevisionQuery
case self::STATUS_NEEDS_REVISION: case self::STATUS_NEEDS_REVISION:
$where[] = qsprintf( $where[] = qsprintf(
$conn_r, $conn_r,
'r.status IN (%Ld)', 'r.status IN (%Ls)',
array( array(
ArcanistDifferentialRevisionStatus::NEEDS_REVISION, ArcanistDifferentialRevisionStatus::NEEDS_REVISION,
)); ));
@ -799,7 +803,7 @@ final class DifferentialRevisionQuery
case self::STATUS_ACCEPTED: case self::STATUS_ACCEPTED:
$where[] = qsprintf( $where[] = qsprintf(
$conn_r, $conn_r,
'r.status IN (%Ld)', 'r.status IN (%Ls)',
array( array(
ArcanistDifferentialRevisionStatus::ACCEPTED, ArcanistDifferentialRevisionStatus::ACCEPTED,
)); ));
@ -807,13 +811,13 @@ final class DifferentialRevisionQuery
case self::STATUS_CLOSED: case self::STATUS_CLOSED:
$where[] = qsprintf( $where[] = qsprintf(
$conn_r, $conn_r,
'r.status IN (%Ld)', 'r.status IN (%Ls)',
DifferentialRevisionStatus::getClosedStatuses()); DifferentialRevisionStatus::getClosedStatuses());
break; break;
case self::STATUS_ABANDONED: case self::STATUS_ABANDONED:
$where[] = qsprintf( $where[] = qsprintf(
$conn_r, $conn_r,
'r.status IN (%Ld)', 'r.status IN (%Ls)',
array( array(
ArcanistDifferentialRevisionStatus::ABANDONED, ArcanistDifferentialRevisionStatus::ABANDONED,
)); ));

8
src/applications/differential/storage/DifferentialRevision.php
View file

@ -102,6 +102,14 @@ final class DifferentialRevision extends DifferentialDAO
'repositoryPHID' => array( 'repositoryPHID' => array(
'columns' => array('repositoryPHID'), 'columns' => array('repositoryPHID'),
), ),
// If you (or a project you are a member of) is reviewing a significant
// fraction of the revisions on an install, the result set of open
// revisions may be smaller than the result set of revisions where you
// are a reviewer. In these cases, this key is better than keys on the
// edge table.
'key_status' => array(
'columns' => array('status', 'phid'),
),
), ),
) + parent::getConfiguration(); ) + parent::getConfiguration();
} }