@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Reduce the cost of loading large numbers of macros
Summary:
Ref T6013. I accidentally made this cost explosviely huge when fixing macros for logged out users in D10411.
Specifically, we'd load all the macros, which would load all the files, which would load all the macros (to do policy checks), which would fill out of cache I think (but maybe only some of the time?). Anyway, bad news.
Instead, only load the files if we need them.
Test Plan: Viewed macro main page, macro detail, used a macro, used a meme, edited a macro, edited audio.
Reviewers: btrahan, csilvers
Reviewed By: csilvers
Subscribers: epriestley, spicyj
Maniphest Tasks: T6013
Differential Revision: https://secure.phabricator.com/D10428
+29
-15
+3
-2
src/applications/macro/conduit/MacroQueryConduitAPIMethod.php
+3
-2
src/applications/macro/conduit/MacroQueryConduitAPIMethod.php
···
30
30
}
31
31
32
32
protected function execute(ConduitAPIRequest $request) {
33
-
$query = new PhabricatorMacroQuery();
34
-
$query->setViewer($request->getUser());
33
+
$query = id(new PhabricatorMacroQuery())
34
+
->setViewer($request->getUser())
35
+
->needFiles(true);
35
36
36
37
$author_phids = $request->getValue('authorPHIDs');
37
38
$phids = $request->getValue('phids');
+1
src/applications/macro/controller/PhabricatorMacroEditController.php
+1
src/applications/macro/controller/PhabricatorMacroEditController.php
+1
src/applications/macro/controller/PhabricatorMacroMemeController.php
+1
src/applications/macro/controller/PhabricatorMacroMemeController.php
+1
src/applications/macro/controller/PhabricatorMacroViewController.php
+1
src/applications/macro/controller/PhabricatorMacroViewController.php
+22
-13
src/applications/macro/query/PhabricatorMacroQuery.php
+22
-13
src/applications/macro/query/PhabricatorMacroQuery.php
···
12
12
private $dateCreatedBefore;
13
13
private $flagColor;
14
14
15
+
private $needFiles;
16
+
15
17
private $status = 'status-any';
16
18
const STATUS_ANY = 'status-any';
17
19
const STATUS_ACTIVE = 'status-active';
···
80
82
81
83
public function withFlagColor($flag_color) {
82
84
$this->flagColor = $flag_color;
85
+
return $this;
86
+
}
87
+
88
+
public function needFiles($need_files) {
89
+
$this->needFiles = $need_files;
83
90
return $this;
84
91
}
85
92
···
196
203
}
197
204
198
205
protected function didFilterPage(array $macros) {
199
-
$file_phids = mpull($macros, 'getFilePHID');
200
-
$files = id(new PhabricatorFileQuery())
201
-
->setViewer($this->getViewer())
202
-
->setParentQuery($this)
203
-
->withPHIDs($file_phids)
204
-
->execute();
205
-
$files = mpull($files, null, 'getPHID');
206
+
if ($this->needFiles) {
207
+
$file_phids = mpull($macros, 'getFilePHID');
208
+
$files = id(new PhabricatorFileQuery())
209
+
->setViewer($this->getViewer())
210
+
->setParentQuery($this)
211
+
->withPHIDs($file_phids)
212
+
->execute();
213
+
$files = mpull($files, null, 'getPHID');
206
214
207
-
foreach ($macros as $key => $macro) {
208
-
$file = idx($files, $macro->getFilePHID());
209
-
if (!$file) {
210
-
unset($macros[$key]);
211
-
continue;
215
+
foreach ($macros as $key => $macro) {
216
+
$file = idx($files, $macro->getFilePHID());
217
+
if (!$file) {
218
+
unset($macros[$key]);
219
+
continue;
220
+
}
221
+
$macro->attachFile($file);
212
222
}
213
-
$macro->attachFile($file);
214
223
}
215
224
216
225
return $macros;
+1
src/applications/macro/query/PhabricatorMacroSearchEngine.php
+1
src/applications/macro/query/PhabricatorMacroSearchEngine.php
···
29
29
30
30
public function buildQueryFromSavedQuery(PhabricatorSavedQuery $saved) {
31
31
$query = id(new PhabricatorMacroQuery())
32
+
->needFiles(true)
32
33
->withIDs($saved->getParameter('ids', array()))
33
34
->withPHIDs($saved->getParameter('phids', array()))
34
35
->withAuthorPHIDs($saved->getParameter('authorPHIDs', array()));