This is an automated email from the ASF dual-hosted git repository.
jamesfredley pushed a commit to branch perf/8.0.x-jmh-pr-benchmarks
in repository https://gitbox.apache.org/repos/asf/grails-core.git
The following commit(s) were added to refs/heads/perf/8.0.x-jmh-pr-benchmarks
by this push:
new b06d51784c Close JMH metadata stream and run comparison tests in CI
b06d51784c is described below
commit b06d51784c18781c93dda2391d5f72e95aa305ff
Author: t <t@t>
AuthorDate: Thu Jul 30 16:10:56 2026 -0400
Close JMH metadata stream and run comparison tests in CI
Address review feedback on the JMH benchmark reporting workflow.
The classpath metadata merge read line-oriented entries with
getInputStream(entry).getText(...) and never closed the stream, while
the two neighbouring Properties reads already used withCloseable. That
branch handles every META-INF/services and META-INF/groovy entry across
the whole benchmark runtime classpath, so it was the most frequently
executed of the three and could leak file handles across a large
dependency set. It now closes the stream like the others; the merged
output is unchanged.
The comparison script's unit tests were not executed anywhere, leaving
the reporting logic unguarded against regressions. They now run in the
benchmark job immediately after checkout, before the JDK and Gradle
setup and before the paired build and measurement steps, so a broken
reporter fails within seconds rather than after an hour of benchmarking.
Running them in the shard job also covers pull requests from forks,
which never reach the reporting job.
That step is deliberately permitted to fail the job. The workflow's
advisory-only rule applies to performance regressions; a comparison
script that does not pass its own tests invalidates every number the run
produces, so it should stop the run rather than publish results.
Assisted-by: claude-code:claude-opus-5
---
.github/workflows/benchmark.yml | 5 +++++
grails-benchmarks/build.gradle | 8 +++++---
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/.github/workflows/benchmark.yml b/.github/workflows/benchmark.yml
index 301c086c2a..655e7b69d1 100644
--- a/.github/workflows/benchmark.yml
+++ b/.github/workflows/benchmark.yml
@@ -63,6 +63,11 @@ jobs:
uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd #
v6.0.2
with:
fetch-depth: 0
+ # Deliberately allowed to fail the job, and placed before the expensive
work: a broken
+ # comparison script makes every number produced here untrustworthy. That
is a tooling
+ # failure rather than a performance finding, so regressions themselves
stay advisory.
+ - name: "🧪 Verify JMH comparison script"
+ run: python3 .github/scripts/test_jmh_compare.py
- name: "☕️ Setup JDK"
uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 #
v5.2.0
with:
diff --git a/grails-benchmarks/build.gradle b/grails-benchmarks/build.gradle
index 86a7e09f2e..56489e2659 100644
--- a/grails-benchmarks/build.gradle
+++ b/grails-benchmarks/build.gradle
@@ -152,9 +152,11 @@ def mergeJmhClasspathMetadata =
tasks.register('mergeJmhClasspathMetadata') {
}
} else if (lineMetadataPrefixes.any { String
prefix -> path.startsWith(prefix) }) {
Set<String> lines =
lineEntries.computeIfAbsent(path) { new TreeSet<>() }
-
zipFile.getInputStream(entry).getText('UTF-8').readLines().each { String line ->
- if (line) {
- lines.add(line)
+
zipFile.getInputStream(entry).withCloseable { input ->
+
input.getText('UTF-8').readLines().each { String line ->
+ if (line) {
+ lines.add(line)
+ }
}
}
} else if
(propertiesMetadataPaths.contains(path)) {
