Skip to content

Commit 03fb188

Browse files
author
epriestley
committed
Fix file URI perf regression
Summary: The CSRF changes meant that we can't generate a file URI with just its PHID anymore, and converted a mathematical function into a service call. Unfortunately, this caused massive perf problems in some parts of the application, critically handles, where loading N users became N single gets. Derp derp derp. Remedy this by doing a single multiget. This substantially improves performance of many interfaces, particularly the Maniphest task list. I need to go through the rest of the PhabricatorFileURI callsites and get rid of them, but I think this is the most substantive one. Test Plan: Profiled Maniphest task list, queries went from >100 to a handful. Explosion of multiderp. :/ Looked at some views with profile photos to verify they still render accurately. Reviewers: jungejason, nh, tuomaspelkonen, aran Reviewed By: aran CC: aran Differential Revision: 921
1 parent 888af73 commit 03fb188

File tree

2 files changed

+15
-5
lines changed

2 files changed

+15
-5
lines changed

src/applications/phid/handle/data/PhabricatorObjectHandleData.php

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,17 @@ public function loadHandles() {
132132
$users = $object->loadAllWhere('phid IN (%Ls)', $phids);
133133
$users = mpull($users, null, 'getPHID');
134134

135+
$image_phids = mpull($users, 'getProfileImagePHID');
136+
$image_phids = array_unique(array_filter($image_phids));
137+
138+
$images = array();
139+
if ($image_phids) {
140+
$images = id(new PhabricatorFile())->loadAllWhere(
141+
'phid IN (%Ls)',
142+
$image_phids);
143+
$images = mpull($images, 'getViewURI', 'getPHID');
144+
}
145+
135146
foreach ($phids as $phid) {
136147
$handle = new PhabricatorObjectHandle();
137148
$handle->setPHID($phid);
@@ -148,10 +159,9 @@ public function loadHandles() {
148159
$handle->setAlternateID($user->getID());
149160
$handle->setComplete(true);
150161

151-
$img_phid = $user->getProfileImagePHID();
152-
if ($img_phid) {
153-
$handle->setImageURI(
154-
PhabricatorFileURI::getViewURIForPHID($img_phid));
162+
$img_uri = idx($images, $user->getProfileImagePHID());
163+
if ($img_uri) {
164+
$handle->setImageURI($img_uri);
155165
}
156166
}
157167
$handles[$phid] = $handle;

src/applications/phid/handle/data/__init__.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77

88

99
phutil_require_module('phabricator', 'applications/differential/constants/revisionstatus');
10-
phutil_require_module('phabricator', 'applications/files/uri');
10+
phutil_require_module('phabricator', 'applications/files/storage/file');
1111
phutil_require_module('phabricator', 'applications/maniphest/constants/owner');
1212
phutil_require_module('phabricator', 'applications/maniphest/constants/status');
1313
phutil_require_module('phabricator', 'applications/phid/constants');

0 commit comments

Comments
 (0)