Skip to content

Commit 5f1f757

Browse files
committed
Claude proposed changes
1 parent 7b14237 commit 5f1f757

File tree

2 files changed

+198
-33
lines changed

2 files changed

+198
-33
lines changed

runner/main/jobtypes/performance/performance.sh

Lines changed: 167 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
# Performance needed variables to go to the env file.
2121
function performance_to_env_file() {
2222
local env=(
23+
JOBTYPE
2324
DBTYPE
2425
DBTAG
2526
DBHOST
@@ -47,6 +48,12 @@ function performance_to_summary() {
4748
echo "== MOODLE_CONFIG: ${MOODLE_CONFIG}"
4849
echo "== PLUGINSTOINSTALL: ${PLUGINSTOINSTALL}"
4950
echo "== SITESIZE: ${SITESIZE}"
51+
echo "== PERF_USERS: ${PERF_USERS}"
52+
echo "== PERF_LOOPS: ${PERF_LOOPS}"
53+
echo "== PERF_RAMPUP: ${PERF_RAMPUP}"
54+
echo "== PERF_THROUGHPUT: ${PERF_THROUGHPUT}"
55+
echo "== PERF_BASELINE_FILE: ${PERF_BASELINE_FILE:-<none>}"
56+
echo "== PERF_THRESHOLD_PCT: ${PERF_THRESHOLD_PCT}%"
5057
echo "== TARGET_FILE: ${TARGET_FILE}"
5158
}
5259

@@ -99,6 +106,43 @@ function performance_config() {
99106

100107
# Default target file (relative to WORKSPACE) where rundata.json will be stored.
101108
export TARGET_FILE="${TARGET_FILE:-storage/performance/${MOODLE_BRANCH}/rundata.json}"
109+
110+
# Optional baseline file for regression comparison.
111+
# If set, the teardown will compare current results against this baseline.
112+
export PERF_BASELINE_FILE="${PERF_BASELINE_FILE:-}"
113+
114+
# Percentage threshold for flagging a regression (default 20%).
115+
export PERF_THRESHOLD_PCT="${PERF_THRESHOLD_PCT:-20}"
116+
117+
# Derive JMeter run parameters from SITESIZE.
118+
# These arrays match the ones in the generator.php plugin and Moodle core.
119+
# XS S M L XL XXL
120+
local -a _users=( 1 30 100 1000 5000 10000 )
121+
local -a _loops=( 5 5 5 6 6 7 )
122+
local -a _rampups=( 1 6 40 100 500 800 )
123+
local -a _throughput=(120 120 120 120 120 120 )
124+
125+
local sizeindex
126+
sizeindex=$(performance_size_to_index "${SITESIZE}")
127+
128+
export PERF_USERS="${_users[$sizeindex]}"
129+
export PERF_LOOPS="${_loops[$sizeindex]}"
130+
export PERF_RAMPUP="${_rampups[$sizeindex]}"
131+
export PERF_THROUGHPUT="${_throughput[$sizeindex]}"
132+
}
133+
134+
# Convert a SITESIZE name (XS, S, M, L, XL, XXL) to a numeric index (0-5).
135+
function performance_size_to_index() {
136+
local size="${1:-XS}"
137+
case "${size}" in
138+
XS) echo 0 ;;
139+
S) echo 1 ;;
140+
M) echo 2 ;;
141+
L) echo 3 ;;
142+
XL) echo 4 ;;
143+
XXL) echo 5 ;;
144+
*) echo 0 ;; # Default to XS.
145+
esac
102146
}
103147

104148
# Performance job type setup for normal mode.
@@ -109,7 +153,8 @@ function performance_setup() {
109153
echo "============================================================================"
110154

111155
# Ensure host shared directories exist and are writable so plugin can save files.
112-
mkdir -p "${SHAREDDIR}/output/logs" "${SHAREDDIR}/output/runs"
156+
# Note: runs_samples is needed by the JMeter SimpleDataWriter in the test plan.
157+
mkdir -p "${SHAREDDIR}/output/logs" "${SHAREDDIR}/output/runs" "${SHAREDDIR}/runs_samples"
113158
chmod -R 2777 "${SHAREDDIR}"
114159

115160
# Run the moodle install_database.php.
@@ -145,7 +190,7 @@ function performance_run() {
145190

146191
group="${MOODLE_BRANCH}"
147192
description="${GIT_COMMIT}"
148-
siteversion=""
193+
siteversion="${MOODLE_BRANCH}"
149194
sitebranch="${MOODLE_BRANCH}"
150195
sitecommit="${GIT_COMMIT}"
151196
runoutput="${SHAREDDIR}/output/logs/run.log"
@@ -215,6 +260,112 @@ function performance_teardown() {
215260
# Copy the formatted rundata.json to the target path.
216261
echo "Copying formatted rundata.json to ${targetpath}"
217262
cp -f "${DATADIR}/rundata.json" "${targetpath}"
263+
264+
# --- Regression detection ---
265+
# If a baseline file is provided, compare the current results against it.
266+
if [[ -n "${PERF_BASELINE_FILE}" ]]; then
267+
local baselinepath
268+
if [[ "${PERF_BASELINE_FILE}" = /* ]]; then
269+
baselinepath="${PERF_BASELINE_FILE}"
270+
else
271+
baselinepath="${WORKSPACE}/${PERF_BASELINE_FILE}"
272+
fi
273+
274+
if [[ -f "${baselinepath}" ]]; then
275+
echo
276+
echo ">>> startsection Regression comparison <<<"
277+
echo "============================================================================"
278+
echo "Comparing current results against baseline: ${baselinepath}"
279+
echo "Threshold: ${PERF_THRESHOLD_PCT}%"
280+
281+
# Use a PHP one-liner to compute median response times per sampler and compare.
282+
local comparison_result
283+
comparison_result=$(docker run --rm \
284+
-v "${DATADIR}:/current" \
285+
-v "$(dirname "${baselinepath}"):/baseline" \
286+
php:8.3-cli \
287+
php -r '
288+
$baseline = json_decode(file_get_contents("/baseline/" . basename($argv[1])), true);
289+
$current = json_decode(file_get_contents("/current/rundata.json"), true);
290+
$threshold = (float)$argv[2];
291+
292+
if (!$baseline || !$current) {
293+
echo "ERROR: Could not parse JSON files.\n";
294+
exit(2);
295+
}
296+
297+
// Flatten results: group response times by sampler name.
298+
function collect_times($results) {
299+
$times = [];
300+
foreach ($results as $thread) {
301+
if (!is_array($thread)) continue;
302+
foreach ($thread as $sample) {
303+
$name = trim($sample["name"] ?? "");
304+
if ($name === "") continue;
305+
$times[$name][] = (float)($sample["time"] ?? 0);
306+
}
307+
}
308+
return $times;
309+
}
310+
311+
function median($arr) {
312+
sort($arr);
313+
$n = count($arr);
314+
if ($n === 0) return 0;
315+
$mid = (int)($n / 2);
316+
return ($n % 2 === 0) ? ($arr[$mid - 1] + $arr[$mid]) / 2 : $arr[$mid];
317+
}
318+
319+
$base_times = collect_times($baseline["results"] ?? []);
320+
$curr_times = collect_times($current["results"] ?? []);
321+
322+
$regressions = [];
323+
$all_ok = true;
324+
325+
// Only compare samplers from the main "Moodle Test" thread group (skip warm-up).
326+
// The warm-up results are in lower-numbered indices; main test results follow.
327+
// We compare all samplers present in both runs.
328+
foreach ($curr_times as $name => $ctimes) {
329+
if (!isset($base_times[$name])) continue;
330+
$base_median = median($base_times[$name]);
331+
$curr_median = median($ctimes);
332+
if ($base_median <= 0) continue;
333+
334+
$pct_change = (($curr_median - $base_median) / $base_median) * 100;
335+
$status = ($pct_change > $threshold) ? "REGRESSION" : "OK";
336+
if ($status === "REGRESSION") {
337+
$all_ok = false;
338+
$regressions[] = $name;
339+
}
340+
341+
printf(" %-35s base=%6.1fms curr=%6.1fms change=%+.1f%% [%s]\n",
342+
$name, $base_median, $curr_median, $pct_change, $status);
343+
}
344+
345+
if ($all_ok) {
346+
echo "\nResult: PASS — No performance regressions detected.\n";
347+
exit(0);
348+
} else {
349+
echo "\nResult: FAIL — Performance regressions detected in: " . implode(", ", $regressions) . "\n";
350+
exit(1);
351+
}
352+
' "$(basename "${baselinepath}")" "${PERF_THRESHOLD_PCT}")
353+
local comparison_exit=$?
354+
355+
echo "${comparison_result}"
356+
echo "============================================================================"
357+
echo ">>> stopsection <<<"
358+
359+
if [[ ${comparison_exit} -eq 1 ]]; then
360+
echo "Performance regression detected — marking build as FAILED."
361+
EXITCODE=1
362+
elif [[ ${comparison_exit} -ge 2 ]]; then
363+
echo "WARNING: Regression comparison encountered an error (exit code ${comparison_exit})."
364+
fi
365+
else
366+
echo "Baseline file not found: ${baselinepath} — skipping regression comparison."
367+
fi
368+
fi
218369
}
219370

220371
# Returns the command to install the performance site.
@@ -236,12 +387,18 @@ function performance_initcmd() {
236387
function performance_perftoolcmd() {
237388
local -n cmd=$1
238389

239-
# Build the complete init command.
390+
# Build the complete command to generate test data and plan files.
391+
# Note: --bypasscheck is needed because CI may not have debugdeveloper set at the point
392+
# where the generator checks it. --quiet is intentionally omitted to see progress output.
393+
# Note: --updateuserspassword ensures the users' passwords in the database match the
394+
# password written to the CSV ($CFG->tool_generator_users_password), which is
395+
# critical for JMeter to be able to login as those users.
240396
cmd=(
241397
php public/local/performancetool/generate_test_data.php \
242398
--size="${SITESIZE}" \
243399
--planfilespath="/shared" \
244-
--quiet="false"
400+
--bypasscheck \
401+
--updateuserspassword
245402
)
246403
}
247404

@@ -256,10 +413,8 @@ function performance_main_command() {
256413
includelogsstr="-Jincludelogs=$includelogs"
257414
samplerinitstr="-Jbeanshell.listener.init=recorderfunctions.bsf"
258415

259-
260-
# TODO: Get all of these values from somewhere?
261-
# In particular where to get users, loops, rampup, and throughput from?
262-
# Build the complete perf command for the run.
416+
# Users, loops, rampup, and throughput are derived from SITESIZE in performance_config().
417+
# They match the arrays baked into generator.php and the JMX template defaults.
263418
_cmd=(
264419
-n \
265420
-j "/shared/output/logs/jmeter.log" \
@@ -270,7 +425,10 @@ function performance_main_command() {
270425
-Jsiteversion="$siteversion" \
271426
-Jsitebranch="$sitebranch" \
272427
-Jsitecommit="$sitecommit" \
273-
-Jusers=5 -Jloops=1 -Jrampup=1 -Jthroughput=120 \
428+
-Jusers="${PERF_USERS}" \
429+
-Jloops="${PERF_LOOPS}" \
430+
-Jrampup="${PERF_RAMPUP}" \
431+
-Jthroughput="${PERF_THROUGHPUT}" \
274432
$samplerinitstr $includelogsstr
275433
)
276434
}

runner/main/modules/docker-jmeter/libraries/recorder.bsf

Lines changed: 31 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -11,69 +11,75 @@ MoodleResult(JMeterContext ctx) {
1111

1212
String html = result.getResponseDataAsString();
1313

14-
String dbreads = "0";
15-
Pattern pdbreads = Pattern.compile(".*?DB reads/writes: (\\d+)/\\d+.*", Pattern.UNIX_LINES | Pattern.DOTALL);
16-
Matcher mdbreads = pdbreads.matcher(html);
17-
if (mdbreads.matches()) {
18-
dbreads = mdbreads.group(1);
19-
}
14+
// Extract Moodle server-side performance metrics from the HTML response body.
15+
// Modern Moodle (4.x+/Boost) renders these inside <div id="performanceinfo"><ul>
16+
// as <li class="...">LABEL: VALUE</li>. Older themes output plain text.
17+
// Using find() instead of matches() so the pattern doesn't need to match the entire string.
2018

19+
String dbreads = "0";
2120
String dbwritesstr = "0";
22-
Pattern pdbwrites = Pattern.compile(".*?DB reads/writes: \\d+/(\\d+).*", Pattern.UNIX_LINES | Pattern.DOTALL);
23-
Matcher mdbwrites = pdbwrites.matcher(html);
24-
if (mdbwrites.matches()) {
25-
dbwritesstr = mdbwrites.group(1);
21+
// Match "DB reads/writes: X/Y" (works for both old plain-text and modern HTML).
22+
Pattern pdbrw = Pattern.compile("DB reads/writes:\\s*(\\d+)/(\\d+)");
23+
Matcher mdbrw = pdbrw.matcher(html);
24+
if (mdbrw.find()) {
25+
dbreads = mdbrw.group(1);
26+
dbwritesstr = mdbrw.group(2);
2627
}
2728
Integer dbwrites = Integer.parseInt(dbwritesstr);
2829

2930
// Adding logs if required.
3031
if (props.get("includelogs") != null) {
31-
Pattern plogwrites = Pattern.compile(".*?Log DB writes (\\d+).*", Pattern.UNIX_LINES | Pattern.DOTALL);
32+
Pattern plogwrites = Pattern.compile("Log DB writes\\s*(\\d+)");
3233
Matcher mlogwrites = plogwrites.matcher(html);
33-
if (mlogwrites.matches()) {
34+
if (mlogwrites.find()) {
3435
dbwrites = dbwrites + Integer.parseInt(mlogwrites.group(1));
3536
}
3637
}
3738

3839
String dbquerytime = "0";
39-
Pattern pdbquerytime = Pattern.compile(".*?DB queries time: (\\d+(\\.\\d+)?) secs.*", Pattern.UNIX_LINES | Pattern.DOTALL);
40+
Pattern pdbquerytime = Pattern.compile("DB queries time:\\s*(\\d+(?:\\.\\d+)?)\\s*secs");
4041
Matcher mdbquerytime = pdbquerytime.matcher(html);
41-
if (mdbquerytime.matches()) {
42+
if (mdbquerytime.find()) {
4243
dbquerytime = mdbquerytime.group(1);
4344
}
4445

4546
String memoryused = "0";
46-
Pattern pmemoryused = Pattern.compile(".*?RAM: (\\d+(\\.\\d+)?)[^M]*MB.*", Pattern.UNIX_LINES | Pattern.DOTALL);
47+
// Note: Moodle uses a non-breaking space (\u00A0) between the number and MB.
48+
Pattern pmemoryused = Pattern.compile("RAM:[\\s\\u00A0]*(\\d+(?:\\.\\d+)?)[\\s\\u00A0]*MB");
4749
Matcher mmemoryused = pmemoryused.matcher(html);
48-
if (mmemoryused.matches()) {
50+
if (mmemoryused.find()) {
4951
memoryused = mmemoryused.group(1);
5052
}
5153

5254
String filesincluded = "0";
53-
Pattern pfilesincluded = Pattern.compile(".*?Included (\\d+) files.*", Pattern.UNIX_LINES | Pattern.DOTALL);
55+
Pattern pfilesincluded = Pattern.compile("Included\\s*(\\d+)\\s*files");
5456
Matcher mfilesincluded = pfilesincluded.matcher(html);
55-
if (mfilesincluded.matches()) {
57+
if (mfilesincluded.find()) {
5658
filesincluded = mfilesincluded.group(1);
5759
}
5860

5961
String serverload = "0";
60-
Pattern pserverload = Pattern.compile(".*?Load average: (\\d+(\\.\\d+)?).*", Pattern.UNIX_LINES | Pattern.DOTALL);
62+
Pattern pserverload = Pattern.compile("Load average:\\s*(\\d+(?:\\.\\d+)?)");
6163
Matcher mserverload = pserverload.matcher(html);
62-
if (mserverload.matches()) {
64+
if (mserverload.find()) {
6365
serverload = mserverload.group(1);
6466
}
6567

6668
String sessionsize = "0";
67-
Pattern psessionsize = Pattern.compile(".*?Session[^:]*: (\\d+(\\.\\d+)? ?[a-zA-Z]*).*", Pattern.UNIX_LINES | Pattern.DOTALL);
69+
// Match "Session (read): X KB" or "Session (core\session\file): 569 bytes" etc.
70+
// Note: Moodle uses non-breaking space (\u00A0) between number and unit.
71+
Pattern psessionsize = Pattern.compile("Session[^:]*:[\\s\\u00A0]*(\\d+(?:\\.\\d+)?[\\s\\u00A0]*[a-zA-Z]*)");
6872
Matcher msessionsize = psessionsize.matcher(html);
69-
if (msessionsize.matches()) {
73+
if (msessionsize.find()) {
7074
sessionsize = msessionsize.group(1);
7175
}
7276

7377
String timeused = "0";
74-
Pattern ptimeused = Pattern.compile(".*?\"timeused[^\"]*\">(\\d+(\\.\\d+)?) secs.*", Pattern.UNIX_LINES | Pattern.DOTALL);
78+
// Match both HTML-wrapped: <span class="timeused">X.XX secs</span>
79+
// and the older: class="timeused...">X.XX secs
80+
Pattern ptimeused = Pattern.compile("timeused[^>]*>\\s*(\\d+(?:\\.\\d+)?)\\s*secs");
7581
Matcher mtimeused = ptimeused.matcher(html);
76-
if (mtimeused.matches()) {
82+
if (mtimeused.find()) {
7783
timeused = mtimeused.group(1);
7884
}
7985

@@ -198,6 +204,7 @@ if (JMeterUtils.getProperty("headerprinted") == null) {
198204
JMeterUtils.setProperty("headerprinted", "1");
199205
print(mr.headerToString());
200206

207+
201208
WritePhpValue("starttime", (System.currentTimeMillis() / 1000L).toString());
202209
WritePhpValue("host", vars.get("host"));
203210
WritePhpValue("sitepath", vars.get("sitepath"));

0 commit comments

Comments
 (0)