Skip to content

Commit f3f1288

Browse files
Improve test creation performance (#3518)
Test insertion is the most performance-critical part of CDash, simply because there are often 2-4 orders of magnitude more tests than builds. Each of those tests then has an additional order of magnitude more measurements and/or labels associated with it. This PR improves two things: 1. Only the test output ID is queried instead of the entire model. This reduces the amount of network I/O to the database. 2. The entire test creation process is wrapped in a transaction. In addition to ensuring consistency, this also avoids the overhead of committing after many small transactions. There's more to be done here, particularly relating to bulk inserts of labels, measurements, and images, but some substantial refactoring needs to be done first.
1 parent 63343e3 commit f3f1288

2 files changed

Lines changed: 39 additions & 44 deletions

File tree

app/Utils/TestCreator.php

Lines changed: 39 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
use CDash\Model\Build;
2525
use CDash\Model\Image;
2626
use ErrorException;
27+
use Illuminate\Support\Facades\DB;
2728
use Illuminate\Support\Facades\Log;
2829

2930
/**
@@ -136,46 +137,46 @@ public function create(Build $build): void
136137
}
137138
}
138139

139-
$outputid = TestOutput::firstOrCreate([
140-
'path' => $this->testPath,
141-
'command' => $this->testCommand,
142-
'output' => $this->testOutput,
143-
])->id;
144-
145-
// build2test
146-
$buildtest = new Test();
147-
$buildtest->buildid = $build->Id;
148-
$buildtest->outputid = $outputid;
149-
$buildtest->status = $this->testStatus;
150-
$buildtest->details = $this->testDetails;
151-
$buildtest->time = "$this->buildTestTime";
152-
$buildtest->testname = $this->testName;
153-
$buildtest->starttime = $this->testStartTime;
154-
155-
// Note: the newstatus column is currently handled in
156-
// ctestparserutils::compute_test_difference. This gets updated when we call
157-
// Build::ComputeTestTiming.
158-
$buildtest->save();
159-
160-
foreach ($this->measurements as $measurement) {
161-
$measurement->testid = $buildtest->id;
162-
$measurement->save();
163-
}
140+
DB::transaction(function () use ($build): void {
141+
$outputid = TestOutput::select('id')->firstOrCreate([
142+
'path' => $this->testPath,
143+
'command' => $this->testCommand,
144+
'output' => $this->testOutput,
145+
])->id;
146+
147+
// Note: the newstatus column is currently handled in
148+
// ctestparserutils::compute_test_difference. This gets updated when we call
149+
// Build::ComputeTestTiming.
150+
$buildtest = Test::create([
151+
'buildid' => $build->Id,
152+
'outputid' => $outputid,
153+
'status' => $this->testStatus,
154+
'details' => $this->testDetails,
155+
'time' => "$this->buildTestTime",
156+
'testname' => $this->testName,
157+
'starttime' => $this->testStartTime,
158+
]);
159+
160+
foreach ($this->measurements as $measurement) {
161+
$measurement->testid = $buildtest->id;
162+
$measurement->save();
163+
}
164164

165-
// Give measurements to the buildtest model so we can properly calculate
166-
// proctime later on.
167-
$buildtest->measurements = $this->measurements;
168-
$build->AddTest($buildtest);
165+
// Give measurements to the buildtest model so we can properly calculate
166+
// proctime later on.
167+
$buildtest->measurements = $this->measurements;
168+
$build->AddTest($buildtest);
169169

170-
foreach ($this->labels as $label) {
171-
$label->Test = $buildtest;
172-
$label->Insert();
173-
$buildtest->addLabel($label);
174-
}
170+
foreach ($this->labels as $label) {
171+
$label->Test = $buildtest;
172+
$label->Insert();
173+
$buildtest->addLabel($label);
174+
}
175175

176-
// test2image
177-
foreach ($this->images as $image) {
178-
$this->saveImage($image, $buildtest->id);
179-
}
176+
// test2image
177+
foreach ($this->images as $image) {
178+
$this->saveImage($image, $buildtest->id);
179+
}
180+
});
180181
}
181182
}

phpstan-baseline.neon

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7062,12 +7062,6 @@ parameters:
70627062
count: 1
70637063
path: app/Utils/TestCreator.php
70647064

7065-
-
7066-
rawMessage: 'Property App\Models\Test::$time (float) does not accept string.'
7067-
identifier: assign.propertyType
7068-
count: 1
7069-
path: app/Utils/TestCreator.php
7070-
70717065
-
70727066
rawMessage: Property App\Utils\TestCreator::$alreadyCompressed has no type specified.
70737067
identifier: missingType.property

0 commit comments

Comments
 (0)