1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-18 19:40:55 +01:00

Improve Conduit performance of special edge-based custom Revision fields

Summary:
Ref T11404. Depends on D16351. Currently, both `differential.query` and `differential.revision.search` issue `2N` queries to fetch:

  - dependencies for each revision; and
  - projects for each revision.

Fix this:

  - Take these custom fields out of Conduit so they don't load this data by default.
  - For `differential.query`, put this data back in by hard coding it.
  - For `differential.revision.search`, just leave it out. You can already optionally get projects efficiently, and this endpoint is a work in progress. I would tentatively be inclined to expose graph data as a "graph" extension once we need it.

This makes both methods execute in `O(1)` time (which is still 20-30 queries, but at least it's not 320 queries anymore).

Test Plan:
  - Ran `differential.query`, observed no change in results but 199 fewer internal queries.
  - Ran `differential.revision.search`, observed data gone from results and 200 fewer internal queries.

Reviewers: yelirekim, chad

Reviewed By: chad

Maniphest Tasks: T11404

Differential Revision: https://secure.phabricator.com/D16352
This commit is contained in:
epriestley 2016-07-31 08:50:20 -07:00
parent b8f75f9511
commit 8fd20e82fc
3 changed files with 35 additions and 6 deletions

View file

@ -178,6 +178,7 @@ abstract class DifferentialConduitAPIMethod extends ConduitAPIMethod {
$results = array();
foreach ($field_lists as $revision_phid => $field_list) {
$results[$revision_phid] = array();
foreach ($field_list->getFields() as $field) {
$field_key = $field->getFieldKeyForConduit();
$value = $field->getConduitDictionaryValue();
@ -185,6 +186,32 @@ abstract class DifferentialConduitAPIMethod extends ConduitAPIMethod {
}
}
// For compatibility, fill in these "custom fields" by querying for them
// efficiently. See T11404 for discussion.
$legacy_edge_map = array(
'phabricator:projects' =>
PhabricatorProjectObjectHasProjectEdgeType::EDGECONST,
'phabricator:depends-on' =>
DifferentialRevisionDependsOnRevisionEdgeType::EDGECONST,
);
$query = id(new PhabricatorEdgeQuery())
->withSourcePHIDs(array_keys($results))
->withEdgeTypes($legacy_edge_map);
$query->execute();
foreach ($results as $revision_phid => $dict) {
foreach ($legacy_edge_map as $edge_key => $edge_type) {
$phid_list = $query->getDestinationPHIDs(
array($revision_phid),
array($edge_type));
$results[$revision_phid][$edge_key] = $phid_list;
}
}
return $results;
}

View file

@ -7,10 +7,6 @@ final class DifferentialParentRevisionsField
return 'differential:depends-on';
}
public function getFieldKeyForConduit() {
return 'phabricator:depends-on';
}
public function getFieldName() {
return pht('Parent Revisions');
}
@ -33,7 +29,10 @@ final class DifferentialParentRevisionsField
}
public function shouldAppearInConduitDictionary() {
return true;
// To improve performance, we exclude this field from Conduit results.
// See T11404 for discussion. In modern "differential.revision.search",
// this information is available efficiently as an attachment.
return false;
}
public function getConduitDictionaryValue() {

View file

@ -91,7 +91,10 @@ final class DifferentialProjectsField
}
public function shouldAppearInConduitDictionary() {
return true;
// To improve performance, we exclude this field from Conduit results.
// See T11404 for discussion. In modern "differential.revision.search",
// this information is available efficiently as an attachment.
return false;
}
public function getApplicationTransactionMetadata() {