Skip to content

Commit a4de3e9

Browse files
williamjallenwilliamjallen
authored
Load all dynamic build groups simultaneously (#3732)
Projects with many builds assigned to dynamic build groups can see reduced performance due to the iterative execution of an expensive query. This addresses the issue by precomputing the queries and combining their results with UNION ALL. The end result is a constant two-query process: a first query which selects the relevant build IDs, and then another which selects the data we need. This can be improved in the future by using a join instead of a union, but that has a number of tricky corner cases to address and should be done separately from this change.
1 parent eae986a commit a4de3e9

2 files changed

Lines changed: 163 additions & 104 deletions

File tree

‎app/cdash/app/Controller/Api/Index.php‎

Lines changed: 162 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -209,131 +209,190 @@ public function getDynamicBuilds(): array
209209
$this->endDate,
210210
]);
211211

212+
$latest_rules = [];
212213
foreach ($stmt as $rule) {
213214
$buildgroup_name = $rule->name;
214215
if (strlen($this->buildGroupName) > 0 && $this->buildGroupName != $buildgroup_name) {
215216
continue;
216217
}
217-
$buildgroup_id = $rule->id;
218-
$buildgroup_position = $rule->position;
219218
if ($rule->type === 'Latest') {
220-
$whereClauses = [];
221-
$query_params = [];
219+
$latest_rules[] = $rule;
220+
}
221+
}
222222

223-
$sql = $this->getIndexQuery();
223+
if (empty($latest_rules)) {
224+
return $builds;
225+
}
224226

225-
// Add a projectid filter to help the planner choose a better execution plan, even
226-
// though all the groups are already associated with the project.
227-
$whereClauses[] = 'b.projectid=?';
228-
$query_params[] = (int) $this->project->Id;
227+
$union_parts = [];
228+
$union_params = [];
229+
foreach ($latest_rules as $idx => $rule) {
230+
// Add a projectid filter to help the planner choose a better execution plan, even
231+
// though all the groups are already associated with the project.
232+
$whereClauses = ['b.projectid=?'];
233+
$params = [(int) $this->project->Id];
234+
if (!empty($rule->parentgroupid)) {
235+
$whereClauses[] = 'b2g.groupid=?';
236+
$params[] = (int) $rule->parentgroupid;
237+
}
238+
if (!empty($rule->siteid)) {
239+
$whereClauses[] = 's.id=?';
240+
$params[] = (int) $rule->siteid;
241+
}
242+
if (!empty($rule->buildname)) {
243+
$whereClauses[] = 'b.name=?';
244+
$params[] = $rule->buildname;
245+
}
229246

230-
// optional fields: parentgroupid, site, and build name match.
231-
// Use these to construct a WHERE clause for our query.
232-
if (!empty($rule->parentgroupid)) {
233-
$whereClauses[] = 'b2g.groupid=?';
234-
$query_params[] = (int) $rule->parentgroupid;
235-
}
236-
if (!empty($rule->siteid)) {
237-
$whereClauses[] = 's.id=?';
238-
$query_params[] = (int) $rule->siteid;
239-
}
240-
if (!empty($rule->buildname)) {
241-
$whereClauses[] = 'b.name = ?';
242-
$query_params[] = $rule->buildname;
243-
}
244-
if (count($whereClauses) > 0) {
245-
$sql .= ' WHERE ' . implode(' AND ', $whereClauses);
246-
$sql .= ' AND b.starttime < ? ';
247-
$query_params[] = $this->endDate;
248-
}
247+
$sql = "SELECT b.id, $idx AS rule_idx " . $this->getIndexJoin();
248+
$sql .= ' WHERE ' . implode(' AND ', $whereClauses);
249+
$sql .= ' AND b.starttime < ? ';
250+
$params[] = $this->endDate;
251+
$sql .= $this->filterSQL;
252+
$sql .= ' ORDER BY b.submittime DESC LIMIT 1 ';
253+
254+
$union_parts[] = "($sql)";
255+
$union_params = array_merge($union_params, $params);
256+
}
257+
258+
$full_union_sql = implode(' UNION ALL ', $union_parts);
259+
$id_results = DB::select($full_union_sql, $union_params);
249260

250-
$sql .= $this->filterSQL;
261+
if (empty($id_results)) {
262+
return $builds;
263+
}
264+
265+
$unique_build_ids = [];
266+
$rule_idx_to_build_id = [];
267+
foreach ($id_results as $row) {
268+
$unique_build_ids[] = (int) $row->id;
269+
$rule_idx_to_build_id[(int) $row->rule_idx] = (int) $row->id;
270+
}
271+
$unique_build_ids = array_unique($unique_build_ids);
251272

252-
// We only want the most recent build.
253-
$sql .= ' ORDER BY b.submittime DESC LIMIT 1 ';
273+
$placeholders = implode(',', array_fill(0, count($unique_build_ids), '?'));
274+
$sql = $this->getIndexQuery() . " WHERE b.id IN ($placeholders)";
275+
$full_build_results = DB::select($sql, array_values($unique_build_ids));
254276

255-
$results = DB::select($sql, $query_params);
256-
foreach ($results as $build) {
257-
$build = (array) $build;
258-
$build['groupname'] = $buildgroup_name;
259-
$build['groupid'] = $buildgroup_id;
260-
$build['position'] = $buildgroup_position;
261-
$builds[] = $build;
277+
$build_id_to_data = [];
278+
foreach ($full_build_results as $full_build) {
279+
$build_id_to_data[(int) $full_build->id][] = (array) $full_build;
280+
}
281+
282+
foreach ($latest_rules as $idx => $rule) {
283+
if (isset($rule_idx_to_build_id[$idx])) {
284+
$build_id = $rule_idx_to_build_id[$idx];
285+
if (isset($build_id_to_data[$build_id])) {
286+
foreach ($build_id_to_data[$build_id] as $build_row) {
287+
$build_row['groupname'] = $rule->name;
288+
$build_row['groupid'] = $rule->id;
289+
$build_row['position'] = $rule->position;
290+
$builds[] = $build_row;
291+
}
262292
}
263293
}
264294
}
295+
265296
return $builds;
266297
}
267298

299+
public function getIndexSelect(): string
300+
{
301+
return '
302+
SELECT
303+
b.id,
304+
b.siteid,
305+
b.parentid,
306+
b.done,
307+
b.changeid,
308+
b.testduration,
309+
bu.status AS updatestatus,
310+
b.osname AS osname,
311+
bu.starttime AS updatestarttime,
312+
bu.endtime AS updateendtime,
313+
bu.nfiles AS countupdatefiles,
314+
bu.warnings AS countupdatewarnings,
315+
bu.revision,
316+
b.configureduration,
317+
be_diff.difference_positive AS countbuilderrordiffp,
318+
be_diff.difference_negative AS countbuilderrordiffn,
319+
bw_diff.difference_positive AS countbuildwarningdiffp,
320+
bw_diff.difference_negative AS countbuildwarningdiffn,
321+
ce_diff.difference AS countconfigurewarningdiff,
322+
btt.time AS testtime,
323+
tnotrun_diff.difference_positive AS counttestsnotrundiffp,
324+
tnotrun_diff.difference_negative AS counttestsnotrundiffn,
325+
tfailed_diff.difference_positive AS counttestsfaileddiffp,
326+
tfailed_diff.difference_negative AS counttestsfaileddiffn,
327+
tpassed_diff.difference_positive AS counttestspasseddiffp,
328+
tpassed_diff.difference_negative AS counttestspasseddiffn,
329+
tstatusfailed_diff.difference_positive AS countteststimestatusfaileddiffp,
330+
tstatusfailed_diff.difference_negative AS countteststimestatusfaileddiffn,
331+
(SELECT count(buildid) FROM build2note WHERE buildid=b.id) AS countnotes,
332+
(SELECT count(buildid) FROM comments WHERE buildid=b.id) AS countcomments,
333+
s.name AS sitename,
334+
s.outoforder AS siteoutoforder,
335+
b.stamp,
336+
b.name,
337+
b.type,
338+
b.generator,
339+
b.starttime,
340+
b.endtime,
341+
b.submittime,
342+
b.configureerrors AS countconfigureerrors,
343+
b.configurewarnings AS countconfigurewarnings,
344+
b.builderrors AS countbuilderrors,
345+
b.buildwarnings AS countbuildwarnings,
346+
b.buildduration,
347+
b.testnotrun AS counttestsnotrun,
348+
b.testfailed AS counttestsfailed,
349+
b.testpassed AS counttestspassed,
350+
b.testtimestatusfailed AS countteststimestatusfailed,
351+
cs.loctested,
352+
cs.locuntested,
353+
cs.loctesteddiff,
354+
cs.locuntesteddiff,
355+
das.checker,
356+
das.numdefects,
357+
sp.id AS subprojectid,
358+
sp.groupid AS subprojectgroup,
359+
sp.position AS subprojectposition,
360+
g.name AS groupname,
361+
gp.position,
362+
g.id AS groupid,
363+
(SELECT count(buildid) FROM label2build WHERE buildid=b.id) AS numlabels,
364+
(SELECT count(buildid) FROM build2uploadfile WHERE buildid=b.id) AS builduploadfiles
365+
';
366+
}
367+
368+
public function getIndexJoin(): string
369+
{
370+
return '
371+
FROM build AS b
372+
LEFT JOIN build2group AS b2g ON (b2g.buildid=b.id)
373+
LEFT JOIN buildgroup AS g ON (g.id=b2g.groupid)
374+
LEFT JOIN buildgroupposition AS gp ON (gp.buildgroupid=g.id)
375+
LEFT JOIN site AS s ON (s.id=b.siteid)
376+
LEFT JOIN buildupdate AS bu ON (b.updateid=bu.id)
377+
LEFT JOIN coveragesummary AS cs ON (cs.buildid=b.id)
378+
LEFT JOIN dynamicanalysissummary AS das ON (das.buildid=b.id)
379+
LEFT JOIN builderrordiff AS be_diff ON (be_diff.buildid=b.id AND be_diff.type=0)
380+
LEFT JOIN builderrordiff AS bw_diff ON (bw_diff.buildid=b.id AND bw_diff.type=1)
381+
LEFT JOIN configureerrordiff AS ce_diff ON (ce_diff.buildid=b.id AND ce_diff.type=1)
382+
LEFT JOIN buildtesttime AS btt ON (btt.buildid=b.id)
383+
LEFT JOIN testdiff AS tnotrun_diff ON (tnotrun_diff.buildid=b.id AND tnotrun_diff.type=0)
384+
LEFT JOIN testdiff AS tfailed_diff ON (tfailed_diff.buildid=b.id AND tfailed_diff.type=1)
385+
LEFT JOIN testdiff AS tpassed_diff ON (tpassed_diff.buildid=b.id AND tpassed_diff.type=2)
386+
LEFT JOIN testdiff AS tstatusfailed_diff ON (tstatusfailed_diff.buildid=b.id AND tstatusfailed_diff.type=3)
387+
LEFT JOIN subproject as sp ON (b.subprojectid = sp.id)
388+
';
389+
}
390+
268391
// Encapsulate this monster query so that it is not duplicated between
269392
// index.php and get_dynamic_builds.
270393
public function getIndexQuery(): string
271394
{
272-
return
273-
'SELECT b.id,b.siteid,b.parentid,b.done,b.changeid,b.testduration,
274-
bu.status AS updatestatus,
275-
b.osname AS osname,
276-
bu.starttime AS updatestarttime,
277-
bu.endtime AS updateendtime,
278-
bu.nfiles AS countupdatefiles,
279-
bu.warnings AS countupdatewarnings,
280-
bu.revision,
281-
b.configureduration,
282-
be_diff.difference_positive AS countbuilderrordiffp,
283-
be_diff.difference_negative AS countbuilderrordiffn,
284-
bw_diff.difference_positive AS countbuildwarningdiffp,
285-
bw_diff.difference_negative AS countbuildwarningdiffn,
286-
ce_diff.difference AS countconfigurewarningdiff,
287-
btt.time AS testtime,
288-
tnotrun_diff.difference_positive AS counttestsnotrundiffp,
289-
tnotrun_diff.difference_negative AS counttestsnotrundiffn,
290-
tfailed_diff.difference_positive AS counttestsfaileddiffp,
291-
tfailed_diff.difference_negative AS counttestsfaileddiffn,
292-
tpassed_diff.difference_positive AS counttestspasseddiffp,
293-
tpassed_diff.difference_negative AS counttestspasseddiffn,
294-
tstatusfailed_diff.difference_positive AS countteststimestatusfaileddiffp,
295-
tstatusfailed_diff.difference_negative AS countteststimestatusfaileddiffn,
296-
(SELECT count(buildid) FROM build2note WHERE buildid=b.id) AS countnotes,
297-
(SELECT count(buildid) FROM comments WHERE buildid=b.id) AS countcomments,
298-
s.name AS sitename,
299-
s.outoforder AS siteoutoforder,
300-
b.stamp,b.name,b.type,b.generator,b.starttime,b.endtime,b.submittime,
301-
b.configureerrors AS countconfigureerrors,
302-
b.configurewarnings AS countconfigurewarnings,
303-
b.builderrors AS countbuilderrors,
304-
b.buildwarnings AS countbuildwarnings,
305-
b.buildduration,
306-
b.testnotrun AS counttestsnotrun,
307-
b.testfailed AS counttestsfailed,
308-
b.testpassed AS counttestspassed,
309-
b.testtimestatusfailed AS countteststimestatusfailed,
310-
cs.loctested, cs.locuntested,
311-
cs.loctesteddiff,
312-
cs.locuntesteddiff,
313-
das.checker, das.numdefects,
314-
sp.id AS subprojectid,
315-
sp.groupid AS subprojectgroup,
316-
sp.position AS subprojectposition,
317-
g.name AS groupname,gp.position,g.id AS groupid,
318-
(SELECT count(buildid) FROM label2build WHERE buildid=b.id) AS numlabels,
319-
(SELECT count(buildid) FROM build2uploadfile WHERE buildid=b.id) AS builduploadfiles
320-
FROM build AS b
321-
LEFT JOIN build2group AS b2g ON (b2g.buildid=b.id)
322-
LEFT JOIN buildgroup AS g ON (g.id=b2g.groupid)
323-
LEFT JOIN buildgroupposition AS gp ON (gp.buildgroupid=g.id)
324-
LEFT JOIN site AS s ON (s.id=b.siteid)
325-
LEFT JOIN buildupdate AS bu ON (b.updateid=bu.id)
326-
LEFT JOIN coveragesummary AS cs ON (cs.buildid=b.id)
327-
LEFT JOIN dynamicanalysissummary AS das ON (das.buildid=b.id)
328-
LEFT JOIN builderrordiff AS be_diff ON (be_diff.buildid=b.id AND be_diff.type=0)
329-
LEFT JOIN builderrordiff AS bw_diff ON (bw_diff.buildid=b.id AND bw_diff.type=1)
330-
LEFT JOIN configureerrordiff AS ce_diff ON (ce_diff.buildid=b.id AND ce_diff.type=1)
331-
LEFT JOIN buildtesttime AS btt ON (btt.buildid=b.id)
332-
LEFT JOIN testdiff AS tnotrun_diff ON (tnotrun_diff.buildid=b.id AND tnotrun_diff.type=0)
333-
LEFT JOIN testdiff AS tfailed_diff ON (tfailed_diff.buildid=b.id AND tfailed_diff.type=1)
334-
LEFT JOIN testdiff AS tpassed_diff ON (tpassed_diff.buildid=b.id AND tpassed_diff.type=2)
335-
LEFT JOIN testdiff AS tstatusfailed_diff ON (tstatusfailed_diff.buildid=b.id AND tstatusfailed_diff.type=3)
336-
LEFT JOIN subproject as sp ON (b.subprojectid = sp.id)';
395+
return $this->getIndexSelect() . $this->getIndexJoin();
337396
}
338397

339398
public function populateBuildRow(array $build_row): array

‎phpstan-baseline.neon‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6456,7 +6456,7 @@ parameters:
64566456
-
64576457
rawMessage: 'Construct empty() is not allowed. Use more strict comparison.'
64586458
identifier: empty.notAllowed
6459-
count: 16
6459+
count: 18
64606460
path: app/cdash/app/Controller/Api/Index.php
64616461

64626462
-

0 commit comments

Comments
 (0)