You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@grails.apache.org by "jamesfredley (via GitHub)" <gi...@apache.org> on 2026/07/30 18:30:15 UTC
[PR] Add JMH performance regression reporting for pull requests [grails-core]
jamesfredley opened a new pull request, #16071:
URL: https://github.com/apache/grails-core/pull/16071
## Why
Today a performance regression in Grails just ships. There is no gate, no signal, no attribution. It surfaces months later as "our app got slower after upgrading", by which point nobody can point at the commit responsible. Every framework change is an unmeasured bet.
Apache Groovy solved this for themselves and it works well: every PR gets an automated JMH report. This brings the same capability to Grails, and improves on the parts of their design that are known to be weakest.
Worth saying plainly, because it is easy to assume we are catching up: **Micronaut, Quarkus and OpenJDK have no PR-level performance check at all.** Groovy is the outlier that does. This puts Grails in that second group.
## What a reviewer actually sees
Label a PR `performance`, and a comment like this appears. This is a **real report from a real run** - a 2x slowdown was deliberately injected into `matchWarmCache` to produce it:
> **Regressions:** 1 **Improvements:** 0
> **Runner health:** worst ruler deviation: 11.3%
> **Ruler movements:** `CpuRulerBenchmark.integerArithmetic: 0.89x`, `MemoryRulerBenchmark.allocateAndCopyArray: 1.08x`
>
> β οΈ **Warning:** The runner was unstable BETWEEN the two halves of the A/B run. Treat results as unreliable.
| Benchmark | Base score | Head score | Speedup | Verdict | Allocation (ADVISORY) |
| --- | ---: | ---: | ---: | --- | ---: |
| urlmappings.UrlMappingsBenchmark.matchWarmCache | 4.07 ns/op | 8.77 ns/op | **0.46x** | **REGRESSED** | ~0 B/op |
| databinding.SimpleDataBinderBenchmark.bindFlatMap | 1.72e+04 ns/op | 1.83e+04 ns/op | 0.94x | no clear change | +17 B/op |
| gsp.GroovyPageParserBenchmark.parseSmallTemplate | 9.94e+03 ns/op | 9.98e+03 ns/op | 1.00x | no clear change | +10 B/op |
| interceptors.UrlMappingMatcherBenchmark.matchUriPattern | 128 ns/op | 128 ns/op | 0.99x | no clear change | +12 B/op |
| urlmappings.UrlMappingsBenchmark.matchColdVariedKeys | 818 ns/op | 802 ns/op | 1.02x | no clear change | +14 B/op |
| views.ViewTemplateRenderingBenchmark.renderJsonTemplate | 1.82e+04 ns/op | 1.67e+04 ns/op | 1.09x | no clear change | +12 B/op |
*(abridged - the real comment lists all 13 in a collapsed `<details>` block, regressions sorted to the top)*
**One regression found, zero false positives across the other twelve.** That is the number that decides whether people keep reading these comments or start muting them. A performance check that cries wolf gets ignored within a month, and then it is worse than nothing because it provides false assurance.
Note the report says **"no clear change"**, never "unchanged". Failing to detect a change is not proof there wasn't one, and the wording says so.
## How it decides
A benchmark is only called REGRESSED or IMPROVED when **both** hold:
1. the effect is at least 10%, **and**
2. the JMH confidence intervals for base and head are disjoint.
Everything else is "no clear change". This is deliberately conservative and will miss small real regressions. That is the right trade for a per-PR check whose entire value depends on being trusted.
**What it deliberately does not do:** no bootstrap resampling, no p-values, no Mann-Whitney, no Benjamini-Hochberg FDR. JMH gives ~10 iteration samples sharing a JVM and its compilation state, so they are not independent observations. Significance testing over them is pseudoreplication - it manufactures precision the measurement does not contain. Threshold plus disjoint intervals is defensible; a p-value here would not be.
## Where this improves on Groovy's design
Groovy's system is the direct inspiration, including their idea of pure-Java "ruler" benchmarks. Credit where due. Three things are done differently.
**1. Paired same-runner A/B instead of a historical baseline.**
Groovy compares a PR run against a trailing 90-day baseline on `gh-pages`, measured on *different* runner hardware. They correct for that using rulers as a calibration factor and tolerate Β±15% drift; beyond that their own report states the speedups are not meaningful.
This builds **both** revisions and measures them back-to-back **on the same runner**, so there is no cross-hardware correction to make at all. A 2-shard matrix runs base-then-head and head-then-base so ordering bias cancels, and only **complete same-shard pairs** are pooled into one verdict - a shard that loses either side is dropped rather than cross-paired against a different runner. The pooled interval *spans* the shards rather than narrowing them, so shards that disagree widen uncertainty and make a verdict harder to reach, never easier.
**2. Rulers as a stability check, not a calibration factor.**
Same idea, different job. If any ruler moves more than 5% between the two halves of a run, the report says the runner was unstable and the numbers should not be trusted.
Critically, each ruler is judged **individually**. In the run above the rulers moved 0.89x and 1.08x - which average to ~0.98x and would look perfectly healthy under a geometric mean, while both had in fact moved materially. Averaging them lets opposite movements cancel and silently hides exactly the instability the check exists to catch.
(That warning firing above is honest, not a flaw: it was measured on a working laptop, and the tool said so rather than pretending the numbers were pristine.)
**3. Per-benchmark verdicts, not just group geomeans.**
Groovy reports a geometric mean per group. You learn `core` moved 0.989x; you do not learn which benchmark moved, or whether it was noise. This names the benchmark, gives it a verdict, and marks the group geomean explicitly descriptive rather than a judgement.
Also included: allocation (`gc.alloc.rate.norm`, bytes/op) as an advisory column, often a clearer signal than wall time.
## What is benchmarked
13 benchmarks over paths that run per-request or per-object, all constructible without a Spring context or servlet container:
- **urlmappings** - URI matching (warm cache, cold varied keys), reverse URL creation
- **databinding** - binding a map onto an object, with and without type conversion
- **gsp** - GSP parsing, template text to generated Groovy source
- **interceptors** - interceptor URI match decisions
- **views** - JSON and markup view rendering
- **ruler** - two pure-JDK probes used only to detect an unstable runner
Benchmarks are Java; only the setup fixtures are Groovy. A Groovy-authored benchmark would measure Groovy's dynamic call-site machinery as much as the Grails API under test, so Groovy is confined to building the closure-based fixtures the URL mappings DSL and view templates require.
**Worth knowing what this already caught, before it ever ran in CI.** The URL mappings fixture was initially written with single-quoted strings, so `$category` and `$id` were literal text rather than DSL capture tokens. `match()` returned `null`, and the three most valuable benchmarks were confidently timing *failed lookups* - with no error. Fixing it moved `matchWarmCache` from ~230 ns to **4.6 ns**, against ~780 ns for a cold varied-key match. That ~170x gap is the cache-effectiveness signal this suite exists to defend, and it was completely invisible beforehand. The benchmark now asserts in `@Setup` that the fixture really matches and produces `/catalog/books/42`, and fails the run rather than publishing wrong numbers.
## Cost, and why it is opt-in
This is expensive, so it does **not** run on every PR. It runs only when a PR carries the `performance` label, or via manual `workflow_dispatch`. Doc-only changes are filtered out regardless. Given the ASF-wide Actions budget, defaulting this on for every PR is not a reasonable use of shared CI.
**It is advisory and never fails a build.** A regression is information for the reviewer, not a gate. If we later want to gate, that should be a separate and deliberate discussion.
## Known gap: validation
`Validateable.validate()` belongs here and is **not** included. A benchmark was written, then withdrawn: it measured 80-160% relative error against 15-20% for the pure-JDK rulers on the same idle machine. At that spread the intervals can never separate, so it would report "no clear change" no matter what happened to validation performance, while still burning CI time and contributing a meaningless number to a group geomean.
Two causes were ruled out first - construction was moved into `@Setup`, and errors were reset per invocation (`doValidate()` copies existing errors into the new set, so repeatedly validating a failing object grows it without bound). Neither fixed the variance, which points at bimodal behaviour across forks. `README.adoc` records this with suggested next steps.
Shipping a benchmark that can never produce a verdict seemed worse than shipping an honest gap.
## Security
Uses `pull_request`, **never** `pull_request_target`, so untrusted PR code never runs with a writable token. Fork PRs get the job summary and downloadable artifacts but no comment, because their token is read-only by design. Only the reporting job - which runs solely for same-repository PRs whose authors already have push access - holds `pull-requests: write`. PR-controlled values pass through `env:` rather than into shell, and benchmark names are sanitised before rendering into a comment.
## Verification
- 30 stdlib unit tests for the comparison script; `actionlint` clean; `./gradlew rat` passes with 0 unapproved files
- All 13 benchmarks executed end-to-end, repeatedly, on real hardware
- The full paired 2-shard flow was run against a deliberately injected 2x regression and reported exactly 1 REGRESSED with 0 false positives
- The artifact-pairing logic was separately proven against a simulated tree with an incomplete shard, confirming the incomplete shard is dropped rather than cross-paired
- The `@Setup` fixture guard was proven by reverting the mappings bug and confirming the run fails loudly instead of publishing wrong numbers
- JMH is GPLv2-with-Classpath-Exception (ASF Category X). Acceptable only because the module is build-time-only and never published, which the build enforces and `README.adoc` records for future release managers.
## Follow-ups deliberately out of scope
Long-term trend history on `gh-pages`, a dashboard, gating on regressions, and restoring the validation benchmark. This PR is intentionally the smallest thing that produces a signal worth trusting.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#discussion_r3685314754
##########
grails-benchmarks/build.gradle:
##########
@@ -0,0 +1,238 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.gradle.api.file.DuplicatesStrategy
+import org.gradle.api.tasks.PathSensitivity
+
+import java.util.Properties
+import java.util.zip.ZipFile
+
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'me.champeau.jmh' version '0.7.3'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.compile'
+}
+
+version = projectVersion
+group = 'org.apache.grails'
+
+// This build-time-only benchmark harness is never published, so it deliberately omits
+// org.apache.grails.buildsrc.publish, org.apache.grails.buildsrc.sbom,
+// org.apache.grails.buildsrc.vulnerability-scan, org.apache.grails.gradle.grails-jacoco,
+// org.apache.grails.buildsrc.dependency-validator, and org.apache.grails.gradle.grails-code-style.
+// The latter remains omitted until existing JMH source import-order and fixture-formatting
+// violations can be remediated together.
+
+// The Groovy fixtures under src/main/groovy build the framework objects whose APIs are
+// closure-based (the URL mappings DSL, the Validateable trait, view templates). The JMH
+// benchmarks themselves live in src/jmh/java and only *call* those fixtures, so that the
+// measured code path is plain Java and does not include Groovy's dynamic dispatch.
+// me.champeau.jmh puts the main source set's output on the jmh compile classpath, which is
+// why the fixtures are in main rather than src/jmh/groovy: keeping the benchmarks in a pure
+// Java source set preserves normal JMH annotation processing.
+dependencies {
+ implementation platform(project(':grails-bom'))
+
+ implementation project(':grails-web-url-mappings')
+ implementation project(':grails-databinding-core')
+ implementation project(':grails-gsp-core')
+ implementation project(':grails-interceptors')
+ implementation project(':grails-views-gson')
+ implementation project(':grails-views-markup')
+ implementation project(':grails-core')
+ implementation 'org.apache.groovy:groovy'
+
+ // The framework modules declare these as compileOnly, so they are absent from the runtime
+ // classpath a benchmark actually executes on. The modules' own test suites add them back
+ // the same way. Without them, URL mapping, interceptor and view benchmarks fail at @Setup
+ // with NoClassDefFoundError: jakarta/servlet/ServletContext.
+ implementation 'jakarta.servlet:jakarta.servlet-api'
+ implementation 'org.springframework:spring-test'
+}
+
+// The jmh source set compiles and runs against everything main depends on.
+configurations.jmh.extendsFrom(configurations.implementation)
+
+// Every JMH knob the CI workflow needs to override is exposed as a Gradle property so that
+// .github/workflows/benchmark.yml can run a fast PR profile and a slower scheduled profile
+// from the same build. Defaults here are the quick local-development profile.
+jmh {
+ jmhVersion = '1.37'
+ includes = providers.gradleProperty('jmh.include')
+ .map { it.split(',') as List<String> }
+ .orElse(['.*'])
+ .get()
+ fork = providers.gradleProperty('jmh.forks').map { it as Integer }.orElse(1).get()
+ warmupIterations = providers.gradleProperty('jmh.warmupIterations').map { it as Integer }.orElse(1).get()
+ iterations = providers.gradleProperty('jmh.iterations').map { it as Integer }.orElse(1).get()
+ warmup = providers.gradleProperty('jmh.warmupTime').orElse('1s').get()
+ timeOnIteration = providers.gradleProperty('jmh.iterationTime').orElse('1s').get()
+ resultFormat = providers.gradleProperty('jmh.resultFormat').orElse('JSON').get()
+ // A benchmark that throws during @Setup is otherwise dropped from the results file while
+ // the build still reports success, which would silently shrink the comparison set.
+ failOnError = providers.gradleProperty('jmh.failOnError').map { it.toBoolean() }.orElse(true).get()
+ resultsFile = file(providers.gradleProperty('jmh.resultFile')
+ .orElse(layout.buildDirectory.file('results/jmh/results.json').get().asFile.absolutePath)
+ .get())
+ // Comma-separated JMH profilers, e.g. -Pjmh.profilers=gc for allocation reporting.
+ profilers = providers.gradleProperty('jmh.profilers')
+ .map { it.split(',') as List<String> }
+ .orElse([])
+ .get()
+}
+
+def mergeJmhClasspathMetadata = tasks.register('mergeJmhClasspathMetadata') {
+ def mergedMetadataDirectory = layout.buildDirectory.dir('generated/jmh-classpath-metadata')
+ def lineMetadataPrefixes = ['META-INF/services/', 'META-INF/groovy/']
+ def extensionModuleSuffix = 'org.codehaus.groovy.runtime.ExtensionModule'
+ def propertiesMetadataPaths = [
+ 'META-INF/spring.factories',
+ 'META-INF/spring.handlers',
+ 'META-INF/spring.schemas'
+ ]
+
+ inputs.files(configurations.jmhRuntimeClasspath)
+ .withPropertyName('jmhRuntimeClasspath')
+ .withPathSensitivity(PathSensitivity.RELATIVE)
+ outputs.dir(mergedMetadataDirectory)
+ outputs.cacheIf { true }
+
+ doLast {
+ File outputDirectory = mergedMetadataDirectory.get().asFile
+ project.delete(outputDirectory)
+
+ Map<String, Set<String>> lineEntries = new TreeMap<>()
+ Map<String, Map<String, Set<String>>> extensionModuleEntries = new TreeMap<>()
+ Map<String, Map<String, Set<String>>> propertyEntries = new TreeMap<>()
+
+ configurations.jmhRuntimeClasspath.files
+ .findAll { File artifact -> artifact.name.endsWith('.jar') }
+ .sort { File artifact -> artifact.absolutePath }
+ .each { File artifact ->
+ new ZipFile(artifact).withCloseable { ZipFile zipFile ->
+ zipFile.entries().each { entry ->
+ if (entry.directory) {
+ return
+ }
+
+ String path = entry.name
+ if (path.endsWith(extensionModuleSuffix)) {
+ Properties properties = new Properties()
+ zipFile.getInputStream(entry).withCloseable { input ->
+ properties.load(input)
+ }
+ Map<String, Set<String>> entries = extensionModuleEntries.computeIfAbsent(path) {
+ new TreeMap<>()
+ }
+ ['extensionClasses', 'staticExtensionClasses'].each { String key ->
+ Set<String> values = entries.computeIfAbsent(key) { new TreeSet<>() }
+ properties.getProperty(key, '').split(',').each { String value ->
+ if (value) {
+ values.add(value.trim())
+ }
+ }
+ }
+ } 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)
+ }
+ }
Review Comment:
`mergeJmhClasspathMetadata` reads ZIP entry streams via `zipFile.getInputStream(entry).getText(...)` without closing the stream. Over many JARs/entries this can leak file handles and slow/flake the build on some systems. Wrap the `InputStream` in `withCloseable` (as is already done for the Properties reads) so each entry stream is reliably closed.
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,404 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
+ paths-ignore:
+ - '**/*.md'
+ - '**/*.adoc'
+ - 'grails-doc/**'
+ workflow_dispatch:
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+permissions:
+ contents: read
+
+jobs:
+ # Each shard builds both revisions before measuring either one on the same runner.
+ # Building first avoids CPU, IO, thermal, cache, and frequency state biasing a measurement.
+ # Shard a measures BASE then HEAD, while shard b measures HEAD then BASE.
+ # Alternating the order cancels first-versus-second ordering bias without doubling runtime.
+ # Both shards use the PR merge commit as HEAD, so they measure what would actually land.
+ # Results are advisory: a detected regression never fails this workflow.
+ benchmark:
+ name: "Paired JMH benchmarks (${{ matrix.shard }})"
+ if: ${{ github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'performance') }}
+ runs-on: ubuntu-24.04
+ strategy:
+ fail-fast: false
+ matrix:
+ shard: [a, b]
+ env:
+ BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ HEAD_SHA: ${{ github.sha }}
+ JMH_INCLUDE: '.*'
+ PR_NUMBER: ${{ github.event.pull_request.number || 0 }}
+ REPOSITORY: ${{ github.repository }}
+ RESULT_DIR: ${{ github.workspace }}/jmh-results/${{ matrix.shard }}
+ REPORT_DIR: ${{ github.workspace }}/jmh-reports/${{ matrix.shard }}
+ SHARD: ${{ matrix.shard }}
+ steps:
+ - name: "π₯ Checkout repository"
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+ with:
+ fetch-depth: 0
+ - name: "βοΈ Setup JDK"
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
+ with:
+ distribution: liberica
+ java-version: 21
+ - name: "π Setup Gradle"
+ uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0
+ with:
+ cache-provider: basic # 'basic' uses the MIT-licensed, open-source cache provider; the default 'enhanced' provider (v6+) is proprietary (Gradle commercial Terms of Use)
+ develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+ - name: "π³ Prepare paired worktrees"
+ run: |
+ WORKTREE_ROOT="$RUNNER_TEMP/jmh-worktrees/$SHARD"
+ mkdir -p "$WORKTREE_ROOT" "$RESULT_DIR" "$REPORT_DIR"
+ if [ -n "$BASE_SHA" ] && git cat-file -e "$BASE_SHA^{commit}" 2>/dev/null; then
+ RESOLVED_BASE_SHA="$BASE_SHA"
+ elif RESOLVED_BASE_SHA="$(git merge-base "$HEAD_SHA^1" "$HEAD_SHA^2" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using merge-base $RESOLVED_BASE_SHA."
+ elif RESOLVED_BASE_SHA="$(git rev-parse "$HEAD_SHA^" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using HEAD parent $RESOLVED_BASE_SHA."
+ else
+ RESOLVED_BASE_SHA=""
+ echo "No base commit could be resolved; comparison will use HEAD-only mode."
+ fi
+ git worktree add --detach "$WORKTREE_ROOT/head" "$HEAD_SHA"
+ {
+ printf 'HEAD_DIR=%s\n' "$WORKTREE_ROOT/head"
+ printf 'WORKTREE_ROOT=%s\n' "$WORKTREE_ROOT"
+ } >> "$GITHUB_ENV"
+ if [ -n "$RESOLVED_BASE_SHA" ] && git worktree add --detach "$WORKTREE_ROOT/base" "$RESOLVED_BASE_SHA"; then
+ {
+ printf 'BASE_DIR=%s\n' "$WORKTREE_ROOT/base"
+ printf 'RESOLVED_BASE_SHA=%s\n' "$RESOLVED_BASE_SHA"
+ } >> "$GITHUB_ENV"
+ echo "Using base commit $RESOLVED_BASE_SHA."
+ else
+ echo "BASE_BENCHMARKS_AVAILABLE=false" >> "$GITHUB_ENV"
+ fi
+ # Build both JMH jars before measuring either revision. Builds are CPU- and IO-heavy,
+ # so building and measuring one revision at a time would bias results with different
+ # thermal, cache, and CPU-frequency state on the shared runner.
+ - name: "π¨ Build paired JMH jars"
+ timeout-minutes: 60
+ run: |
+ base_build_ok=false
+ head_build_ok=false
+ if [ "${BASE_BENCHMARKS_AVAILABLE:-true}" = "true" ] && [ -f "$BASE_DIR/grails-benchmarks/build.gradle" ]; then
+ if (
+ cd "$BASE_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ base_build_ok=true
+ echo "Built base benchmark at $RESOLVED_BASE_SHA."
+ else
+ echo "BASE benchmark JAR build failed."
+ fi
+ else
+ echo "BASE does not contain grails-benchmarks; comparison will use HEAD-only mode."
+ fi
+ if (
+ cd "$HEAD_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ head_build_ok=true
+ echo "Built HEAD benchmark at $HEAD_SHA."
+ else
+ echo "HEAD benchmark JAR build failed."
+ fi
+ {
+ printf 'BASE_BUILD_OK=%s\n' "$base_build_ok"
+ printf 'HEAD_BUILD_OK=%s\n' "$head_build_ok"
+ } >> "$GITHUB_ENV"
+ # Two forks, three warmup iterations, and five measurement iterations balance PR latency
+ # against confidence. Reversing shard order cancels first-versus-second runner-state bias.
+ - name: "π‘οΈ Run paired JMH benchmarks"
+ timeout-minutes: 60
+ run: |
+ # JMH writes results incrementally, so a run that dies partway can leave a file that
+ # parses perfectly while describing only some of the benchmarks. The report job decides
+ # completeness from which files exist, so a partial file would be indistinguishable from
+ # a good one. Write to a staging path and publish it only on success, so a failed run
+ # leaves NO file rather than a plausible one.
+ run_benchmark() {
+ local revision_dir="$1"
+ local result_file="$2"
+ local staging_file="$result_file.partial"
+ rm -f "$staging_file" "$result_file"
+ if (
+ cd "$revision_dir" && ./gradlew :grails-benchmarks:jmh \
+ -Pjmh.include="$JMH_INCLUDE" \
+ -Pjmh.forks=2 \
+ -Pjmh.warmupIterations=3 \
+ -Pjmh.iterations=5 \
+ -Pjmh.resultFile="$staging_file" \
+ -Pjmh.profilers=gc \
+ --max-workers=4
+ ) && [ -s "$staging_file" ] && mv "$staging_file" "$result_file"; then
+ return 0
+ fi
+ rm -f "$staging_file"
+ return 1
+ }
+
+ base_run_failed=false
+ head_run_failed=false
+ run_base_benchmark() {
+ if [ "${BASE_BUILD_OK:-false}" != "true" ]; then
+ echo "BASE benchmark execution skipped because its JAR was not built."
+ elif ! run_benchmark "$BASE_DIR" "$RESULT_DIR/base.json"; then
+ base_run_failed=true
+ echo "BASE benchmark execution failed."
+ fi
+ }
+ run_head_benchmark() {
+ if [ "${HEAD_BUILD_OK:-false}" != "true" ]; then
+ echo "HEAD benchmark execution skipped because its JAR was not built."
+ elif ! run_benchmark "$HEAD_DIR" "$RESULT_DIR/head.json"; then
+ head_run_failed=true
+ echo "HEAD benchmark execution failed."
+ fi
+ }
+
+ if [ "$SHARD" = "a" ]; then
+ run_base_benchmark
+ run_head_benchmark
+ else
+ run_head_benchmark
+ run_base_benchmark
+ fi
+ {
+ printf 'BASE_RUN_FAILED=%s\n' "$base_run_failed"
+ printf 'HEAD_RUN_FAILED=%s\n' "$head_run_failed"
+ } >> "$GITHUB_ENV"
+ # The comparison is advisory. jmh_compare.py exits successfully for regressions, and this
+ # step is non-blocking even if results are incomplete because a benchmark execution failed.
+ # No --pr-number is passed here on purpose: this job renders the report only. The separate
+ # report job owns comment posting, so a two-shard matrix cannot produce duplicate comments.
+ - name: "π Compare JMH results"
+ if: always()
+ continue-on-error: true
+ run: |
+ report_file="$REPORT_DIR/comparison.md"
+ {
+ printf '### JMH shard `%s`\n\n' "$SHARD"
+ if [ "${BASE_RUN_FAILED:-false}" = "true" ]; then
+ printf 'BASE benchmark execution failed.\n\n'
+ fi
+ if [ "${BASE_BUILD_OK:-false}" != "true" ]; then
+ printf 'BASE benchmark JAR was not built.\n\n'
+ fi
+ if [ "${HEAD_RUN_FAILED:-false}" = "true" ]; then
+ printf 'HEAD benchmark execution failed.\n\n'
+ fi
+ if [ "${HEAD_BUILD_OK:-false}" != "true" ]; then
+ printf 'HEAD benchmark JAR was not built.\n\n'
+ fi
+ } > "$report_file"
+ if [ ! -f "$RESULT_DIR/head.json" ]; then
+ printf 'HEAD benchmark result was not produced.\n' >> "$report_file"
+ elif [ "${BASE_BUILD_OK:-false}" = "true" ] && [ -f "$RESULT_DIR/base.json" ]; then
+ python3 .github/scripts/jmh_compare.py --head "$RESULT_DIR/head.json" --base "$RESULT_DIR/base.json" >> "$report_file"
+ else
+ python3 .github/scripts/jmh_compare.py --head "$RESULT_DIR/head.json" >> "$report_file"
+ fi
Review Comment:
The new `jmh_compare.py` unit tests arenβt executed anywhere in CI, so regressions in the reporting logic can land unnoticed. Since this step already invokes the script, consider running the stdlib `test_jmh_compare.py` right before generating the shard report.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley merged PR #16071:
URL: https://github.com/apache/grails-core/pull/16071
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#issuecomment-5153591585
Why are we using python in this PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#discussion_r3725266313
##########
grails-benchmarks/build.gradle:
##########
@@ -0,0 +1,270 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.gradle.api.file.DuplicatesStrategy
+import org.gradle.api.tasks.PathSensitivity
+
+import java.util.Properties
+import java.util.zip.ZipFile
+
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'me.champeau.jmh' version '0.7.3'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.compile'
+ id 'org.apache.grails.buildsrc.vulnerability-scan'
+ id 'org.apache.grails.gradle.grails-code-style'
+}
+
+version = projectVersion
+group = 'org.apache.grails'
+
+// This build-time-only benchmark harness is never published, so it deliberately omits
+// org.apache.grails.buildsrc.publish, org.apache.grails.buildsrc.sbom,
+// org.apache.grails.gradle.grails-jacoco, and org.apache.grails.buildsrc.dependency-validator.
+// Its JMH dependency is GPLv2 with the Classpath Exception (Category X) and must not be published.
+
+sourceSets {
+ report {
+ groovy.srcDirs = ['src/report/groovy']
+ }
+}
+
+// The Groovy fixtures under src/main/groovy build the framework objects whose APIs are
+// closure-based (the URL mappings DSL, the Validateable trait, view templates). The JMH
Review Comment:
Addressed in b8d3233184 and 22c45df65a. The stale Validateable fixture references were removed from the build and README while the withdrawn validation gap remains documented.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "paulk-asert (via GitHub)" <gi...@apache.org>.
paulk-asert commented on PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#issuecomment-5136406775
Just FYI, we find the graphs Groovy has super useful, the per PR/commit regression stats not so much because of the wide variance of GitHub runner speed. It does some "ruler adjusting" to try to account for runner speed but even then it isn't super accurate - we just use it as a guide to then check things manually on a dedicated machine.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#issuecomment-5148355717
## [Codecov](https://app.codecov.io/gh/apache/grails-core/pull/16071?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 51.8562%. Comparing base ([`301b40c`](https://app.codecov.io/gh/apache/grails-core/commit/301b40c20acdb8650ea36cf685fbd54a6341bccb?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`ed2db8a`](https://app.codecov.io/gh/apache/grails-core/commit/ed2db8a1aff7cb870570f653a59c6a22a3d196d7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on 8.0.x.
<details><summary>Additional details and impacted files</summary>
[](https://app.codecov.io/gh/apache/grails-core/pull/16071?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
```diff
@@ Coverage Diff @@
## 8.0.x #16071 +/- ##
==================================================
- Coverage 53.1945% 51.8562% -1.3383%
- Complexity 17798 18110 +312
==================================================
Files 1980 2046 +66
Lines 92519 96274 +3755
Branches 16075 16727 +652
==================================================
+ Hits 49215 49924 +709
- Misses 36076 38980 +2904
- Partials 7228 7370 +142
```
[see 69 files with indirect coverage changes](https://app.codecov.io/gh/apache/grails-core/pull/16071/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
</details>
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#issuecomment-5171927674
## β
All tests passed β
π·οΈ Commit: 202d206a6a19a78c02d46a36ebe067343d0c1ea1
βΆοΈ Tests: 55384 executed
βͺοΈ Checks: 62/62 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#discussion_r3685933051
##########
grails-benchmarks/build.gradle:
##########
@@ -0,0 +1,238 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.gradle.api.file.DuplicatesStrategy
+import org.gradle.api.tasks.PathSensitivity
+
+import java.util.Properties
+import java.util.zip.ZipFile
+
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'me.champeau.jmh' version '0.7.3'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.compile'
+}
+
+version = projectVersion
+group = 'org.apache.grails'
+
+// This build-time-only benchmark harness is never published, so it deliberately omits
+// org.apache.grails.buildsrc.publish, org.apache.grails.buildsrc.sbom,
+// org.apache.grails.buildsrc.vulnerability-scan, org.apache.grails.gradle.grails-jacoco,
+// org.apache.grails.buildsrc.dependency-validator, and org.apache.grails.gradle.grails-code-style.
+// The latter remains omitted until existing JMH source import-order and fixture-formatting
+// violations can be remediated together.
+
+// The Groovy fixtures under src/main/groovy build the framework objects whose APIs are
+// closure-based (the URL mappings DSL, the Validateable trait, view templates). The JMH
+// benchmarks themselves live in src/jmh/java and only *call* those fixtures, so that the
+// measured code path is plain Java and does not include Groovy's dynamic dispatch.
+// me.champeau.jmh puts the main source set's output on the jmh compile classpath, which is
+// why the fixtures are in main rather than src/jmh/groovy: keeping the benchmarks in a pure
+// Java source set preserves normal JMH annotation processing.
+dependencies {
+ implementation platform(project(':grails-bom'))
+
+ implementation project(':grails-web-url-mappings')
+ implementation project(':grails-databinding-core')
+ implementation project(':grails-gsp-core')
+ implementation project(':grails-interceptors')
+ implementation project(':grails-views-gson')
+ implementation project(':grails-views-markup')
+ implementation project(':grails-core')
+ implementation 'org.apache.groovy:groovy'
+
+ // The framework modules declare these as compileOnly, so they are absent from the runtime
+ // classpath a benchmark actually executes on. The modules' own test suites add them back
+ // the same way. Without them, URL mapping, interceptor and view benchmarks fail at @Setup
+ // with NoClassDefFoundError: jakarta/servlet/ServletContext.
+ implementation 'jakarta.servlet:jakarta.servlet-api'
+ implementation 'org.springframework:spring-test'
+}
+
+// The jmh source set compiles and runs against everything main depends on.
+configurations.jmh.extendsFrom(configurations.implementation)
+
+// Every JMH knob the CI workflow needs to override is exposed as a Gradle property so that
+// .github/workflows/benchmark.yml can run a fast PR profile and a slower scheduled profile
+// from the same build. Defaults here are the quick local-development profile.
+jmh {
+ jmhVersion = '1.37'
+ includes = providers.gradleProperty('jmh.include')
+ .map { it.split(',') as List<String> }
+ .orElse(['.*'])
+ .get()
+ fork = providers.gradleProperty('jmh.forks').map { it as Integer }.orElse(1).get()
+ warmupIterations = providers.gradleProperty('jmh.warmupIterations').map { it as Integer }.orElse(1).get()
+ iterations = providers.gradleProperty('jmh.iterations').map { it as Integer }.orElse(1).get()
+ warmup = providers.gradleProperty('jmh.warmupTime').orElse('1s').get()
+ timeOnIteration = providers.gradleProperty('jmh.iterationTime').orElse('1s').get()
+ resultFormat = providers.gradleProperty('jmh.resultFormat').orElse('JSON').get()
+ // A benchmark that throws during @Setup is otherwise dropped from the results file while
+ // the build still reports success, which would silently shrink the comparison set.
+ failOnError = providers.gradleProperty('jmh.failOnError').map { it.toBoolean() }.orElse(true).get()
+ resultsFile = file(providers.gradleProperty('jmh.resultFile')
+ .orElse(layout.buildDirectory.file('results/jmh/results.json').get().asFile.absolutePath)
+ .get())
+ // Comma-separated JMH profilers, e.g. -Pjmh.profilers=gc for allocation reporting.
+ profilers = providers.gradleProperty('jmh.profilers')
+ .map { it.split(',') as List<String> }
+ .orElse([])
+ .get()
+}
+
+def mergeJmhClasspathMetadata = tasks.register('mergeJmhClasspathMetadata') {
+ def mergedMetadataDirectory = layout.buildDirectory.dir('generated/jmh-classpath-metadata')
+ def lineMetadataPrefixes = ['META-INF/services/', 'META-INF/groovy/']
+ def extensionModuleSuffix = 'org.codehaus.groovy.runtime.ExtensionModule'
+ def propertiesMetadataPaths = [
+ 'META-INF/spring.factories',
+ 'META-INF/spring.handlers',
+ 'META-INF/spring.schemas'
+ ]
+
+ inputs.files(configurations.jmhRuntimeClasspath)
+ .withPropertyName('jmhRuntimeClasspath')
+ .withPathSensitivity(PathSensitivity.RELATIVE)
+ outputs.dir(mergedMetadataDirectory)
+ outputs.cacheIf { true }
+
+ doLast {
+ File outputDirectory = mergedMetadataDirectory.get().asFile
+ project.delete(outputDirectory)
+
+ Map<String, Set<String>> lineEntries = new TreeMap<>()
+ Map<String, Map<String, Set<String>>> extensionModuleEntries = new TreeMap<>()
+ Map<String, Map<String, Set<String>>> propertyEntries = new TreeMap<>()
+
+ configurations.jmhRuntimeClasspath.files
+ .findAll { File artifact -> artifact.name.endsWith('.jar') }
+ .sort { File artifact -> artifact.absolutePath }
+ .each { File artifact ->
+ new ZipFile(artifact).withCloseable { ZipFile zipFile ->
+ zipFile.entries().each { entry ->
+ if (entry.directory) {
+ return
+ }
+
+ String path = entry.name
+ if (path.endsWith(extensionModuleSuffix)) {
+ Properties properties = new Properties()
+ zipFile.getInputStream(entry).withCloseable { input ->
+ properties.load(input)
+ }
+ Map<String, Set<String>> entries = extensionModuleEntries.computeIfAbsent(path) {
+ new TreeMap<>()
+ }
+ ['extensionClasses', 'staticExtensionClasses'].each { String key ->
+ Set<String> values = entries.computeIfAbsent(key) { new TreeSet<>() }
+ properties.getProperty(key, '').split(',').each { String value ->
+ if (value) {
+ values.add(value.trim())
+ }
+ }
+ }
+ } 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)
+ }
+ }
Review Comment:
Good catch, fixed in b06d51784c.
This was a genuine inconsistency rather than a theoretical one: of the three places `mergeJmhClasspathMetadata` reads a zip entry, the two `Properties.load` branches already wrapped the stream in `withCloseable`, and only the line-metadata branch called `.getText('UTF-8')` on an unclosed stream. That branch is the one that runs most often, since it handles every `META-INF/services/**` and `META-INF/groovy/**` entry across the whole jmh runtime classpath.
It now matches the other two:
```groovy
zipFile.getInputStream(entry).withCloseable { input ->
input.getText('UTF-8').readLines().each { String line ->
if (line) {
lines.add(line)
}
}
}
```
Verified the fix changes nothing about the merge output: a full `--rerun-tasks` rebuild of `:grails-benchmarks:jmhJar` still produces the merged `META-INF/services/org.codehaus.groovy.runtime.ExtensionModule` with all 9 extension classes, and `META-INF/spring.factories` with 24 merged keys. All 13 benchmarks still execute cleanly from the rebuilt jar.
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,404 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
+ paths-ignore:
+ - '**/*.md'
+ - '**/*.adoc'
+ - 'grails-doc/**'
+ workflow_dispatch:
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+permissions:
+ contents: read
+
+jobs:
+ # Each shard builds both revisions before measuring either one on the same runner.
+ # Building first avoids CPU, IO, thermal, cache, and frequency state biasing a measurement.
+ # Shard a measures BASE then HEAD, while shard b measures HEAD then BASE.
+ # Alternating the order cancels first-versus-second ordering bias without doubling runtime.
+ # Both shards use the PR merge commit as HEAD, so they measure what would actually land.
+ # Results are advisory: a detected regression never fails this workflow.
+ benchmark:
+ name: "Paired JMH benchmarks (${{ matrix.shard }})"
+ if: ${{ github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'performance') }}
+ runs-on: ubuntu-24.04
+ strategy:
+ fail-fast: false
+ matrix:
+ shard: [a, b]
+ env:
+ BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ HEAD_SHA: ${{ github.sha }}
+ JMH_INCLUDE: '.*'
+ PR_NUMBER: ${{ github.event.pull_request.number || 0 }}
+ REPOSITORY: ${{ github.repository }}
+ RESULT_DIR: ${{ github.workspace }}/jmh-results/${{ matrix.shard }}
+ REPORT_DIR: ${{ github.workspace }}/jmh-reports/${{ matrix.shard }}
+ SHARD: ${{ matrix.shard }}
+ steps:
+ - name: "π₯ Checkout repository"
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+ with:
+ fetch-depth: 0
+ - name: "βοΈ Setup JDK"
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
+ with:
+ distribution: liberica
+ java-version: 21
+ - name: "π Setup Gradle"
+ uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0
+ with:
+ cache-provider: basic # 'basic' uses the MIT-licensed, open-source cache provider; the default 'enhanced' provider (v6+) is proprietary (Gradle commercial Terms of Use)
+ develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+ - name: "π³ Prepare paired worktrees"
+ run: |
+ WORKTREE_ROOT="$RUNNER_TEMP/jmh-worktrees/$SHARD"
+ mkdir -p "$WORKTREE_ROOT" "$RESULT_DIR" "$REPORT_DIR"
+ if [ -n "$BASE_SHA" ] && git cat-file -e "$BASE_SHA^{commit}" 2>/dev/null; then
+ RESOLVED_BASE_SHA="$BASE_SHA"
+ elif RESOLVED_BASE_SHA="$(git merge-base "$HEAD_SHA^1" "$HEAD_SHA^2" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using merge-base $RESOLVED_BASE_SHA."
+ elif RESOLVED_BASE_SHA="$(git rev-parse "$HEAD_SHA^" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using HEAD parent $RESOLVED_BASE_SHA."
+ else
+ RESOLVED_BASE_SHA=""
+ echo "No base commit could be resolved; comparison will use HEAD-only mode."
+ fi
+ git worktree add --detach "$WORKTREE_ROOT/head" "$HEAD_SHA"
+ {
+ printf 'HEAD_DIR=%s\n' "$WORKTREE_ROOT/head"
+ printf 'WORKTREE_ROOT=%s\n' "$WORKTREE_ROOT"
+ } >> "$GITHUB_ENV"
+ if [ -n "$RESOLVED_BASE_SHA" ] && git worktree add --detach "$WORKTREE_ROOT/base" "$RESOLVED_BASE_SHA"; then
+ {
+ printf 'BASE_DIR=%s\n' "$WORKTREE_ROOT/base"
+ printf 'RESOLVED_BASE_SHA=%s\n' "$RESOLVED_BASE_SHA"
+ } >> "$GITHUB_ENV"
+ echo "Using base commit $RESOLVED_BASE_SHA."
+ else
+ echo "BASE_BENCHMARKS_AVAILABLE=false" >> "$GITHUB_ENV"
+ fi
+ # Build both JMH jars before measuring either revision. Builds are CPU- and IO-heavy,
+ # so building and measuring one revision at a time would bias results with different
+ # thermal, cache, and CPU-frequency state on the shared runner.
+ - name: "π¨ Build paired JMH jars"
+ timeout-minutes: 60
+ run: |
+ base_build_ok=false
+ head_build_ok=false
+ if [ "${BASE_BENCHMARKS_AVAILABLE:-true}" = "true" ] && [ -f "$BASE_DIR/grails-benchmarks/build.gradle" ]; then
+ if (
+ cd "$BASE_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ base_build_ok=true
+ echo "Built base benchmark at $RESOLVED_BASE_SHA."
+ else
+ echo "BASE benchmark JAR build failed."
+ fi
+ else
+ echo "BASE does not contain grails-benchmarks; comparison will use HEAD-only mode."
+ fi
+ if (
+ cd "$HEAD_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ head_build_ok=true
+ echo "Built HEAD benchmark at $HEAD_SHA."
+ else
+ echo "HEAD benchmark JAR build failed."
+ fi
+ {
+ printf 'BASE_BUILD_OK=%s\n' "$base_build_ok"
+ printf 'HEAD_BUILD_OK=%s\n' "$head_build_ok"
+ } >> "$GITHUB_ENV"
+ # Two forks, three warmup iterations, and five measurement iterations balance PR latency
+ # against confidence. Reversing shard order cancels first-versus-second runner-state bias.
+ - name: "π‘οΈ Run paired JMH benchmarks"
+ timeout-minutes: 60
+ run: |
+ # JMH writes results incrementally, so a run that dies partway can leave a file that
+ # parses perfectly while describing only some of the benchmarks. The report job decides
+ # completeness from which files exist, so a partial file would be indistinguishable from
+ # a good one. Write to a staging path and publish it only on success, so a failed run
+ # leaves NO file rather than a plausible one.
+ run_benchmark() {
+ local revision_dir="$1"
+ local result_file="$2"
+ local staging_file="$result_file.partial"
+ rm -f "$staging_file" "$result_file"
+ if (
+ cd "$revision_dir" && ./gradlew :grails-benchmarks:jmh \
+ -Pjmh.include="$JMH_INCLUDE" \
+ -Pjmh.forks=2 \
+ -Pjmh.warmupIterations=3 \
+ -Pjmh.iterations=5 \
+ -Pjmh.resultFile="$staging_file" \
+ -Pjmh.profilers=gc \
+ --max-workers=4
+ ) && [ -s "$staging_file" ] && mv "$staging_file" "$result_file"; then
+ return 0
+ fi
+ rm -f "$staging_file"
+ return 1
+ }
+
+ base_run_failed=false
+ head_run_failed=false
+ run_base_benchmark() {
+ if [ "${BASE_BUILD_OK:-false}" != "true" ]; then
+ echo "BASE benchmark execution skipped because its JAR was not built."
+ elif ! run_benchmark "$BASE_DIR" "$RESULT_DIR/base.json"; then
+ base_run_failed=true
+ echo "BASE benchmark execution failed."
+ fi
+ }
+ run_head_benchmark() {
+ if [ "${HEAD_BUILD_OK:-false}" != "true" ]; then
+ echo "HEAD benchmark execution skipped because its JAR was not built."
+ elif ! run_benchmark "$HEAD_DIR" "$RESULT_DIR/head.json"; then
+ head_run_failed=true
+ echo "HEAD benchmark execution failed."
+ fi
+ }
+
+ if [ "$SHARD" = "a" ]; then
+ run_base_benchmark
+ run_head_benchmark
+ else
+ run_head_benchmark
+ run_base_benchmark
+ fi
+ {
+ printf 'BASE_RUN_FAILED=%s\n' "$base_run_failed"
+ printf 'HEAD_RUN_FAILED=%s\n' "$head_run_failed"
+ } >> "$GITHUB_ENV"
+ # The comparison is advisory. jmh_compare.py exits successfully for regressions, and this
+ # step is non-blocking even if results are incomplete because a benchmark execution failed.
+ # No --pr-number is passed here on purpose: this job renders the report only. The separate
+ # report job owns comment posting, so a two-shard matrix cannot produce duplicate comments.
+ - name: "π Compare JMH results"
+ if: always()
+ continue-on-error: true
+ run: |
+ report_file="$REPORT_DIR/comparison.md"
+ {
+ printf '### JMH shard `%s`\n\n' "$SHARD"
+ if [ "${BASE_RUN_FAILED:-false}" = "true" ]; then
+ printf 'BASE benchmark execution failed.\n\n'
+ fi
+ if [ "${BASE_BUILD_OK:-false}" != "true" ]; then
+ printf 'BASE benchmark JAR was not built.\n\n'
+ fi
+ if [ "${HEAD_RUN_FAILED:-false}" = "true" ]; then
+ printf 'HEAD benchmark execution failed.\n\n'
+ fi
+ if [ "${HEAD_BUILD_OK:-false}" != "true" ]; then
+ printf 'HEAD benchmark JAR was not built.\n\n'
+ fi
+ } > "$report_file"
+ if [ ! -f "$RESULT_DIR/head.json" ]; then
+ printf 'HEAD benchmark result was not produced.\n' >> "$report_file"
+ elif [ "${BASE_BUILD_OK:-false}" = "true" ] && [ -f "$RESULT_DIR/base.json" ]; then
+ python3 .github/scripts/jmh_compare.py --head "$RESULT_DIR/head.json" --base "$RESULT_DIR/base.json" >> "$report_file"
+ else
+ python3 .github/scripts/jmh_compare.py --head "$RESULT_DIR/head.json" >> "$report_file"
+ fi
Review Comment:
Agreed, and fixed in b06d51784c - this was a real gap. The 39 tests were the only thing standing behind the comparison logic, and nothing in CI ran them.
I've added a `π§ͺ Verify JMH comparison script` step that runs `python3 .github/scripts/test_jmh_compare.py`, with one deliberate change from your suggestion: rather than placing it immediately before the report is generated, it runs right after checkout and **before** the JDK/Gradle setup and the build-and-measure steps. A broken reporter should fail in seconds rather than after roughly an hour of building and benchmarking two revisions, and the shard job is the right home because it runs for fork PRs too, which never reach the reporting job.
The step is also deliberately allowed to **fail** the job. That is an intentional exception to this workflow's "never fails a build" rule, and the YAML carries a comment saying so, because the rule governs performance *regressions* (which stay advisory) whereas a broken comparison script invalidates every number the run produces. Without the note, the obvious later "fix" would be to add `continue-on-error: true` and silently defeat the guard.
Worth noting these tests are not decorative - the same suite already caught several defects during development, including shard results being silently overwritten instead of pooled, and a benchmark receiving a verdict after its JMH mode changed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#issuecomment-5148626973
## β
All tests passed β
π·οΈ Commit: ed2db8a1aff7cb870570f653a59c6a22a3d196d7
βΆοΈ Tests: 44367 executed
βͺοΈ Checks: 62/62 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#issuecomment-5158539800
The implementation was based on https://github.com/apache/groovy/blob/master/subprojects/performance/dashboard/jmh-summary.py, didn't notice the Python, but will make it Groovy instead.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#issuecomment-5158683276
## π¨ TestLens detected 6 failed tests π¨
Here is what you can do:
1) Inspect the test failures carefully.
2) If you are convinced that some of the tests are flaky, you can mute them below.
3) Finally, trigger a rerun by checking the rerun checkbox.
### Test Summary
#### [CI / Build Grails-Core \(windows-latest, 25\)](https://github.com/apache/grails-core/actions/runs/30752280498/job/91508824375?pr=16071) > :grails-benchmarks:test
| Test | Runs | Flakiness |
|---|---|--:|
| GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file > broad report exactly matches the Python golden file | β | 0% π’ |
| GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file > edges report exactly matches the Python golden file | β | 0% π’ |
| GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file > headonly report exactly matches the Python golden file | β | 0% π’ |
| GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file > norulers report exactly matches the Python golden file | β | 0% π’ |
| GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file > numbers report exactly matches the Python golden file | β | 0% π’ |
| GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file > shards report exactly matches the Python golden file | β | 0% π’ |
π·οΈ Commit: e510a627a1beaeae6d16c5d26e53407d4daaf1be
βΆοΈ Tests: 13275 executed
π‘ Checks: 26/56 completed
### Test Failures
<details>
<summary><strong>GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file > broad report exactly matches the Python golden file</strong> (:grails-benchmarks:test in <a href="https://github.com/apache/grails-core/actions/runs/30752280498/job/91508824375?pr=16071">CI / Build Grails-Core (windows-latest, 25)</a>)</summary>
```
Condition not satisfied:
Files.readString(output) == resource(expected)
| | | | | |
| | | | | jmh-golden/expected-broad.md
| | | | ### JMH Benchmark Report
| | | |
| | | | **Regressions:** 1
| | | | **Improvements:** 2
| | | | **Runner health:** worst ruler deviation: 12.4% in anonymous shard (CpuRulerBenchmark.integerArithmetic)
| | | | Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.
| | | | **Ruler movements:** anonymous shard: CpuRulerBenchmark.integerArithmetic: 1.12x, anonymous shard: MemoryRulerBenchmark.allocateAndCopyArray: 0.93x
| | | |
| | | | > **Warning:** The runner was unstable BETWEEN the two halves of the A/B run. Treat results as unreliable. Runner health is a stability check, not a calibration factor.
| | | |
| | | | Group geometric means are descriptive only, not verdicts.
| | | |
| | | | | Group | Descriptive geomean speedup | n |
| | | | | --- | ---: | ---: |
| | | | | databinding | 0.94x | 1 |
| | | | | gsp | 1.00x | 1 |
| | | | | throughput | 1.20x | 1 |
| | | | | urlmappings | 0.69x | 2 |
| | | | | views | 2.00x | 1 |
| | | |
| | | | <details>
| | | | <summary>Per-benchmark results</summary>
| | | |
| | | | | Benchmark | Base score | Head score | Speedup | Verdict | Allocation delta (ADVISORY) |
| | | | | --- | ---: | ---: | ---: | --- | ---: |
| | | | | urlmappings.UrlMappingsBenchmark.matchWarmCache | 4.07 ns/op | 8.77 ns/op | 0.46x | REGRESSED | ~0 B/op |
| | | | | throughput.Ops.run | 100 ops/s | 120 ops/s | 1.20x | IMPROVED | β |
| | | | | views.ViewTemplateRenderingBenchmark.renderJsonTemplate | 0.92 ns/op | 0.46 ns/op | 2.00x | IMPROVED | +17 B/op (+17.0%) **candidate** |
| | | | | databinding.SimpleDataBinderBenchmark.bindFlatMap | 1.72e+04 ns/op | 1.83e+04 ns/op | 0.94x | no clear change | +17 B/op (+1.7%) |
| | | | | gsp.GroovyPageParserBenchmark.parseSmallTemplate | 9.98e+03 ns/op | 9.94e+03 ns/op | 1.00x | no clear change | ~0 B/op |
| | | | | ruler.CpuRulerBenchmark.integerArithmetic | 100 ns/op | 89 ns/op | 1.12x | ruler - excluded | β |
| | | | | ruler.MemoryRulerBenchmark.allocateAndCopyArray | 100 ns/op | 108 ns/op | 0.93x | ruler - excluded | β |
| | | | | urlmappings.UrlMappingsBenchmark.matchColdVariedKeys | 818 ns/op | 802 ns/op | 1.02x | no clear change | +10 B/op (+1.0%) |
| | | |
| | | | </details>
| | | |
| | | | <!-- grails-jmh-benchmark -->
| | | false
| | | Strings too large to calculate edit distance.
| | C:\Users\RUNNER~1\AppData\Local\Temp\spock_broad_report_exactl_0_temporaryDirectory17241353759838329739\broad.md
| ### JMH Benchmark Report
|
| **Regressions:** 1
| **Improvements:** 2
| **Runner health:** worst ruler deviation: 12.4% in anonymous shard (CpuRulerBenchmark.integerArithmetic)
| Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.
| **Ruler movements:** anonymous shard: CpuRulerBenchmark.integerArithmetic: 1.12x, anonymous shard: MemoryRulerBenchmark.allocateAndCopyArray: 0.93x
|
| > **Warning:** The runner was unstable BETWEEN the two halves of the A/B run. Treat results as unreliable. Runner health is a stability check, not a calibration factor.
|
| Group geometric means are descriptive only, not verdicts.
|
| | Group | Descriptive geomean speedup | n |
| | --- | ---: | ---: |
| | databinding | 0.94x | 1 |
| | gsp | 1.00x | 1 |
| | throughput | 1.20x | 1 |
| | urlmappings | 0.69x | 2 |
| | views | 2.00x | 1 |
|
| <details>
| <summary>Per-benchmark results</summary>
|
| | Benchmark | Base score | Head score | Speedup | Verdict | Allocation delta (ADVISORY) |
| | --- | ---: | ---: | ---: | --- | ---: |
| | urlmappings.UrlMappingsBenchmark.matchWarmCache | 4.07 ns/op | 8.77 ns/op | 0.46x | REGRESSED | ~0 B/op |
| | throughput.Ops.run | 100 ops/s | 120 ops/s | 1.20x | IMPROVED | β |
| | views.ViewTemplateRenderingBenchmark.renderJsonTemplate | 0.92 ns/op | 0.46 ns/op | 2.00x | IMPROVED | +17 B/op (+17.0%) **candidate** |
| | databinding.SimpleDataBinderBenchmark.bindFlatMap | 1.72e+04 ns/op | 1.83e+04 ns/op | 0.94x | no clear change | +17 B/op (+1.7%) |
| | gsp.GroovyPageParserBenchmark.parseSmallTemplate | 9.98e+03 ns/op | 9.94e+03 ns/op | 1.00x | no clear change | ~0 B/op |
| | ruler.CpuRulerBenchmark.integerArithmetic | 100 ns/op | 89 ns/op | 1.12x | ruler - excluded | β |
| | ruler.MemoryRulerBenchmark.allocateAndCopyArray | 100 ns/op | 108 ns/op | 0.93x | ruler - excluded | β |
| | urlmappings.UrlMappingsBenchmark.matchColdVariedKeys | 818 ns/op | 802 ns/op | 1.02x | no clear change | +10 B/op (+1.0%) |
|
| </details>
|
| <!-- grails-jmh-benchmark -->
class java.nio.file.Files
at org.apache.grails.benchmarks.report.GoldenReportSpec.#name report exactly matches the Python golden file(GoldenReportSpec.groovy:41)
```
|expected|actual|
|---|---|
|<span>#</span><span>#</span><span>#</span> JMH Benchmark Report|<span>#</span><span>#</span><span>#</span> JMH Benchmark Report|
|||
|\*\*Regressions:\*\* 1|\*\*Regressions:\*\* 1|
|\*\*Improvements:\*\* 2|\*\*Improvements:\*\* 2|
|\*\*Runner health:\*\* worst ruler deviation: 12.4\% in anonymous shard \(CpuRulerBenchmark.integerArithmetic\)|\*\*Runner health:\*\* worst ruler deviation: 12.4\% in anonymous shard \(CpuRulerBenchmark.integerArithmetic\)|
|Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.|Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.|
|\*\*Ruler movements:\*\* anonymous shard: CpuRulerBenchmark.integerArithmetic: 1.12x, anonymous shard: MemoryRulerBenchmark.allocateAndCopyArray: 0.93x|\*\*Ruler movements:\*\* anonymous shard: CpuRulerBenchmark.integerArithmetic: 1.12x, anonymous shard: MemoryRulerBenchmark.allocateAndCopyArray: 0.93x|
|||
|\> \*\*Warning:\*\* The runner was unstable BETWEEN the two halves of the A/B run. Treat results as unreliable. Runner health is a stability check, not a calibration factor.|\> \*\*Warning:\*\* The runner was unstable BETWEEN the two halves of the A/B run. Treat results as unreliable. Runner health is a stability check, not a calibration factor.|
|||
|Group geometric means are descriptive only, not verdicts.|Group geometric means are descriptive only, not verdicts.|
|||
|\| Group \| Descriptive geomean speedup \| n \||\| Group \| Descriptive geomean speedup \| n \||
|\| --- \| ---: \| ---: \||\| --- \| ---: \| ---: \||
|\| databinding \| 0.94x \| 1 \||\| databinding \| 0.94x \| 1 \||
|\| gsp \| 1.00x \| 1 \||\| gsp \| 1.00x \| 1 \||
|\| throughput \| 1.20x \| 1 \||\| throughput \| 1.20x \| 1 \||
|\| urlmappings \| 0.69x \| 2 \||\| urlmappings \| 0.69x \| 2 \||
|\| views \| 2.00x \| 1 \||\| views \| 2.00x \| 1 \||
|||
|\<details\>|\<details\>|
|\<summary\>Per-benchmark results\</summary\>|\<summary\>Per-benchmark results\</summary\>|
|||
|\| Benchmark \| Base score \| Head score \| Speedup \| Verdict \| Allocation delta \(ADVISORY\) \||\| Benchmark \| Base score \| Head score \| Speedup \| Verdict \| Allocation delta \(ADVISORY\) \||
|\| --- \| ---: \| ---: \| ---: \| --- \| ---: \||\| --- \| ---: \| ---: \| ---: \| --- \| ---: \||
|\| urlmappings.UrlMappingsBenchmark.matchWarmCache \| 4.07 ns/op \| 8.77 ns/op \| 0.46x \| REGRESSED \| \~0 B/op \||\| urlmappings.UrlMappingsBenchmark.matchWarmCache \| 4.07 ns/op \| 8.77 ns/op \| 0.46x \| REGRESSED \| \~0 B/op \||
|\| throughput.Ops.run \| 100 ops/s \| 120 ops/s \| 1.20x \| IMPROVED \| β \||\| throughput.Ops.run \| 100 ops/s \| 120 ops/s \| 1.20x \| IMPROVED \| β \||
|\| views.ViewTemplateRenderingBenchmark.renderJsonTemplate \| 0.92 ns/op \| 0.46 ns/op \| 2.00x \| IMPROVED \| \+17 B/op \(\+17.0\%\) \*\*candidate\*\* \||\| views.ViewTemplateRenderingBenchmark.renderJsonTemplate \| 0.92 ns/op \| 0.46 ns/op \| 2.00x \| IMPROVED \| \+17 B/op \(\+17.0\%\) \*\*candidate\*\* \||
|\| databinding.SimpleDataBinderBenchmark.bindFlatMap \| 1.72e\+04 ns/op \| 1.83e\+04 ns/op \| 0.94x \| no clear change \| \+17 B/op \(\+1.7\%\) \||\| databinding.SimpleDataBinderBenchmark.bindFlatMap \| 1.72e\+04 ns/op \| 1.83e\+04 ns/op \| 0.94x \| no clear change \| \+17 B/op \(\+1.7\%\) \||
|\| gsp.GroovyPageParserBenchmark.parseSmallTemplate \| 9.98e\+03 ns/op \| 9.94e\+03 ns/op \| 1.00x \| no clear change \| \~0 B/op \||\| gsp.GroovyPageParserBenchmark.parseSmallTemplate \| 9.98e\+03 ns/op \| 9.94e\+03 ns/op \| 1.00x \| no clear change \| \~0 B/op \||
|\| ruler.CpuRulerBenchmark.integerArithmetic \| 100 ns/op \| 89 ns/op \| 1.12x \| ruler - excluded \| β \||\| ruler.CpuRulerBenchmark.integerArithmetic \| 100 ns/op \| 89 ns/op \| 1.12x \| ruler - excluded \| β \||
|\| ruler.MemoryRulerBenchmark.allocateAndCopyArray \| 100 ns/op \| 108 ns/op \| 0.93x \| ruler - excluded \| β \||\| ruler.MemoryRulerBenchmark.allocateAndCopyArray \| 100 ns/op \| 108 ns/op \| 0.93x \| ruler - excluded \| β \||
|\| urlmappings.UrlMappingsBenchmark.matchColdVariedKeys \| 818 ns/op \| 802 ns/op \| 1.02x \| no clear change \| \+10 B/op \(\+1.0\%\) \||\| urlmappings.UrlMappingsBenchmark.matchColdVariedKeys \| 818 ns/op \| 802 ns/op \| 1.02x \| no clear change \| \+10 B/op \(\+1.0\%\) \||
|||
|\</details\>|\</details\>|
|||
|\<\!-- grails-jmh-benchmark --\>|\<\!-- grails-jmh-benchmark --\>|
|||
</details>
<details>
<summary><strong>GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file > edges report exactly matches the Python golden file</strong> (:grails-benchmarks:test in <a href="https://github.com/apache/grails-core/actions/runs/30752280498/job/91508824375?pr=16071">CI / Build Grails-Core (windows-latest, 25)</a>)</summary>
```
Condition not satisfied:
Files.readString(output) == resource(expected)
| | | | | |
| | | | | jmh-golden/expected-edges.md
| | | | ### JMH Benchmark Report
| | | |
| | | | **Regressions:** 0
| | | | **Improvements:** 1
| | | | **Runner health:** not measured
| | | | Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.
| | | |
| | | | Group geometric means are descriptive only, not verdicts.
| | | |
| | | | | Group | Descriptive geomean speedup | n |
| | | | | --- | ---: | ---: |
| | | | | e | 0.89x | 3 |
| | | | | feature | 1.25x | 1 |
| | | | | sample | 1.00x | 3 |
| | | |
| | | | <details>
| | | | <summary>Per-benchmark results</summary>
| | | |
| | | | | Benchmark | Base score | Head score | Speedup | Verdict | Allocation delta (ADVISORY) |
| | | | | --- | ---: | ---: | ---: | --- | ---: |
| | | | | feature.Subject.runlabel=Ruler | 100 ns/op | 80 ns/op | 1.25x | IMPROVED | β |
| | | | | e.Insufficient.run | 100 ns/op | 120 ns/op | 0.83x | insufficient data | β |
| | | | | e.NonFinite.run | 100 ns/op | 120 ns/op | 0.83x | insufficient data | β |
| | | | | e.Shared.run | 100 ns/op | 100 ns/op | 1.00x | no clear change | β |
| | | | | sample.binjected.run | 100 ns/op | 100 ns/op | 1.00x | no clear change | β |
| | | | | sample.Bang.runxy | 100 ns/op | 100 ns/op | 1.00x | no clear change | β |
| | | | | sample.Evil.runlabel=xhttp://example.com | 100 ns/op | 100 ns/op | 1.00x | no clear change | β |
| | | |
| | | | </details>
| | | |
| | | | **Dropped unpaired shard samples (2):** e.BaseOnly.run, e.HeadOnly.run
| | | |
| | | | **Malformed comparisons skipped:** 3
| | | |
| | | | <!-- grails-jmh-benchmark -->
| | | false
| | | Strings too large to calculate edit distance.
| | C:\Users\RUNNER~1\AppData\Local\Temp\spock_edges_report_exactl_2_temporaryDirectory9150001334869807736\edges.md
| ### JMH Benchmark Report
|
| **Regressions:** 0
| **Improvements:** 1
| **Runner health:** not measured
| Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.
|
| Group geometric means are descriptive only, not verdicts.
|
| | Group | Descriptive geomean speedup | n |
| | --- | ---: | ---: |
| | e | 0.89x | 3 |
| | feature | 1.25x | 1 |
| | sample | 1.00x | 3 |
|
| <details>
| <summary>Per-benchmark results</summary>
|
| | Benchmark | Base score | Head score | Speedup | Verdict | Allocation delta (ADVISORY) |
| | --- | ---: | ---: | ---: | --- | ---: |
| | feature.Subject.runlabel=Ruler | 100 ns/op | 80 ns/op | 1.25x | IMPROVED | β |
| | e.Insufficient.run | 100 ns/op | 120 ns/op | 0.83x | insufficient data | β |
| | e.NonFinite.run | 100 ns/op | 120 ns/op | 0.83x | insufficient data | β |
| | e.Shared.run | 100 ns/op | 100 ns/op | 1.00x | no clear change | β |
| | sample.binjected.run | 100 ns/op | 100 ns/op | 1.00x | no clear change | β |
| | sample.Bang.runxy | 100 ns/op | 100 ns/op | 1.00x | no clear change | β |
| | sample.Evil.runlabel=xhttp://example.com | 100 ns/op | 100 ns/op | 1.00x | no clear change | β |
|
| </details>
|
| **Dropped unpaired shard samples (2):** e.BaseOnly.run, e.HeadOnly.run
|
| **Malformed comparisons skipped:** 3
|
| <!-- grails-jmh-benchmark -->
class java.nio.file.Files
at org.apache.grails.benchmarks.report.GoldenReportSpec.#name report exactly matches the Python golden file(GoldenReportSpec.groovy:41)
```
|expected|actual|
|---|---|
|<span>#</span><span>#</span><span>#</span> JMH Benchmark Report|<span>#</span><span>#</span><span>#</span> JMH Benchmark Report|
|||
|\*\*Regressions:\*\* 0|\*\*Regressions:\*\* 0|
|\*\*Improvements:\*\* 1|\*\*Improvements:\*\* 1|
|\*\*Runner health:\*\* not measured|\*\*Runner health:\*\* not measured|
|Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.|Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.|
|||
|Group geometric means are descriptive only, not verdicts.|Group geometric means are descriptive only, not verdicts.|
|||
|\| Group \| Descriptive geomean speedup \| n \||\| Group \| Descriptive geomean speedup \| n \||
|\| --- \| ---: \| ---: \||\| --- \| ---: \| ---: \||
|\| e \| 0.89x \| 3 \||\| e \| 0.89x \| 3 \||
|\| feature \| 1.25x \| 1 \||\| feature \| 1.25x \| 1 \||
|\| sample \| 1.00x \| 3 \||\| sample \| 1.00x \| 3 \||
|||
|\<details\>|\<details\>|
|\<summary\>Per-benchmark results\</summary\>|\<summary\>Per-benchmark results\</summary\>|
|||
|\| Benchmark \| Base score \| Head score \| Speedup \| Verdict \| Allocation delta \(ADVISORY\) \||\| Benchmark \| Base score \| Head score \| Speedup \| Verdict \| Allocation delta \(ADVISORY\) \||
|\| --- \| ---: \| ---: \| ---: \| --- \| ---: \||\| --- \| ---: \| ---: \| ---: \| --- \| ---: \||
|\| feature.Subject.runlabel\=Ruler \| 100 ns/op \| 80 ns/op \| 1.25x \| IMPROVED \| β \||\| feature.Subject.runlabel\=Ruler \| 100 ns/op \| 80 ns/op \| 1.25x \| IMPROVED \| β \||
|\| e.Insufficient.run \| 100 ns/op \| 120 ns/op \| 0.83x \| insufficient data \| β \||\| e.Insufficient.run \| 100 ns/op \| 120 ns/op \| 0.83x \| insufficient data \| β \||
|\| e.NonFinite.run \| 100 ns/op \| 120 ns/op \| 0.83x \| insufficient data \| β \||\| e.NonFinite.run \| 100 ns/op \| 120 ns/op \| 0.83x \| insufficient data \| β \||
|\| e.Shared.run \| 100 ns/op \| 100 ns/op \| 1.00x \| no clear change \| β \||\| e.Shared.run \| 100 ns/op \| 100 ns/op \| 1.00x \| no clear change \| β \||
|\| sample.binjected.run \| 100 ns/op \| 100 ns/op \| 1.00x \| no clear change \| β \||\| sample.binjected.run \| 100 ns/op \| 100 ns/op \| 1.00x \| no clear change \| β \||
|\| sample.Bang.runxy \| 100 ns/op \| 100 ns/op \| 1.00x \| no clear change \| β \||\| sample.Bang.runxy \| 100 ns/op \| 100 ns/op \| 1.00x \| no clear change \| β \||
|\| sample.Evil.runlabel\=xhttp://example.com \| 100 ns/op \| 100 ns/op \| 1.00x \| no clear change \| β \||\| sample.Evil.runlabel\=xhttp://example.com \| 100 ns/op \| 100 ns/op \| 1.00x \| no clear change \| β \||
|||
|\</details\>|\</details\>|
|||
|\*\*Dropped unpaired shard samples \(2\):\*\* e.BaseOnly.run, e.HeadOnly.run|\*\*Dropped unpaired shard samples \(2\):\*\* e.BaseOnly.run, e.HeadOnly.run|
|||
|\*\*Malformed comparisons skipped:\*\* 3|\*\*Malformed comparisons skipped:\*\* 3|
|||
|\<\!-- grails-jmh-benchmark --\>|\<\!-- grails-jmh-benchmark --\>|
|||
</details>
<details>
<summary><strong>GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file > headonly report exactly matches the Python golden file</strong> (:grails-benchmarks:test in <a href="https://github.com/apache/grails-core/actions/runs/30752280498/job/91508824375?pr=16071">CI / Build Grails-Core (windows-latest, 25)</a>)</summary>
```
Condition not satisfied:
Files.readString(output) == resource(expected)
| | | | | |
| | | | | jmh-golden/expected-headonly.md
| | | | ### JMH Benchmark Report
| | | |
| | | | **Regressions:** 0 (no base)
| | | | **Improvements:** 0 (no base)
| | | | **Runner health:** not measured (no base)
| | | |
| | | | No comparison was possible because the base revision has no benchmark harness.
| | | |
| | | | | Benchmark | Head score | Error | Unit |
| | | | | --- | ---: | ---: | --- |
| | | | | databinding.SimpleDataBinderBenchmark.bindFlatMap | 1.83e+04 | 1 | ns/op |
| | | | | gsp.GroovyPageParserBenchmark.parseSmallTemplate | 9.94e+03 | 1 | ns/op |
| | | | | ruler.CpuRulerBenchmark.integerArithmetic | 89 | 1 | ns/op |
| | | | | ruler.MemoryRulerBenchmark.allocateAndCopyArray | 108 | 1 | ns/op |
| | | | | throughput.Ops.run | 120 | 1 | ops/s |
| | | | | urlmappings.UrlMappingsBenchmark.matchColdVariedKeys | 802 | 1 | ns/op |
| | | | | urlmappings.UrlMappingsBenchmark.matchWarmCache | 8.77 | 1 | ns/op |
| | | | | views.ViewTemplateRenderingBenchmark.renderJsonTemplate | 0.46 | 1 | ns/op |
| | | |
| | | | <!-- grails-jmh-benchmark -->
| | | false
| | | Strings too large to calculate edit distance.
| | C:\Users\RUNNER~1\AppData\Local\Temp\spock_headonly_report_exa_5_temporaryDirectory10567053704218395916\headonly.md
| ### JMH Benchmark Report
|
| **Regressions:** 0 (no base)
| **Improvements:** 0 (no base)
| **Runner health:** not measured (no base)
|
| No comparison was possible because the base revision has no benchmark harness.
|
| | Benchmark | Head score | Error | Unit |
| | --- | ---: | ---: | --- |
| | databinding.SimpleDataBinderBenchmark.bindFlatMap | 1.83e+04 | 1 | ns/op |
| | gsp.GroovyPageParserBenchmark.parseSmallTemplate | 9.94e+03 | 1 | ns/op |
| | ruler.CpuRulerBenchmark.integerArithmetic | 89 | 1 | ns/op |
| | ruler.MemoryRulerBenchmark.allocateAndCopyArray | 108 | 1 | ns/op |
| | throughput.Ops.run | 120 | 1 | ops/s |
| | urlmappings.UrlMappingsBenchmark.matchColdVariedKeys | 802 | 1 | ns/op |
| | urlmappings.UrlMappingsBenchmark.matchWarmCache | 8.77 | 1 | ns/op |
| | views.ViewTemplateRenderingBenchmark.renderJsonTemplate | 0.46 | 1 | ns/op |
|
| <!-- grails-jmh-benchmark -->
class java.nio.file.Files
at org.apache.grails.benchmarks.report.GoldenReportSpec.#name report exactly matches the Python golden file(GoldenReportSpec.groovy:41)
```
|expected|actual|
|---|---|
|<span>#</span><span>#</span><span>#</span> JMH Benchmark Report|<span>#</span><span>#</span><span>#</span> JMH Benchmark Report|
|||
|\*\*Regressions:\*\* 0 \(no base\)|\*\*Regressions:\*\* 0 \(no base\)|
|\*\*Improvements:\*\* 0 \(no base\)|\*\*Improvements:\*\* 0 \(no base\)|
|\*\*Runner health:\*\* not measured \(no base\)|\*\*Runner health:\*\* not measured \(no base\)|
|||
|No comparison was possible because the base revision has no benchmark harness.|No comparison was possible because the base revision has no benchmark harness.|
|||
|\| Benchmark \| Head score \| Error \| Unit \||\| Benchmark \| Head score \| Error \| Unit \||
|\| --- \| ---: \| ---: \| --- \||\| --- \| ---: \| ---: \| --- \||
|\| databinding.SimpleDataBinderBenchmark.bindFlatMap \| 1.83e\+04 \| 1 \| ns/op \||\| databinding.SimpleDataBinderBenchmark.bindFlatMap \| 1.83e\+04 \| 1 \| ns/op \||
|\| gsp.GroovyPageParserBenchmark.parseSmallTemplate \| 9.94e\+03 \| 1 \| ns/op \||\| gsp.GroovyPageParserBenchmark.parseSmallTemplate \| 9.94e\+03 \| 1 \| ns/op \||
|\| ruler.CpuRulerBenchmark.integerArithmetic \| 89 \| 1 \| ns/op \||\| ruler.CpuRulerBenchmark.integerArithmetic \| 89 \| 1 \| ns/op \||
|\| ruler.MemoryRulerBenchmark.allocateAndCopyArray \| 108 \| 1 \| ns/op \||\| ruler.MemoryRulerBenchmark.allocateAndCopyArray \| 108 \| 1 \| ns/op \||
|\| throughput.Ops.run \| 120 \| 1 \| ops/s \||\| throughput.Ops.run \| 120 \| 1 \| ops/s \||
|\| urlmappings.UrlMappingsBenchmark.matchColdVariedKeys \| 802 \| 1 \| ns/op \||\| urlmappings.UrlMappingsBenchmark.matchColdVariedKeys \| 802 \| 1 \| ns/op \||
|\| urlmappings.UrlMappingsBenchmark.matchWarmCache \| 8.77 \| 1 \| ns/op \||\| urlmappings.UrlMappingsBenchmark.matchWarmCache \| 8.77 \| 1 \| ns/op \||
|\| views.ViewTemplateRenderingBenchmark.renderJsonTemplate \| 0.46 \| 1 \| ns/op \||\| views.ViewTemplateRenderingBenchmark.renderJsonTemplate \| 0.46 \| 1 \| ns/op \||
|||
|\<\!-- grails-jmh-benchmark --\>|\<\!-- grails-jmh-benchmark --\>|
|||
</details>
<details>
<summary><strong>GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file > norulers report exactly matches the Python golden file</strong> (:grails-benchmarks:test in <a href="https://github.com/apache/grails-core/actions/runs/30752280498/job/91508824375?pr=16071">CI / Build Grails-Core (windows-latest, 25)</a>)</summary>
```
Condition not satisfied:
Files.readString(output) == resource(expected)
| | | | | |
| | | | | jmh-golden/expected-norulers.md
| | | | ### JMH Benchmark Report
| | | |
| | | | **Regressions:** 1
| | | | **Improvements:** 1
| | | | **Runner health:** not measured
| | | | Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.
| | | |
| | | | Group geometric means are descriptive only, not verdicts.
| | | |
| | | | | Group | Descriptive geomean speedup | n |
| | | | | --- | ---: | ---: |
| | | | | group | 1.00x | 2 |
| | | |
| | | | <details>
| | | | <summary>Per-benchmark results</summary>
| | | |
| | | | | Benchmark | Base score | Head score | Speedup | Verdict | Allocation delta (ADVISORY) |
| | | | | --- | ---: | ---: | ---: | --- | ---: |
| | | | | g.group.First.run | 100 ns/op | 200 ns/op | 0.50x | REGRESSED | β |
| | | | | g.group.Second.run | 100 ns/op | 50 ns/op | 2.00x | IMPROVED | β |
| | | |
| | | | </details>
| | | |
| | | | <!-- grails-jmh-benchmark -->
| | | false
| | | Strings too large to calculate edit distance.
| | C:\Users\RUNNER~1\AppData\Local\Temp\spock_norulers_report_exa_3_temporaryDirectory17048898167073638243\norulers.md
| ### JMH Benchmark Report
|
| **Regressions:** 1
| **Improvements:** 1
| **Runner health:** not measured
| Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.
|
| Group geometric means are descriptive only, not verdicts.
|
| | Group | Descriptive geomean speedup | n |
| | --- | ---: | ---: |
| | group | 1.00x | 2 |
|
| <details>
| <summary>Per-benchmark results</summary>
|
| | Benchmark | Base score | Head score | Speedup | Verdict | Allocation delta (ADVISORY) |
| | --- | ---: | ---: | ---: | --- | ---: |
| | g.group.First.run | 100 ns/op | 200 ns/op | 0.50x | REGRESSED | β |
| | g.group.Second.run | 100 ns/op | 50 ns/op | 2.00x | IMPROVED | β |
|
| </details>
|
| <!-- grails-jmh-benchmark -->
class java.nio.file.Files
at org.apache.grails.benchmarks.report.GoldenReportSpec.#name report exactly matches the Python golden file(GoldenReportSpec.groovy:41)
```
|expected|actual|
|---|---|
|<span>#</span><span>#</span><span>#</span> JMH Benchmark Report|<span>#</span><span>#</span><span>#</span> JMH Benchmark Report|
|||
|\*\*Regressions:\*\* 1|\*\*Regressions:\*\* 1|
|\*\*Improvements:\*\* 1|\*\*Improvements:\*\* 1|
|\*\*Runner health:\*\* not measured|\*\*Runner health:\*\* not measured|
|Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.|Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.|
|||
|Group geometric means are descriptive only, not verdicts.|Group geometric means are descriptive only, not verdicts.|
|||
|\| Group \| Descriptive geomean speedup \| n \||\| Group \| Descriptive geomean speedup \| n \||
|\| --- \| ---: \| ---: \||\| --- \| ---: \| ---: \||
|\| group \| 1.00x \| 2 \||\| group \| 1.00x \| 2 \||
|||
|\<details\>|\<details\>|
|\<summary\>Per-benchmark results\</summary\>|\<summary\>Per-benchmark results\</summary\>|
|||
|\| Benchmark \| Base score \| Head score \| Speedup \| Verdict \| Allocation delta \(ADVISORY\) \||\| Benchmark \| Base score \| Head score \| Speedup \| Verdict \| Allocation delta \(ADVISORY\) \||
|\| --- \| ---: \| ---: \| ---: \| --- \| ---: \||\| --- \| ---: \| ---: \| ---: \| --- \| ---: \||
|\| g.group.First.run \| 100 ns/op \| 200 ns/op \| 0.50x \| REGRESSED \| β \||\| g.group.First.run \| 100 ns/op \| 200 ns/op \| 0.50x \| REGRESSED \| β \||
|\| g.group.Second.run \| 100 ns/op \| 50 ns/op \| 2.00x \| IMPROVED \| β \||\| g.group.Second.run \| 100 ns/op \| 50 ns/op \| 2.00x \| IMPROVED \| β \||
|||
|\</details\>|\</details\>|
|||
|\<\!-- grails-jmh-benchmark --\>|\<\!-- grails-jmh-benchmark --\>|
|||
</details>
<details>
<summary><strong>GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file > numbers report exactly matches the Python golden file</strong> (:grails-benchmarks:test in <a href="https://github.com/apache/grails-core/actions/runs/30752280498/job/91508824375?pr=16071">CI / Build Grails-Core (windows-latest, 25)</a>)</summary>
```
Condition not satisfied:
Files.readString(output) == resource(expected)
| | | | | |
| | | | | jmh-golden/expected-numbers.md
| | | | ### JMH Benchmark Report
| | | |
| | | | **Regressions:** 0
| | | | **Improvements:** 0
| | | | **Runner health:** not measured
| | | | Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.
| | | |
| | | | Group geometric means are descriptive only, not verdicts.
| | | |
| | | | | Group | Descriptive geomean speedup | n |
| | | | | --- | ---: | ---: |
| | | | | n | 1.00x | 10 |
| | | |
| | | | <details>
| | | | <summary>Per-benchmark results</summary>
| | | |
| | | | | Benchmark | Base score | Head score | Speedup | Verdict | Allocation delta (ADVISORY) |
| | | | | --- | ---: | ---: | ---: | --- | ---: |
| | | | | n.A.run | 0.46 ns/op | 0.46 ns/op | 1.00x | no clear change | β |
| | | | | n.B.run | 100 ns/op | 100 ns/op | 1.00x | no clear change | β |
| | | | | n.C.run | 802 ns/op | 802 ns/op | 1.00x | no clear change | β |
| | | | | n.D.run | 1.83e+04 ns/op | 1.83e+04 ns/op | 1.00x | no clear change | β |
| | | | | n.E.run | 1 ns/op | 1 ns/op | 1.00x | no clear change | β |
| | | | | n.F.run | 0.000123 ns/op | 0.000123 ns/op | 1.00x | no clear change | β |
| | | | | n.G.run | 1.23e+08 ns/op | 1.23e+08 ns/op | 1.00x | no clear change | β |
| | | | | n.H.run | 1.5 ns/op | 1.5 ns/op | 1.00x | no clear change | β |
| | | | | n.I.run | 1e+06 ns/op | 1e+06 ns/op | 1.00x | no clear change | β |
| | | | | n.J.run | 0.1 ns/op | 0.1 ns/op | 1.00x | no clear change | β |
| | | |
| | | | </details>
| | | |
| | | | <!-- grails-jmh-benchmark -->
| | | false
| | | Strings too large to calculate edit distance.
| | C:\Users\RUNNER~1\AppData\Local\Temp\spock_numbers_report_exac_1_temporaryDirectory9881359309060659325\numbers.md
| ### JMH Benchmark Report
|
| **Regressions:** 0
| **Improvements:** 0
| **Runner health:** not measured
| Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.
|
| Group geometric means are descriptive only, not verdicts.
|
| | Group | Descriptive geomean speedup | n |
| | --- | ---: | ---: |
| | n | 1.00x | 10 |
|
| <details>
| <summary>Per-benchmark results</summary>
|
| | Benchmark | Base score | Head score | Speedup | Verdict | Allocation delta (ADVISORY) |
| | --- | ---: | ---: | ---: | --- | ---: |
| | n.A.run | 0.46 ns/op | 0.46 ns/op | 1.00x | no clear change | β |
| | n.B.run | 100 ns/op | 100 ns/op | 1.00x | no clear change | β |
| | n.C.run | 802 ns/op | 802 ns/op | 1.00x | no clear change | β |
| | n.D.run | 1.83e+04 ns/op | 1.83e+04 ns/op | 1.00x | no clear change | β |
| | n.E.run | 1 ns/op | 1 ns/op | 1.00x | no clear change | β |
| | n.F.run | 0.000123 ns/op | 0.000123 ns/op | 1.00x | no clear change | β |
| | n.G.run | 1.23e+08 ns/op | 1.23e+08 ns/op | 1.00x | no clear change | β |
| | n.H.run | 1.5 ns/op | 1.5 ns/op | 1.00x | no clear change | β |
| | n.I.run | 1e+06 ns/op | 1e+06 ns/op | 1.00x | no clear change | β |
| | n.J.run | 0.1 ns/op | 0.1 ns/op | 1.00x | no clear change | β |
|
| </details>
|
| <!-- grails-jmh-benchmark -->
class java.nio.file.Files
at org.apache.grails.benchmarks.report.GoldenReportSpec.#name report exactly matches the Python golden file(GoldenReportSpec.groovy:41)
```
|expected|actual|
|---|---|
|<span>#</span><span>#</span><span>#</span> JMH Benchmark Report|<span>#</span><span>#</span><span>#</span> JMH Benchmark Report|
|||
|\*\*Regressions:\*\* 0|\*\*Regressions:\*\* 0|
|\*\*Improvements:\*\* 0|\*\*Improvements:\*\* 0|
|\*\*Runner health:\*\* not measured|\*\*Runner health:\*\* not measured|
|Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.|Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.|
|||
|Group geometric means are descriptive only, not verdicts.|Group geometric means are descriptive only, not verdicts.|
|||
|\| Group \| Descriptive geomean speedup \| n \||\| Group \| Descriptive geomean speedup \| n \||
|\| --- \| ---: \| ---: \||\| --- \| ---: \| ---: \||
|\| n \| 1.00x \| 10 \||\| n \| 1.00x \| 10 \||
|||
|\<details\>|\<details\>|
|\<summary\>Per-benchmark results\</summary\>|\<summary\>Per-benchmark results\</summary\>|
|||
|\| Benchmark \| Base score \| Head score \| Speedup \| Verdict \| Allocation delta \(ADVISORY\) \||\| Benchmark \| Base score \| Head score \| Speedup \| Verdict \| Allocation delta \(ADVISORY\) \||
|\| --- \| ---: \| ---: \| ---: \| --- \| ---: \||\| --- \| ---: \| ---: \| ---: \| --- \| ---: \||
|\| n.A.run \| 0.46 ns/op \| 0.46 ns/op \| 1.00x \| no clear change \| β \||\| n.A.run \| 0.46 ns/op \| 0.46 ns/op \| 1.00x \| no clear change \| β \||
|\| n.B.run \| 100 ns/op \| 100 ns/op \| 1.00x \| no clear change \| β \||\| n.B.run \| 100 ns/op \| 100 ns/op \| 1.00x \| no clear change \| β \||
|\| n.C.run \| 802 ns/op \| 802 ns/op \| 1.00x \| no clear change \| β \||\| n.C.run \| 802 ns/op \| 802 ns/op \| 1.00x \| no clear change \| β \||
|\| n.D.run \| 1.83e\+04 ns/op \| 1.83e\+04 ns/op \| 1.00x \| no clear change \| β \||\| n.D.run \| 1.83e\+04 ns/op \| 1.83e\+04 ns/op \| 1.00x \| no clear change \| β \||
|\| n.E.run \| 1 ns/op \| 1 ns/op \| 1.00x \| no clear change \| β \||\| n.E.run \| 1 ns/op \| 1 ns/op \| 1.00x \| no clear change \| β \||
|\| n.F.run \| 0.000123 ns/op \| 0.000123 ns/op \| 1.00x \| no clear change \| β \||\| n.F.run \| 0.000123 ns/op \| 0.000123 ns/op \| 1.00x \| no clear change \| β \||
|\| n.G.run \| 1.23e\+08 ns/op \| 1.23e\+08 ns/op \| 1.00x \| no clear change \| β \||\| n.G.run \| 1.23e\+08 ns/op \| 1.23e\+08 ns/op \| 1.00x \| no clear change \| β \||
|\| n.H.run \| 1.5 ns/op \| 1.5 ns/op \| 1.00x \| no clear change \| β \||\| n.H.run \| 1.5 ns/op \| 1.5 ns/op \| 1.00x \| no clear change \| β \||
|\| n.I.run \| 1e\+06 ns/op \| 1e\+06 ns/op \| 1.00x \| no clear change \| β \||\| n.I.run \| 1e\+06 ns/op \| 1e\+06 ns/op \| 1.00x \| no clear change \| β \||
|\| n.J.run \| 0.1 ns/op \| 0.1 ns/op \| 1.00x \| no clear change \| β \||\| n.J.run \| 0.1 ns/op \| 0.1 ns/op \| 1.00x \| no clear change \| β \||
|||
|\</details\>|\</details\>|
|||
|\<\!-- grails-jmh-benchmark --\>|\<\!-- grails-jmh-benchmark --\>|
|||
</details>
<details>
<summary><strong>GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file > shards report exactly matches the Python golden file</strong> (:grails-benchmarks:test in <a href="https://github.com/apache/grails-core/actions/runs/30752280498/job/91508824375?pr=16071">CI / Build Grails-Core (windows-latest, 25)</a>)</summary>
```
Condition not satisfied:
Files.readString(output) == resource(expected)
| | | | | |
| | | | | jmh-golden/expected-shards.md
| | | | ### JMH Benchmark Report
| | | |
| | | | **Regressions:** 0
| | | | **Improvements:** 0
| | | | **Runner health:** worst ruler deviation: 12.0% in shard-b.json (CpuRuler.run)
| | | | Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.
| | | |
| | | | > **Warning:** No usable base/head pair was produced by shard-c.json. This comparison rests on 2 shard pair(s) instead of the expected 3, so the alternating measurement order did not fully cancel and the result is weaker than a normal run.
| | | | **Ruler movements:** shard-a.json: CpuRuler.run: 0.89x, shard-b.json: CpuRuler.run: 1.12x
| | | |
| | | | > **Warning:** The runner was unstable BETWEEN the two halves of the A/B run. Treat results as unreliable. Runner health is a stability check, not a calibration factor.
| | | |
| | | | Group geometric means are descriptive only, not verdicts.
| | | |
| | | | | Group | Descriptive geomean speedup | n |
| | | | | --- | ---: | ---: |
| | | | | s | 0.95x | 1 |
| | | |
| | | | <details>
| | | | <summary>Per-benchmark results</summary>
| | | |
| | | | | Benchmark | Base score | Head score | Speedup | Verdict | Allocation delta (ADVISORY) |
| | | | | --- | ---: | ---: | ---: | --- | ---: |
| | | | | ruler.CpuRuler.run | 100 ns/op | 101 ns/op | 0.99x | ruler - excluded | β |
| | | | | s.Paired.run | 100 ns/op | 105 ns/op | 0.95x | no clear change | +200 B/op (+200.0%) **candidate** |
| | | |
| | | | </details>
| | | |
| | | | **Dropped unpaired shard samples (1):** s.CrossShard.run
| | | |
| | | | <!-- grails-jmh-benchmark -->
| | | false
| | | Strings too large to calculate edit distance.
| | C:\Users\RUNNER~1\AppData\Local\Temp\spock_shards_report_exact_4_temporaryDirectory2033684034873501116\shards.md
| ### JMH Benchmark Report
|
| **Regressions:** 0
| **Improvements:** 0
| **Runner health:** worst ruler deviation: 12.0% in shard-b.json (CpuRuler.run)
| Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.
|
| > **Warning:** No usable base/head pair was produced by shard-c.json. This comparison rests on 2 shard pair(s) instead of the expected 3, so the alternating measurement order did not fully cancel and the result is weaker than a normal run.
| **Ruler movements:** shard-a.json: CpuRuler.run: 0.89x, shard-b.json: CpuRuler.run: 1.12x
|
| > **Warning:** The runner was unstable BETWEEN the two halves of the A/B run. Treat results as unreliable. Runner health is a stability check, not a calibration factor.
|
| Group geometric means are descriptive only, not verdicts.
|
| | Group | Descriptive geomean speedup | n |
| | --- | ---: | ---: |
| | s | 0.95x | 1 |
|
| <details>
| <summary>Per-benchmark results</summary>
|
| | Benchmark | Base score | Head score | Speedup | Verdict | Allocation delta (ADVISORY) |
| | --- | ---: | ---: | ---: | --- | ---: |
| | ruler.CpuRuler.run | 100 ns/op | 101 ns/op | 0.99x | ruler - excluded | β |
| | s.Paired.run | 100 ns/op | 105 ns/op | 0.95x | no clear change | +200 B/op (+200.0%) **candidate** |
|
| </details>
|
| **Dropped unpaired shard samples (1):** s.CrossShard.run
|
| <!-- grails-jmh-benchmark -->
class java.nio.file.Files
at org.apache.grails.benchmarks.report.GoldenReportSpec.#name report exactly matches the Python golden file(GoldenReportSpec.groovy:41)
```
|expected|actual|
|---|---|
|<span>#</span><span>#</span><span>#</span> JMH Benchmark Report|<span>#</span><span>#</span><span>#</span> JMH Benchmark Report|
|||
|\*\*Regressions:\*\* 0|\*\*Regressions:\*\* 0|
|\*\*Improvements:\*\* 0|\*\*Improvements:\*\* 0|
|\*\*Runner health:\*\* worst ruler deviation: 12.0\% in shard-b.json \(CpuRuler.run\)|\*\*Runner health:\*\* worst ruler deviation: 12.0\% in shard-b.json \(CpuRuler.run\)|
|Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.|Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.|
|||
|\> \*\*Warning:\*\* No usable base/head pair was produced by shard-c.json. This comparison rests on 2 shard pair\(s\) instead of the expected 3, so the alternating measurement order did not fully cancel and the result is weaker than a normal run.|\> \*\*Warning:\*\* No usable base/head pair was produced by shard-c.json. This comparison rests on 2 shard pair\(s\) instead of the expected 3, so the alternating measurement order did not fully cancel and the result is weaker than a normal run.|
|\*\*Ruler movements:\*\* shard-a.json: CpuRuler.run: 0.89x, shard-b.json: CpuRuler.run: 1.12x|\*\*Ruler movements:\*\* shard-a.json: CpuRuler.run: 0.89x, shard-b.json: CpuRuler.run: 1.12x|
|||
|\> \*\*Warning:\*\* The runner was unstable BETWEEN the two halves of the A/B run. Treat results as unreliable. Runner health is a stability check, not a calibration factor.|\> \*\*Warning:\*\* The runner was unstable BETWEEN the two halves of the A/B run. Treat results as unreliable. Runner health is a stability check, not a calibration factor.|
|||
|Group geometric means are descriptive only, not verdicts.|Group geometric means are descriptive only, not verdicts.|
|||
|\| Group \| Descriptive geomean speedup \| n \||\| Group \| Descriptive geomean speedup \| n \||
|\| --- \| ---: \| ---: \||\| --- \| ---: \| ---: \||
|\| s \| 0.95x \| 1 \||\| s \| 0.95x \| 1 \||
|||
|\<details\>|\<details\>|
|\<summary\>Per-benchmark results\</summary\>|\<summary\>Per-benchmark results\</summary\>|
|||
|\| Benchmark \| Base score \| Head score \| Speedup \| Verdict \| Allocation delta \(ADVISORY\) \||\| Benchmark \| Base score \| Head score \| Speedup \| Verdict \| Allocation delta \(ADVISORY\) \||
|\| --- \| ---: \| ---: \| ---: \| --- \| ---: \||\| --- \| ---: \| ---: \| ---: \| --- \| ---: \||
|\| ruler.CpuRuler.run \| 100 ns/op \| 101 ns/op \| 0.99x \| ruler - excluded \| β \||\| ruler.CpuRuler.run \| 100 ns/op \| 101 ns/op \| 0.99x \| ruler - excluded \| β \||
|\| s.Paired.run \| 100 ns/op \| 105 ns/op \| 0.95x \| no clear change \| \+200 B/op \(\+200.0\%\) \*\*candidate\*\* \||\| s.Paired.run \| 100 ns/op \| 105 ns/op \| 0.95x \| no clear change \| \+200 B/op \(\+200.0\%\) \*\*candidate\*\* \||
|||
|\</details\>|\</details\>|
|||
|\*\*Dropped unpaired shard samples \(1\):\*\* s.CrossShard.run|\*\*Dropped unpaired shard samples \(1\):\*\* s.CrossShard.run|
|||
|\<\!-- grails-jmh-benchmark --\>|\<\!-- grails-jmh-benchmark --\>|
|||
</details>
### Muted Tests
> [!NOTE]
> Checks are currently running using the configuration below.
Select tests to mute in this pull request:
π² GoldenReportSpec > <span>#</span>name report exactly matches the Python golden file <!-- uniqueId=[engine:spock]/[spec:org.apache.grails.benchmarks.report.GoldenReportSpec]/[feature:$spock_feature_0_0] -->
Reuse successful test results:
π² β»οΈ Only rerun the tests that failed or were muted before
Click the checkbox to trigger a rerun:
π² **Rerun jobs**
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#discussion_r3721632692
##########
grails-benchmarks/src/report/groovy/org/apache/grails/benchmarks/report/ReportRenderer.groovy:
##########
@@ -0,0 +1,148 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.grails.benchmarks.report
+
+import groovy.transform.CompileStatic
+
+@CompileStatic
+class ReportRenderer {
+
+ static final String MARKER = '<!-- grails-jmh-benchmark -->'
+ static final String DASH = 'β'
+ static final String BENCHMARK_PACKAGE = 'org.apache.grails.benchmarks.'
+
+ static String safe(String value) {
+ value.replaceAll('[`|<>\\[\\]\\(\\)!\\r\\n]', '')
+ }
+
+ static String number(Double value) {
+ if (value == null || !Double.isFinite(value)) {
+ return DASH
+ }
+ String rendered = String.format(Locale.ROOT, '%.3g', value)
+ int exponent = rendered.indexOf('e')
+ String mantissa = exponent >= 0 ? rendered.substring(0, exponent) : rendered
+ String suffix = exponent >= 0 ? rendered.substring(exponent) : ''
+ if (mantissa.contains('.')) {
+ mantissa = mantissa.replaceFirst('0+$', '').replaceFirst('\\.$', '')
+ }
+ return mantissa + suffix
+ }
+
+ static String speedup(Double value) {
+ value != null && Double.isFinite(value)
+ ? String.format(Locale.ROOT, '%.2fx', value)
+ : DASH
+ }
+
+ static String identity(String value) {
+ safe(removePrefix(value, BENCHMARK_PACKAGE))
+ }
+
+ static String score(Benchmark value) {
+ String rendered = number(value.score)
+ rendered == DASH ? DASH : rendered + ' ' + safe(value.unit)
+ }
+
+ static String allocation(ComparisonRow row) {
+ if (row.allocationDelta == null || row.allocationPercent == null) {
+ return DASH
+ }
+ if (Math.abs(row.allocationDelta) < 1D) {
+ return '~0 B/op'
+ }
+ return String.format(Locale.ROOT, '%+,.0f B/op (%+.1f%%)%s',
+ row.allocationDelta,
+ row.allocationPercent * 100D,
+ row.allocationCandidate ? ' **candidate**' : '')
+ }
+
+ static String render(Comparison comparison) {
+ int regressions = comparison.rows.count { ComparisonRow row -> row.verdict == 'REGRESSED' }.intValue()
+ int improvements = comparison.rows.count { ComparisonRow row -> row.verdict == 'IMPROVED' }.intValue()
+ List<RulerDeviation> deviations = comparison.rulerDeviations ?: comparison.rulerSpeedups.collect { List<Object> item -> new RulerDeviation('', (String) item[0], (Double) item[1]) }
+ RulerDeviation worst = deviations ? deviations.max { RulerDeviation item -> deviation(item.speedup) } : null
+ String health = comparison.rulerIncomplete
+ ? 'INCOMPLETE/unreliable (missing or non-finite ruler measurements)'
+ : worst == null
+ ? 'not measured'
+ : "worst ruler deviation: ${String.format(Locale.ROOT, '%.1f%%', deviation(worst.speedup) * 100D)}${worst.shard ? ' in ' + safe(worst.shard) : ''} (${safe(removePrefix(worst.identity, BenchmarkComparator.RULER_PACKAGE))})"
+ List<String> lines = ['### JMH Benchmark Report', '', "**Regressions:** ${regressions}".toString(), "**Improvements:** ${improvements}".toString(), "**Runner health:** ${health}".toString(), 'Ruler benchmarks are excluded from verdicts and group summaries; runner health is a stability check, not a calibration factor.']
+ if (comparison.missingShards) lines.addAll(['', '> **Warning:** No usable base/head pair was produced by ' + comparison.missingShards.collect { String item -> safe(item) }.join(', ') + ". This comparison rests on ${comparison.pairedShards.size()} shard pair(s) instead of the expected ${comparison.pairedShards.size() + comparison.missingShards.size()}, so the alternating measurement order did not fully cancel and the result is weaker than a normal run."])
+ if (deviations) {
+ lines.add('**Ruler movements:** ' + deviations.collect { RulerDeviation item ->
Review Comment:
Line 85 emits a `>` blockquote and this line follows it with no blank line, so GFM lazy continuation pulls "Ruler movements" *inside* the warning blockquote. Confirmed against GitHub's own renderer (`POST /markdown`, `mode: gfm`):
```html
<blockquote>
<p><strong>Warning:</strong> No usable base/head pair was produced by shard-c.json.<br>
<strong>Ruler movements:</strong> shard-a.json: CpuRuler.run: 0.89x, ...</p>
</blockquote>
```
This only fires when `missingShards` is non-empty β which is exactly the degraded run the warning exists to call out, and the one where the ruler numbers matter most. Prefixing the `lines.add` with `''` fixes it.
The two warnings below (lines 93 and 94) already do this correctly, so it's just this one.
Worth noting the golden fixtures can't catch this class of bug: they lock the Markdown *source* bytes (`expected-shards.md` lines 8-9 encode the current, broken layout), not the rendered output.
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,396 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
+ paths-ignore:
+ - '**/*.md'
+ - '**/*.adoc'
+ - 'grails-doc/**'
+ workflow_dispatch:
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+permissions:
+ contents: read
+
+jobs:
+ # Each shard builds both revisions before measuring either one on the same runner.
+ # Building first avoids CPU, IO, thermal, cache, and frequency state biasing a measurement.
+ # Shard a measures BASE then HEAD, while shard b measures HEAD then BASE.
+ # Alternating the order cancels first-versus-second ordering bias without doubling runtime.
+ # Both shards use the PR merge commit as HEAD, so they measure what would actually land.
+ # Results are advisory: a detected regression never fails this workflow.
+ benchmark:
+ name: "Paired JMH benchmarks (${{ matrix.shard }})"
+ if: >-
+ github.event_name == 'workflow_dispatch' ||
+ (contains(github.event.pull_request.labels.*.name, 'performance') &&
+ (github.event.action != 'labeled' || github.event.label.name == 'performance'))
+ runs-on: ubuntu-24.04
+ strategy:
+ fail-fast: false
+ matrix:
+ shard: [a, b]
+ env:
+ BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ HEAD_SHA: ${{ github.sha }}
+ JMH_INCLUDE: '.*'
+ RESULT_DIR: ${{ github.workspace }}/jmh-results/${{ matrix.shard }}
+ REPORT_DIR: ${{ github.workspace }}/jmh-reports/${{ matrix.shard }}
+ SHARD: ${{ matrix.shard }}
+ steps:
+ - name: "π₯ Checkout repository"
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+ with:
+ fetch-depth: 0
+ - name: "βοΈ Setup JDK"
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
+ with:
+ distribution: liberica
+ java-version: 21
+ - name: "π Setup Gradle"
+ uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0
+ with:
+ cache-provider: basic # 'basic' uses the MIT-licensed, open-source cache provider; the default 'enhanced' provider (v6+) is proprietary (Gradle commercial Terms of Use)
+ develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+ # Deliberately allowed to fail the job, and placed before the expensive work: a broken
+ # comparison tool 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 tool"
+ run: ./gradlew :grails-benchmarks:test --max-workers=4
Review Comment:
This gate is placed first, and the comment above it explains why: a broken comparison tool should fail in seconds rather than after an hour of building and benchmarking two revisions. But `:grails-benchmarks:test` isn't seconds β it builds 33 framework projects before it can run a single spec.
The `test` source set inherits `main`'s output, and `main` is where the Groovy fixtures live, so the six `implementation project(...)` dependencies come along. Measured on this branch:
| gate | tasks | projects |
| --- | ---: | ---: |
| `:grails-benchmarks:test` (current) | 222 | 33 |
| isolated `reportTest` source set | 9 | 1 |
I probed the isolated version β a `reportTest` source set whose classpath is `sourceSets.report.output` plus Spock and `groovy-json`, with its own `Test` task β and every spec passes unchanged in 19s including compilation. No spec touches the fixtures, so nothing is lost by cutting `main` out of the classpath.
`README.adoc` already states this as the design intent: "The `report` source set depends on Groovy alone and on no Grails project." The *test* source set is what breaks that isolation.
One thing to watch if you split it: `grails-test-report` collects test tasks with `tasks.withType(Test).matching { it.name == phase }`, so a task named anything other than `test` drops out of the aggregate report and would need adding there.
##########
grails-benchmarks/src/jmh/java/org/apache/grails/benchmarks/gsp/GroovyPageParserBenchmark.java:
##########
@@ -0,0 +1,83 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.grails.benchmarks.gsp;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.util.concurrent.TimeUnit;
+
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Threads;
+import org.openjdk.jmh.annotations.Warmup;
+
+import org.grails.gsp.compiler.GroovyPageParser;
+
+/**
+ * Measures GSP source parsing into generated Groovy source, the work performed when GSP templates
+ * are prepared. Parser regressions slow application startup and template reloads.
+ */
+@State(Scope.Benchmark)
+@Threads(1)
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
+@Fork(value = 2, jvmArgsAppend = {"-Xms1g", "-Xmx1g", "-XX:+UseG1GC"})
+public class GroovyPageParserBenchmark {
Review Comment:
This is the one benchmark left without a fixture guard β it has no `@Setup` at all. `UrlMappingsBenchmark`, `UrlMappingMatcherBenchmark`, `SimpleDataBinderBenchmark` and `ViewTemplateRenderingBenchmark` all now fail the run rather than publish a silent no-op.
The same failure mode applies here, and arguably more directly than anywhere else: the measured work *is* the volume of generated source. If `parse()` ever returned an empty or near-empty stream, both methods would keep producing plausible, self-consistent, meaningless numbers with no error anywhere.
A `@Setup` that parses both templates once and asserts the generated source is non-trivial (and, for `TAGGED_TEMPLATE`, that it actually contains the tag output) would close the last gap in the fail-loud story.
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,396 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
+ paths-ignore:
+ - '**/*.md'
+ - '**/*.adoc'
+ - 'grails-doc/**'
+ workflow_dispatch:
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+permissions:
+ contents: read
+
+jobs:
+ # Each shard builds both revisions before measuring either one on the same runner.
+ # Building first avoids CPU, IO, thermal, cache, and frequency state biasing a measurement.
+ # Shard a measures BASE then HEAD, while shard b measures HEAD then BASE.
+ # Alternating the order cancels first-versus-second ordering bias without doubling runtime.
+ # Both shards use the PR merge commit as HEAD, so they measure what would actually land.
+ # Results are advisory: a detected regression never fails this workflow.
+ benchmark:
+ name: "Paired JMH benchmarks (${{ matrix.shard }})"
+ if: >-
+ github.event_name == 'workflow_dispatch' ||
+ (contains(github.event.pull_request.labels.*.name, 'performance') &&
+ (github.event.action != 'labeled' || github.event.label.name == 'performance'))
+ runs-on: ubuntu-24.04
+ strategy:
+ fail-fast: false
+ matrix:
+ shard: [a, b]
+ env:
+ BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ HEAD_SHA: ${{ github.sha }}
+ JMH_INCLUDE: '.*'
+ RESULT_DIR: ${{ github.workspace }}/jmh-results/${{ matrix.shard }}
+ REPORT_DIR: ${{ github.workspace }}/jmh-reports/${{ matrix.shard }}
+ SHARD: ${{ matrix.shard }}
+ steps:
+ - name: "π₯ Checkout repository"
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+ with:
+ fetch-depth: 0
+ - name: "βοΈ Setup JDK"
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
+ with:
+ distribution: liberica
+ java-version: 21
+ - name: "π Setup Gradle"
+ uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0
+ with:
+ cache-provider: basic # 'basic' uses the MIT-licensed, open-source cache provider; the default 'enhanced' provider (v6+) is proprietary (Gradle commercial Terms of Use)
+ develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+ # Deliberately allowed to fail the job, and placed before the expensive work: a broken
+ # comparison tool 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 tool"
+ run: ./gradlew :grails-benchmarks:test --max-workers=4
+ - name: "π³ Prepare paired worktrees"
+ run: |
+ WORKTREE_ROOT="$RUNNER_TEMP/jmh-worktrees/$SHARD"
+ mkdir -p "$WORKTREE_ROOT" "$RESULT_DIR" "$REPORT_DIR"
+ if [ -n "$BASE_SHA" ] && git cat-file -e "$BASE_SHA^{commit}" 2>/dev/null; then
+ RESOLVED_BASE_SHA="$BASE_SHA"
+ elif RESOLVED_BASE_SHA="$(git merge-base "$HEAD_SHA^1" "$HEAD_SHA^2" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using merge-base $RESOLVED_BASE_SHA."
+ elif RESOLVED_BASE_SHA="$(git rev-parse "$HEAD_SHA^" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using HEAD parent $RESOLVED_BASE_SHA."
+ else
+ RESOLVED_BASE_SHA=""
+ echo "No base commit could be resolved; comparison will use HEAD-only mode."
+ fi
+ git worktree add --detach "$WORKTREE_ROOT/head" "$HEAD_SHA"
+ {
+ printf 'HEAD_DIR=%s\n' "$WORKTREE_ROOT/head"
+ printf 'WORKTREE_ROOT=%s\n' "$WORKTREE_ROOT"
+ } >> "$GITHUB_ENV"
+ if [ -n "$RESOLVED_BASE_SHA" ] && git worktree add --detach "$WORKTREE_ROOT/base" "$RESOLVED_BASE_SHA"; then
+ {
+ printf 'BASE_DIR=%s\n' "$WORKTREE_ROOT/base"
+ printf 'RESOLVED_BASE_SHA=%s\n' "$RESOLVED_BASE_SHA"
+ } >> "$GITHUB_ENV"
+ echo "Using base commit $RESOLVED_BASE_SHA."
+ else
+ echo "BASE_BENCHMARKS_AVAILABLE=false" >> "$GITHUB_ENV"
+ fi
+ # Build both JMH jars before measuring either revision. Builds are CPU- and IO-heavy,
+ # so building and measuring one revision at a time would bias results with different
+ # thermal, cache, and CPU-frequency state on the shared runner.
+ - name: "π¨ Build paired JMH jars"
+ timeout-minutes: 60
+ run: |
+ base_build_ok=false
+ head_build_ok=false
+ if [ "${BASE_BENCHMARKS_AVAILABLE:-true}" = "true" ] && [ -f "$BASE_DIR/grails-benchmarks/build.gradle" ]; then
+ if (
+ cd "$BASE_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ base_build_ok=true
+ echo "Built base benchmark at $RESOLVED_BASE_SHA."
+ else
+ echo "BASE benchmark JAR build failed."
+ fi
+ else
+ echo "BASE does not contain grails-benchmarks; comparison will use HEAD-only mode."
+ fi
+ if (
+ cd "$HEAD_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ head_build_ok=true
+ echo "Built HEAD benchmark at $HEAD_SHA."
+ else
+ echo "HEAD benchmark JAR build failed."
+ fi
+ {
+ printf 'BASE_BUILD_OK=%s\n' "$base_build_ok"
+ printf 'HEAD_BUILD_OK=%s\n' "$head_build_ok"
+ } >> "$GITHUB_ENV"
+ - name: "π Stop Gradle daemons before measurement"
+ if: always()
+ run: |
+ ./gradlew --stop || true
+ if [ -n "${BASE_DIR:-}" ] && [ -d "$BASE_DIR" ]; then (cd "$BASE_DIR" && ./gradlew --stop) || true; fi
+ if [ -n "${HEAD_DIR:-}" ] && [ -d "$HEAD_DIR" ]; then (cd "$HEAD_DIR" && ./gradlew --stop) || true; fi
+ # Two forks, three warmup iterations, and five measurement iterations balance PR latency
+ # against confidence. Reversing shard order cancels first-versus-second runner-state bias.
+ - name: "π‘οΈ Run paired JMH benchmarks"
Review Comment:
Minor follow-on to the daemon fix: the `--stop` above clears the daemons left by the build phase, but not the one the *first* measurement starts. Whichever revision runs second in a shard is measured on a daemon already warmed and loaded by the first (Gradle reuses a compatible idle daemon across project directories, so it's the same JVM rather than a second one).
The alternating shard order cancels this on average, which is why it's a nit rather than a finding. But `--no-daemon` on the two `run_benchmark` invocations would make both halves start from identical daemon state and remove a source of noise the rulers would otherwise have to absorb.
##########
grails-benchmarks/src/report/groovy/org/apache/grails/benchmarks/report/CommentPoster.groovy:
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.grails.benchmarks.report
+
+import groovy.json.JsonOutput
+import groovy.json.JsonSlurper
+import groovy.transform.CompileStatic
+
+import java.net.http.HttpClient
+import java.net.http.HttpRequest
+import java.net.http.HttpResponse
+import java.time.Duration
+
+@CompileStatic
+interface CommentPoster {
+
+ void post(String report, String repo, String prNumber, String token)
+}
+
+@CompileStatic
+class GitHubComments implements CommentPoster {
+
+ static final int MAX_BODY = 65000
+ static final String TRUNCATION = '\n\n_Report truncated. The full report is available in the workflow artifacts._\n\n<!-- grails-jmh-benchmark -->'
+ private final HttpClient client = HttpClient.newBuilder().connectTimeout(Duration.ofSeconds(15)).build()
+
+ @Override
+ void post(String report, String repo, String prNumber, String token) {
Review Comment:
`post` itself is untested. The specs cover `commentBody` and `latestMatchingCommentId`, and everything through `JmhCompare` uses `Mock(CommentPoster)`, so the paginated GET loop and the create-versus-update decision on line 63 never execute in a test.
That's the sticky-comment mechanism β the thing whose failure mode is a duplicate report on every push rather than an updated one, which is the specific behaviour the marker exists to prevent. A `com.sun.net.httpserver.HttpServer` stub would cover the page-boundary walk (the `comments.size() < 100` break) and assert PATCH-versus-POST against a canned comment list.
Worth flagging that coverage tooling won't catch this for you: the module deliberately omits `grails-jacoco`, so the "all modified and coverable lines are covered by tests" report on this PR says nothing about `grails-benchmarks`.
Unrelated, while you're in the specs: `JmhCompareSpec`'s first two features ("speedup respects throughput and latency direction", "verdict requires threshold and disjoint intervals") duplicate five features in `BenchmarkComparatorSpec` exactly.
##########
grails-benchmarks/build.gradle:
##########
@@ -0,0 +1,270 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.gradle.api.file.DuplicatesStrategy
+import org.gradle.api.tasks.PathSensitivity
+
+import java.util.Properties
+import java.util.zip.ZipFile
+
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'me.champeau.jmh' version '0.7.3'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.compile'
+ id 'org.apache.grails.buildsrc.vulnerability-scan'
+ id 'org.apache.grails.gradle.grails-code-style'
+}
+
+version = projectVersion
+group = 'org.apache.grails'
+
+// This build-time-only benchmark harness is never published, so it deliberately omits
+// org.apache.grails.buildsrc.publish, org.apache.grails.buildsrc.sbom,
+// org.apache.grails.gradle.grails-jacoco, and org.apache.grails.buildsrc.dependency-validator.
+// Its JMH dependency is GPLv2 with the Classpath Exception (Category X) and must not be published.
+
+sourceSets {
+ report {
+ groovy.srcDirs = ['src/report/groovy']
+ }
+}
+
+// The Groovy fixtures under src/main/groovy build the framework objects whose APIs are
+// closure-based (the URL mappings DSL, the Validateable trait, view templates). The JMH
Review Comment:
`the Validateable trait` is stale β there is no Validateable fixture in the module. `README.adoc` line 93 carries the same claim in its Layout section, and is then contradicted by its own "Known gap: validation" section 28 lines later, which explains that the validation benchmark was written and withdrawn.
Same category as the Python leftovers from the last round: a reader looking for the Groovy fixture that needs the trait won't find one. The remaining two examples (the URL mappings DSL and view templates) carry the argument on their own.
##########
grails-benchmarks/src/report/groovy/org/apache/grails/benchmarks/report/JmhResults.groovy:
##########
@@ -0,0 +1,139 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.grails.benchmarks.report
+
+import groovy.json.JsonSlurper
+import groovy.transform.CompileStatic
+
+import java.nio.file.Files
+import java.nio.file.Path
+
+@CompileStatic
+class JmhResults {
+
+ static final String ALLOCATION_METRIC = 'gc.alloc.rate.norm'
+
+ static Double finiteNumber(Object value) {
+ if (value == null) {
+ return null
+ }
+ try {
+ Double number = Double.valueOf(value.toString())
+ return Double.isFinite(number) ? number : null
+ } catch (NumberFormatException ignored) {
+ return null
+ }
+ }
+
+ static List<Double> confidenceInterval(Map<String, Object> metric) {
+ Object value = metric.get('scoreConfidence')
+ if (value instanceof List && ((List<?>) value).size() == 2) {
+ Double lower = finiteNumber(((List<?>) value).get(0))
+ Double upper = finiteNumber(((List<?>) value).get(1))
+ if (lower != null && upper != null) {
+ return [Math.min(lower, upper), Math.max(lower, upper)]
+ }
+ }
+ Double score = finiteNumber(metric.get('score'))
+ Double error = finiteNumber(metric.get('scoreError'))
+ return score != null && error != null && error >= 0D ? [score - error, score + error] : null
+ }
+
+ static String identity(String name, Map<String, Object> params) {
Review Comment:
`identity` keys on benchmark name plus params but not on `mode`, and `parseEntries` does a plain `benchmarks.put(benchmarkId, ...)`. A benchmark declaring more than one mode β `@BenchmarkMode({Mode.AverageTime, Mode.Throughput})` β emits one JSON entry per mode under the same name, so all but the last are silently dropped.
Nothing in the suite does this today, and `comparable()` plus `poolBenchmarks` correctly refuse to compare across modes, so it can't produce a *wrong* verdict. But it can silently shrink the comparison set, surfacing only as an anonymous "Malformed comparisons skipped: N" count β the same silent-shrink failure that `jmh.failOnError = true` was set to prevent on the measurement side.
Adding mode to the identity key would make it a visible `Only in head` / `Only in base` entry instead.
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,396 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
+ paths-ignore:
+ - '**/*.md'
+ - '**/*.adoc'
+ - 'grails-doc/**'
+ workflow_dispatch:
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+permissions:
+ contents: read
+
+jobs:
+ # Each shard builds both revisions before measuring either one on the same runner.
+ # Building first avoids CPU, IO, thermal, cache, and frequency state biasing a measurement.
+ # Shard a measures BASE then HEAD, while shard b measures HEAD then BASE.
+ # Alternating the order cancels first-versus-second ordering bias without doubling runtime.
+ # Both shards use the PR merge commit as HEAD, so they measure what would actually land.
+ # Results are advisory: a detected regression never fails this workflow.
+ benchmark:
+ name: "Paired JMH benchmarks (${{ matrix.shard }})"
+ if: >-
+ github.event_name == 'workflow_dispatch' ||
+ (contains(github.event.pull_request.labels.*.name, 'performance') &&
+ (github.event.action != 'labeled' || github.event.label.name == 'performance'))
+ runs-on: ubuntu-24.04
+ strategy:
+ fail-fast: false
+ matrix:
+ shard: [a, b]
+ env:
+ BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ HEAD_SHA: ${{ github.sha }}
+ JMH_INCLUDE: '.*'
+ RESULT_DIR: ${{ github.workspace }}/jmh-results/${{ matrix.shard }}
+ REPORT_DIR: ${{ github.workspace }}/jmh-reports/${{ matrix.shard }}
+ SHARD: ${{ matrix.shard }}
+ steps:
+ - name: "π₯ Checkout repository"
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+ with:
+ fetch-depth: 0
+ - name: "βοΈ Setup JDK"
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
+ with:
+ distribution: liberica
+ java-version: 21
+ - name: "π Setup Gradle"
+ uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0
+ with:
+ cache-provider: basic # 'basic' uses the MIT-licensed, open-source cache provider; the default 'enhanced' provider (v6+) is proprietary (Gradle commercial Terms of Use)
+ develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+ # Deliberately allowed to fail the job, and placed before the expensive work: a broken
+ # comparison tool 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 tool"
+ run: ./gradlew :grails-benchmarks:test --max-workers=4
+ - name: "π³ Prepare paired worktrees"
+ run: |
+ WORKTREE_ROOT="$RUNNER_TEMP/jmh-worktrees/$SHARD"
+ mkdir -p "$WORKTREE_ROOT" "$RESULT_DIR" "$REPORT_DIR"
+ if [ -n "$BASE_SHA" ] && git cat-file -e "$BASE_SHA^{commit}" 2>/dev/null; then
+ RESOLVED_BASE_SHA="$BASE_SHA"
+ elif RESOLVED_BASE_SHA="$(git merge-base "$HEAD_SHA^1" "$HEAD_SHA^2" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using merge-base $RESOLVED_BASE_SHA."
+ elif RESOLVED_BASE_SHA="$(git rev-parse "$HEAD_SHA^" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using HEAD parent $RESOLVED_BASE_SHA."
+ else
+ RESOLVED_BASE_SHA=""
+ echo "No base commit could be resolved; comparison will use HEAD-only mode."
+ fi
+ git worktree add --detach "$WORKTREE_ROOT/head" "$HEAD_SHA"
+ {
+ printf 'HEAD_DIR=%s\n' "$WORKTREE_ROOT/head"
+ printf 'WORKTREE_ROOT=%s\n' "$WORKTREE_ROOT"
+ } >> "$GITHUB_ENV"
+ if [ -n "$RESOLVED_BASE_SHA" ] && git worktree add --detach "$WORKTREE_ROOT/base" "$RESOLVED_BASE_SHA"; then
+ {
+ printf 'BASE_DIR=%s\n' "$WORKTREE_ROOT/base"
+ printf 'RESOLVED_BASE_SHA=%s\n' "$RESOLVED_BASE_SHA"
+ } >> "$GITHUB_ENV"
+ echo "Using base commit $RESOLVED_BASE_SHA."
+ else
+ echo "BASE_BENCHMARKS_AVAILABLE=false" >> "$GITHUB_ENV"
+ fi
+ # Build both JMH jars before measuring either revision. Builds are CPU- and IO-heavy,
+ # so building and measuring one revision at a time would bias results with different
+ # thermal, cache, and CPU-frequency state on the shared runner.
+ - name: "π¨ Build paired JMH jars"
+ timeout-minutes: 60
+ run: |
+ base_build_ok=false
+ head_build_ok=false
+ if [ "${BASE_BENCHMARKS_AVAILABLE:-true}" = "true" ] && [ -f "$BASE_DIR/grails-benchmarks/build.gradle" ]; then
+ if (
+ cd "$BASE_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ base_build_ok=true
+ echo "Built base benchmark at $RESOLVED_BASE_SHA."
+ else
+ echo "BASE benchmark JAR build failed."
+ fi
+ else
+ echo "BASE does not contain grails-benchmarks; comparison will use HEAD-only mode."
+ fi
+ if (
+ cd "$HEAD_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ head_build_ok=true
+ echo "Built HEAD benchmark at $HEAD_SHA."
+ else
+ echo "HEAD benchmark JAR build failed."
+ fi
+ {
+ printf 'BASE_BUILD_OK=%s\n' "$base_build_ok"
+ printf 'HEAD_BUILD_OK=%s\n' "$head_build_ok"
+ } >> "$GITHUB_ENV"
+ - name: "π Stop Gradle daemons before measurement"
+ if: always()
+ run: |
+ ./gradlew --stop || true
+ if [ -n "${BASE_DIR:-}" ] && [ -d "$BASE_DIR" ]; then (cd "$BASE_DIR" && ./gradlew --stop) || true; fi
+ if [ -n "${HEAD_DIR:-}" ] && [ -d "$HEAD_DIR" ]; then (cd "$HEAD_DIR" && ./gradlew --stop) || true; fi
+ # Two forks, three warmup iterations, and five measurement iterations balance PR latency
+ # against confidence. Reversing shard order cancels first-versus-second runner-state bias.
+ - name: "π‘οΈ Run paired JMH benchmarks"
+ timeout-minutes: 60
+ run: |
+ # JMH writes results incrementally, so a run that dies partway can leave a file that
+ # parses perfectly while describing only some of the benchmarks. The report job decides
+ # completeness from which files exist, so a partial file would be indistinguishable from
+ # a good one. Write to a staging path and publish it only on success, so a failed run
+ # leaves NO file rather than a plausible one.
+ run_benchmark() {
+ local revision_dir="$1"
+ local result_file="$2"
+ local staging_file="$result_file.partial"
+ rm -f "$staging_file" "$result_file"
+ if (
+ cd "$revision_dir" && ./gradlew :grails-benchmarks:jmh \
+ -Pjmh.include="$JMH_INCLUDE" \
+ -Pjmh.forks=2 \
+ -Pjmh.warmupIterations=3 \
+ -Pjmh.iterations=5 \
+ -Pjmh.resultFile="$staging_file" \
+ -Pjmh.profilers=gc \
+ --max-workers=4
+ ) && [ -s "$staging_file" ] && mv "$staging_file" "$result_file"; then
+ return 0
+ fi
+ rm -f "$staging_file"
+ return 1
+ }
+
+ base_run_failed=false
+ head_run_failed=false
+ run_base_benchmark() {
+ if [ "${BASE_BUILD_OK:-false}" != "true" ]; then
+ echo "BASE benchmark execution skipped because its JAR was not built."
+ elif ! run_benchmark "$BASE_DIR" "$RESULT_DIR/base.json"; then
+ base_run_failed=true
+ echo "BASE benchmark execution failed."
+ fi
+ }
+ run_head_benchmark() {
+ if [ "${HEAD_BUILD_OK:-false}" != "true" ]; then
+ echo "HEAD benchmark execution skipped because its JAR was not built."
+ elif ! run_benchmark "$HEAD_DIR" "$RESULT_DIR/head.json"; then
+ head_run_failed=true
+ echo "HEAD benchmark execution failed."
+ fi
+ }
+
+ if [ "$SHARD" = "a" ]; then
+ run_base_benchmark
+ run_head_benchmark
+ else
+ run_head_benchmark
+ run_base_benchmark
+ fi
+ {
+ printf 'BASE_RUN_FAILED=%s\n' "$base_run_failed"
+ printf 'HEAD_RUN_FAILED=%s\n' "$head_run_failed"
+ } >> "$GITHUB_ENV"
+ # The comparison is advisory. The comparison tool exits successfully for regressions, and this
+ # step is non-blocking even if results are incomplete because a benchmark execution failed.
+ # No --pr-number is passed here on purpose: this job renders the report only. The separate
+ # report job owns comment posting, so a two-shard matrix cannot produce duplicate comments.
+ - name: "π Compare JMH results"
+ if: always()
+ continue-on-error: true
+ run: |
+ report_file="$REPORT_DIR/comparison.md"
+ {
+ printf '### JMH shard `%s`\n\n' "$SHARD"
+ if [ "${BASE_RUN_FAILED:-false}" = "true" ]; then
+ printf 'BASE benchmark execution failed.\n\n'
+ fi
+ if [ "${BASE_BUILD_OK:-false}" != "true" ]; then
+ printf 'BASE benchmark JAR was not built.\n\n'
+ fi
+ if [ "${HEAD_RUN_FAILED:-false}" = "true" ]; then
+ printf 'HEAD benchmark execution failed.\n\n'
+ fi
+ if [ "${HEAD_BUILD_OK:-false}" != "true" ]; then
+ printf 'HEAD benchmark JAR was not built.\n\n'
+ fi
+ } > "$report_file"
+ comparison_file="$RUNNER_TEMP/jmh-comparison-$SHARD.md"
+ if [ ! -f "$RESULT_DIR/head.json" ]; then
+ printf 'HEAD benchmark result was not produced.\n' >> "$report_file"
+ elif [ "${BASE_BUILD_OK:-false}" = "true" ] && [ -f "$RESULT_DIR/base.json" ]; then
+ ./gradlew -q --console=plain :grails-benchmarks:jmhCompare --args="--head $RESULT_DIR/head.json --base $RESULT_DIR/base.json --output $comparison_file"
+ cat "$comparison_file" >> "$report_file"
+ else
+ ./gradlew -q --console=plain :grails-benchmarks:jmhCompare --args="--head $RESULT_DIR/head.json --output $comparison_file"
+ cat "$comparison_file" >> "$report_file"
+ fi
+ - name: "π Publish JMH report in job summary"
+ if: always()
+ run: |
+ if [ -f "$REPORT_DIR/comparison.md" ]; then
+ cat "$REPORT_DIR/comparison.md" >> "$GITHUB_STEP_SUMMARY"
+ else
+ printf '## JMH benchmark comparison\n\nNo comparison report was produced.\n' >> "$GITHUB_STEP_SUMMARY"
+ fi
+ - name: "π€ Upload JMH artifacts"
+ if: always()
+ uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+ with:
+ name: jmh-results-${{ matrix.shard }}
+ path: |
+ jmh-results/${{ matrix.shard }}/
+ jmh-reports/${{ matrix.shard }}/
+ if-no-files-found: warn
+ - name: "π§Ή Remove paired worktrees"
+ if: always()
+ run: |
+ WORKTREE_ROOT="$RUNNER_TEMP/jmh-worktrees/$SHARD"
+ git worktree remove --force "$WORKTREE_ROOT/base" || true
+ git worktree remove --force "$WORKTREE_ROOT/head" || true
+
+ report:
+ name: "Publish JMH benchmark comparison"
+ needs: benchmark
+ if: >-
+ always() && github.event_name == 'pull_request' &&
Review Comment:
`github.event_name == 'pull_request'` here means a `workflow_dispatch` run can never reach the reporting job, so it produces artifacts and per-shard job summaries but no sticky comment. The PR description says a comment appears for `workflow_dispatch` too.
There's a second consequence: on dispatch there's no `pull_request` context, so `BASE_SHA` is empty and base resolution in "Prepare paired worktrees" falls through the merge-base branch to `git rev-parse "$HEAD_SHA^"`. A manual run therefore benchmarks HEAD against its own parent commit, not against a release branch.
Both behaviours are defensible, but neither is what the description promises. Either document what dispatch actually does, or drop the trigger.
##########
grails-benchmarks/src/report/groovy/org/apache/grails/benchmarks/report/CommentPoster.groovy:
##########
@@ -0,0 +1,111 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.grails.benchmarks.report
+
+import groovy.json.JsonOutput
+import groovy.json.JsonSlurper
+import groovy.transform.CompileStatic
+
+import java.net.http.HttpClient
+import java.net.http.HttpRequest
+import java.net.http.HttpResponse
+import java.time.Duration
+
+@CompileStatic
+interface CommentPoster {
+
+ void post(String report, String repo, String prNumber, String token)
+}
+
+@CompileStatic
+class GitHubComments implements CommentPoster {
+
+ static final int MAX_BODY = 65000
+ static final String TRUNCATION = '\n\n_Report truncated. The full report is available in the workflow artifacts._\n\n<!-- grails-jmh-benchmark -->'
+ private final HttpClient client = HttpClient.newBuilder().connectTimeout(Duration.ofSeconds(15)).build()
+
+ @Override
+ void post(String report, String repo, String prNumber, String token) {
+ String body = commentBody(report)
+ Long commentId = null
+ for (int page = 1; page <= 100; page++) {
+ Object response = request(
+ "https://api.github.com/repos/${repo}/issues/${prNumber}/comments?per_page=100&page=${page}",
+ token,
+ 'GET',
+ null)
+ if (!(response instanceof List)) {
+ break
+ }
+ List<?> comments = (List<?>) response
+ Long pageMatch = latestMatchingCommentId(comments)
+ if (pageMatch != null) {
+ commentId = pageMatch
+ }
+ if (comments.size() < 100) {
+ break
+ }
+ }
+ String endpoint = commentId == null
+ ? "https://api.github.com/repos/${repo}/issues/${prNumber}/comments"
+ : "https://api.github.com/repos/${repo}/issues/comments/${commentId}"
+ request(endpoint, token, commentId == null ? 'POST' : 'PATCH', JsonOutput.toJson([body: body]))
+ }
+
+ /**
+ * Keep the last matching comment on a page so duplicate marker comments update the
+ * newest bot report (GitHub returns comments oldest-first).
+ */
+ static Long latestMatchingCommentId(List<?> comments) {
+ Long commentId = null
+ for (Object comment : comments) {
+ if (!(comment instanceof Map) || !markerComment((Map<String, Object>) comment)) {
+ continue
+ }
+ Object id = ((Map<String, Object>) comment).get('id')
+ if (id instanceof Number) {
+ commentId = ((Number) id).longValue()
+ }
+ }
+ return commentId
+ }
+
+ static String commentBody(String report) {
+ String body = report.contains(ReportRenderer.MARKER)
+ ? report
+ : report + '\n\n' + ReportRenderer.MARKER
+ if (body.length() > MAX_BODY) {
+ body = body.substring(0, MAX_BODY - TRUNCATION.length()) + TRUNCATION
+ }
+ return body
+ }
+
+ private static boolean markerComment(Map<String, Object> comment) {
+ Object body = comment.get('body')
+ Object user = comment.get('user')
+ Object login = user instanceof Map ? ((Map<String, Object>) user).get('login') : null
+ return body instanceof String && ((String) body).contains(ReportRenderer.MARKER) && (!(login instanceof String) || login == 'github-actions[bot]')
Review Comment:
The author check defaults the wrong way: `!(login instanceof String) || login == 'github-actions[bot]'` treats a comment whose `user` is absent or non-object as ours. If GitHub returns a comment without a usable `user` (deleted accounts are the usual way this happens) and that body contains the marker, it becomes a PATCH target.
The reporting job holds `pull-requests: write`, which is enough to edit any comment in the repository, so the failure mode is overwriting someone else's comment rather than a harmless 403.
Requiring the bot instead of assuming it β `login == 'github-actions[bot]'` β keeps the intended behaviour and makes the unknown-author case fall through to posting a fresh comment, which is the safe direction.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on code in PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#discussion_r3706555344
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,381 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
Review Comment:
Fixed in 202d206a6a.
Both the `benchmark` and `report` job gates now require:
```yaml
github.event.action != 'labeled' || github.event.label.name == 'performance'
```
So once a PR already carries `performance`, adding `bug`/`deps`/etc. no longer kicks off another two-shard run.
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,381 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
+ paths-ignore:
+ - '**/*.md'
+ - '**/*.adoc'
+ - 'grails-doc/**'
+ workflow_dispatch:
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+permissions:
+ contents: read
+
+jobs:
+ # Each shard builds both revisions before measuring either one on the same runner.
+ # Building first avoids CPU, IO, thermal, cache, and frequency state biasing a measurement.
+ # Shard a measures BASE then HEAD, while shard b measures HEAD then BASE.
+ # Alternating the order cancels first-versus-second ordering bias without doubling runtime.
+ # Both shards use the PR merge commit as HEAD, so they measure what would actually land.
+ # Results are advisory: a detected regression never fails this workflow.
+ benchmark:
+ name: "Paired JMH benchmarks (${{ matrix.shard }})"
+ if: ${{ github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'performance') }}
+ runs-on: ubuntu-24.04
+ strategy:
+ fail-fast: false
+ matrix:
+ shard: [a, b]
+ env:
+ BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ HEAD_SHA: ${{ github.sha }}
+ JMH_INCLUDE: '.*'
+ PR_NUMBER: ${{ github.event.pull_request.number || 0 }}
Review Comment:
Fixed in 202d206a6a.
Dropped `PR_NUMBER` and `REPOSITORY` from the shard job env. They remain only on the `report` job, which is the sole comment-posting path.
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,381 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
+ paths-ignore:
+ - '**/*.md'
+ - '**/*.adoc'
+ - 'grails-doc/**'
+ workflow_dispatch:
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+permissions:
+ contents: read
+
+jobs:
+ # Each shard builds both revisions before measuring either one on the same runner.
+ # Building first avoids CPU, IO, thermal, cache, and frequency state biasing a measurement.
+ # Shard a measures BASE then HEAD, while shard b measures HEAD then BASE.
+ # Alternating the order cancels first-versus-second ordering bias without doubling runtime.
+ # Both shards use the PR merge commit as HEAD, so they measure what would actually land.
+ # Results are advisory: a detected regression never fails this workflow.
+ benchmark:
+ name: "Paired JMH benchmarks (${{ matrix.shard }})"
+ if: ${{ github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'performance') }}
+ runs-on: ubuntu-24.04
+ strategy:
+ fail-fast: false
+ matrix:
+ shard: [a, b]
+ env:
+ BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ HEAD_SHA: ${{ github.sha }}
+ JMH_INCLUDE: '.*'
+ PR_NUMBER: ${{ github.event.pull_request.number || 0 }}
+ REPOSITORY: ${{ github.repository }}
+ RESULT_DIR: ${{ github.workspace }}/jmh-results/${{ matrix.shard }}
+ REPORT_DIR: ${{ github.workspace }}/jmh-reports/${{ matrix.shard }}
+ SHARD: ${{ matrix.shard }}
+ steps:
+ - name: "π₯ Checkout repository"
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+ with:
+ fetch-depth: 0
+ - name: "βοΈ Setup JDK"
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
+ with:
+ distribution: liberica
+ java-version: 21
+ - name: "π Setup Gradle"
+ uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0
+ with:
+ cache-provider: basic # 'basic' uses the MIT-licensed, open-source cache provider; the default 'enhanced' provider (v6+) is proprietary (Gradle commercial Terms of Use)
+ develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+ # Deliberately allowed to fail the job, and placed before the expensive work: a broken
+ # comparison tool 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 tool"
+ run: ./gradlew :grails-benchmarks:test --max-workers=4
+ - name: "π³ Prepare paired worktrees"
+ run: |
+ WORKTREE_ROOT="$RUNNER_TEMP/jmh-worktrees/$SHARD"
+ mkdir -p "$WORKTREE_ROOT" "$RESULT_DIR" "$REPORT_DIR"
+ if [ -n "$BASE_SHA" ] && git cat-file -e "$BASE_SHA^{commit}" 2>/dev/null; then
+ RESOLVED_BASE_SHA="$BASE_SHA"
+ elif RESOLVED_BASE_SHA="$(git merge-base "$HEAD_SHA^1" "$HEAD_SHA^2" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using merge-base $RESOLVED_BASE_SHA."
+ elif RESOLVED_BASE_SHA="$(git rev-parse "$HEAD_SHA^" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using HEAD parent $RESOLVED_BASE_SHA."
+ else
+ RESOLVED_BASE_SHA=""
+ echo "No base commit could be resolved; comparison will use HEAD-only mode."
+ fi
+ git worktree add --detach "$WORKTREE_ROOT/head" "$HEAD_SHA"
+ {
+ printf 'HEAD_DIR=%s\n' "$WORKTREE_ROOT/head"
+ printf 'WORKTREE_ROOT=%s\n' "$WORKTREE_ROOT"
+ } >> "$GITHUB_ENV"
+ if [ -n "$RESOLVED_BASE_SHA" ] && git worktree add --detach "$WORKTREE_ROOT/base" "$RESOLVED_BASE_SHA"; then
+ {
+ printf 'BASE_DIR=%s\n' "$WORKTREE_ROOT/base"
+ printf 'RESOLVED_BASE_SHA=%s\n' "$RESOLVED_BASE_SHA"
+ } >> "$GITHUB_ENV"
+ echo "Using base commit $RESOLVED_BASE_SHA."
+ else
+ echo "BASE_BENCHMARKS_AVAILABLE=false" >> "$GITHUB_ENV"
+ fi
+ # Build both JMH jars before measuring either revision. Builds are CPU- and IO-heavy,
+ # so building and measuring one revision at a time would bias results with different
+ # thermal, cache, and CPU-frequency state on the shared runner.
+ - name: "π¨ Build paired JMH jars"
+ timeout-minutes: 60
+ run: |
+ base_build_ok=false
+ head_build_ok=false
+ if [ "${BASE_BENCHMARKS_AVAILABLE:-true}" = "true" ] && [ -f "$BASE_DIR/grails-benchmarks/build.gradle" ]; then
+ if (
+ cd "$BASE_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ base_build_ok=true
+ echo "Built base benchmark at $RESOLVED_BASE_SHA."
+ else
+ echo "BASE benchmark JAR build failed."
+ fi
+ else
+ echo "BASE does not contain grails-benchmarks; comparison will use HEAD-only mode."
+ fi
+ if (
+ cd "$HEAD_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ head_build_ok=true
+ echo "Built HEAD benchmark at $HEAD_SHA."
+ else
+ echo "HEAD benchmark JAR build failed."
+ fi
+ {
+ printf 'BASE_BUILD_OK=%s\n' "$base_build_ok"
+ printf 'HEAD_BUILD_OK=%s\n' "$head_build_ok"
+ } >> "$GITHUB_ENV"
+ # Two forks, three warmup iterations, and five measurement iterations balance PR latency
+ # against confidence. Reversing shard order cancels first-versus-second runner-state bias.
+ - name: "π‘οΈ Run paired JMH benchmarks"
+ timeout-minutes: 60
+ run: |
+ # JMH writes results incrementally, so a run that dies partway can leave a file that
+ # parses perfectly while describing only some of the benchmarks. The report job decides
+ # completeness from which files exist, so a partial file would be indistinguishable from
+ # a good one. Write to a staging path and publish it only on success, so a failed run
+ # leaves NO file rather than a plausible one.
+ run_benchmark() {
+ local revision_dir="$1"
+ local result_file="$2"
+ local staging_file="$result_file.partial"
+ rm -f "$staging_file" "$result_file"
+ if (
+ cd "$revision_dir" && ./gradlew :grails-benchmarks:jmh \
Review Comment:
Fixed in 202d206a6a.
Added a `Stop Gradle daemons before measurement` step between the paired `jmhJar` builds and the JMH runs. It runs `./gradlew --stop` in the main workspace and both worktrees so idle daemons are not competing with the JMH forks during the measurement window.
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,381 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
+ paths-ignore:
+ - '**/*.md'
+ - '**/*.adoc'
+ - 'grails-doc/**'
+ workflow_dispatch:
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+permissions:
+ contents: read
+
+jobs:
+ # Each shard builds both revisions before measuring either one on the same runner.
+ # Building first avoids CPU, IO, thermal, cache, and frequency state biasing a measurement.
+ # Shard a measures BASE then HEAD, while shard b measures HEAD then BASE.
+ # Alternating the order cancels first-versus-second ordering bias without doubling runtime.
+ # Both shards use the PR merge commit as HEAD, so they measure what would actually land.
+ # Results are advisory: a detected regression never fails this workflow.
+ benchmark:
+ name: "Paired JMH benchmarks (${{ matrix.shard }})"
+ if: ${{ github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'performance') }}
+ runs-on: ubuntu-24.04
+ strategy:
+ fail-fast: false
+ matrix:
+ shard: [a, b]
+ env:
+ BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ HEAD_SHA: ${{ github.sha }}
+ JMH_INCLUDE: '.*'
+ PR_NUMBER: ${{ github.event.pull_request.number || 0 }}
+ REPOSITORY: ${{ github.repository }}
+ RESULT_DIR: ${{ github.workspace }}/jmh-results/${{ matrix.shard }}
+ REPORT_DIR: ${{ github.workspace }}/jmh-reports/${{ matrix.shard }}
+ SHARD: ${{ matrix.shard }}
+ steps:
+ - name: "π₯ Checkout repository"
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+ with:
+ fetch-depth: 0
+ - name: "βοΈ Setup JDK"
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
+ with:
+ distribution: liberica
+ java-version: 21
+ - name: "π Setup Gradle"
+ uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0
+ with:
+ cache-provider: basic # 'basic' uses the MIT-licensed, open-source cache provider; the default 'enhanced' provider (v6+) is proprietary (Gradle commercial Terms of Use)
+ develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+ # Deliberately allowed to fail the job, and placed before the expensive work: a broken
+ # comparison tool 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 tool"
+ run: ./gradlew :grails-benchmarks:test --max-workers=4
+ - name: "π³ Prepare paired worktrees"
+ run: |
+ WORKTREE_ROOT="$RUNNER_TEMP/jmh-worktrees/$SHARD"
+ mkdir -p "$WORKTREE_ROOT" "$RESULT_DIR" "$REPORT_DIR"
+ if [ -n "$BASE_SHA" ] && git cat-file -e "$BASE_SHA^{commit}" 2>/dev/null; then
+ RESOLVED_BASE_SHA="$BASE_SHA"
+ elif RESOLVED_BASE_SHA="$(git merge-base "$HEAD_SHA^1" "$HEAD_SHA^2" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using merge-base $RESOLVED_BASE_SHA."
+ elif RESOLVED_BASE_SHA="$(git rev-parse "$HEAD_SHA^" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using HEAD parent $RESOLVED_BASE_SHA."
+ else
+ RESOLVED_BASE_SHA=""
+ echo "No base commit could be resolved; comparison will use HEAD-only mode."
+ fi
+ git worktree add --detach "$WORKTREE_ROOT/head" "$HEAD_SHA"
+ {
+ printf 'HEAD_DIR=%s\n' "$WORKTREE_ROOT/head"
+ printf 'WORKTREE_ROOT=%s\n' "$WORKTREE_ROOT"
+ } >> "$GITHUB_ENV"
+ if [ -n "$RESOLVED_BASE_SHA" ] && git worktree add --detach "$WORKTREE_ROOT/base" "$RESOLVED_BASE_SHA"; then
+ {
+ printf 'BASE_DIR=%s\n' "$WORKTREE_ROOT/base"
+ printf 'RESOLVED_BASE_SHA=%s\n' "$RESOLVED_BASE_SHA"
+ } >> "$GITHUB_ENV"
+ echo "Using base commit $RESOLVED_BASE_SHA."
+ else
+ echo "BASE_BENCHMARKS_AVAILABLE=false" >> "$GITHUB_ENV"
+ fi
+ # Build both JMH jars before measuring either revision. Builds are CPU- and IO-heavy,
+ # so building and measuring one revision at a time would bias results with different
+ # thermal, cache, and CPU-frequency state on the shared runner.
+ - name: "π¨ Build paired JMH jars"
+ timeout-minutes: 60
+ run: |
+ base_build_ok=false
+ head_build_ok=false
+ if [ "${BASE_BENCHMARKS_AVAILABLE:-true}" = "true" ] && [ -f "$BASE_DIR/grails-benchmarks/build.gradle" ]; then
+ if (
+ cd "$BASE_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ base_build_ok=true
+ echo "Built base benchmark at $RESOLVED_BASE_SHA."
+ else
+ echo "BASE benchmark JAR build failed."
+ fi
+ else
+ echo "BASE does not contain grails-benchmarks; comparison will use HEAD-only mode."
+ fi
+ if (
+ cd "$HEAD_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ head_build_ok=true
+ echo "Built HEAD benchmark at $HEAD_SHA."
+ else
+ echo "HEAD benchmark JAR build failed."
+ fi
+ {
+ printf 'BASE_BUILD_OK=%s\n' "$base_build_ok"
+ printf 'HEAD_BUILD_OK=%s\n' "$head_build_ok"
+ } >> "$GITHUB_ENV"
+ # Two forks, three warmup iterations, and five measurement iterations balance PR latency
+ # against confidence. Reversing shard order cancels first-versus-second runner-state bias.
+ - name: "π‘οΈ Run paired JMH benchmarks"
+ timeout-minutes: 60
+ run: |
+ # JMH writes results incrementally, so a run that dies partway can leave a file that
+ # parses perfectly while describing only some of the benchmarks. The report job decides
+ # completeness from which files exist, so a partial file would be indistinguishable from
+ # a good one. Write to a staging path and publish it only on success, so a failed run
+ # leaves NO file rather than a plausible one.
+ run_benchmark() {
+ local revision_dir="$1"
+ local result_file="$2"
+ local staging_file="$result_file.partial"
+ rm -f "$staging_file" "$result_file"
+ if (
+ cd "$revision_dir" && ./gradlew :grails-benchmarks:jmh \
+ -Pjmh.include="$JMH_INCLUDE" \
+ -Pjmh.forks=2 \
+ -Pjmh.warmupIterations=3 \
+ -Pjmh.iterations=5 \
+ -Pjmh.resultFile="$staging_file" \
+ -Pjmh.profilers=gc \
+ --max-workers=4
+ ) && [ -s "$staging_file" ] && mv "$staging_file" "$result_file"; then
+ return 0
+ fi
+ rm -f "$staging_file"
+ return 1
+ }
+
+ base_run_failed=false
+ head_run_failed=false
+ run_base_benchmark() {
+ if [ "${BASE_BUILD_OK:-false}" != "true" ]; then
+ echo "BASE benchmark execution skipped because its JAR was not built."
+ elif ! run_benchmark "$BASE_DIR" "$RESULT_DIR/base.json"; then
+ base_run_failed=true
+ echo "BASE benchmark execution failed."
+ fi
+ }
+ run_head_benchmark() {
+ if [ "${HEAD_BUILD_OK:-false}" != "true" ]; then
+ echo "HEAD benchmark execution skipped because its JAR was not built."
+ elif ! run_benchmark "$HEAD_DIR" "$RESULT_DIR/head.json"; then
+ head_run_failed=true
+ echo "HEAD benchmark execution failed."
+ fi
+ }
+
+ if [ "$SHARD" = "a" ]; then
+ run_base_benchmark
+ run_head_benchmark
+ else
+ run_head_benchmark
+ run_base_benchmark
+ fi
+ {
+ printf 'BASE_RUN_FAILED=%s\n' "$base_run_failed"
+ printf 'HEAD_RUN_FAILED=%s\n' "$head_run_failed"
+ } >> "$GITHUB_ENV"
+ # The comparison is advisory. The comparison tool exits successfully for regressions, and this
+ # step is non-blocking even if results are incomplete because a benchmark execution failed.
+ # No --pr-number is passed here on purpose: this job renders the report only. The separate
+ # report job owns comment posting, so a two-shard matrix cannot produce duplicate comments.
+ - name: "π Compare JMH results"
+ if: always()
+ continue-on-error: true
+ run: |
+ report_file="$REPORT_DIR/comparison.md"
+ {
+ printf '### JMH shard `%s`\n\n' "$SHARD"
+ if [ "${BASE_RUN_FAILED:-false}" = "true" ]; then
+ printf 'BASE benchmark execution failed.\n\n'
+ fi
+ if [ "${BASE_BUILD_OK:-false}" != "true" ]; then
+ printf 'BASE benchmark JAR was not built.\n\n'
+ fi
+ if [ "${HEAD_RUN_FAILED:-false}" = "true" ]; then
+ printf 'HEAD benchmark execution failed.\n\n'
+ fi
+ if [ "${HEAD_BUILD_OK:-false}" != "true" ]; then
+ printf 'HEAD benchmark JAR was not built.\n\n'
+ fi
+ } > "$report_file"
+ comparison_file="$RUNNER_TEMP/jmh-comparison-$SHARD.md"
+ if [ ! -f "$RESULT_DIR/head.json" ]; then
+ printf 'HEAD benchmark result was not produced.\n' >> "$report_file"
+ elif [ "${BASE_BUILD_OK:-false}" = "true" ] && [ -f "$RESULT_DIR/base.json" ]; then
+ ./gradlew -q --console=plain :grails-benchmarks:jmhCompare --args="--head $RESULT_DIR/head.json --base $RESULT_DIR/base.json --output $comparison_file"
+ cat "$comparison_file" >> "$report_file"
+ else
+ ./gradlew -q --console=plain :grails-benchmarks:jmhCompare --args="--head $RESULT_DIR/head.json --output $comparison_file"
+ cat "$comparison_file" >> "$report_file"
+ fi
+ - name: "π Publish JMH report in job summary"
+ if: always()
+ run: |
+ if [ -f "$REPORT_DIR/comparison.md" ]; then
+ cat "$REPORT_DIR/comparison.md" >> "$GITHUB_STEP_SUMMARY"
+ else
+ printf '## JMH benchmark comparison\n\nNo comparison report was produced.\n' >> "$GITHUB_STEP_SUMMARY"
+ fi
+ - name: "π€ Upload JMH artifacts"
+ if: always()
+ uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+ with:
+ name: jmh-results-${{ matrix.shard }}
+ path: |
+ jmh-results/${{ matrix.shard }}/
+ jmh-reports/${{ matrix.shard }}/
+ if-no-files-found: warn
+ - name: "π§Ή Remove paired worktrees"
+ if: always()
+ run: |
+ WORKTREE_ROOT="$RUNNER_TEMP/jmh-worktrees/$SHARD"
+ git worktree remove --force "$WORKTREE_ROOT/base" || true
+ git worktree remove --force "$WORKTREE_ROOT/head" || true
+
+ report:
+ name: "Publish JMH benchmark comparison"
+ needs: benchmark
+ if: ${{ always() && github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'performance') && github.event.pull_request.head.repo.full_name == github.repository }}
Review Comment:
Fixed in 202d206a6a.
The pool/compare step now sets `should_post=true` only when complete pairs or HEAD-only results exist. The comment step is gated on that output. When nothing was produced, the job summary says the run failed and the previous sticky report was left unchanged - no empty overwrite, and the sticky marker is not written in that path.
##########
grails-benchmarks/src/report/groovy/org/apache/grails/benchmarks/report/JmhCompare.groovy:
##########
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.grails.benchmarks.report
+
+import groovy.transform.CompileStatic
+
+import java.nio.charset.StandardCharsets
+import java.nio.file.Files
+import java.nio.file.Path
+
+@CompileStatic
+class JmhCompare {
+ static void main(String[] args) {
+ int exit = run(args, new GitHubComments())
+ if (exit != 0) {
+ System.exit(exit)
+ }
+ }
+
+ static int run(String[] args, CommentPoster poster) {
+ return run(args, poster, System.getenv())
+ }
+
+ static int run(String[] args, CommentPoster poster, Map<String, String> environment) {
+ try {
+ Map<String, String> options = parse(args)
+ if (options.containsKey('post-file')) {
+ return postFile(options, poster, environment)
+ }
+ double threshold = options.containsKey('threshold') ? Double.parseDouble(options.get('threshold')) : .10D
+ if (!Double.isFinite(threshold) || threshold <= 0D) {
+ throw new IllegalArgumentException('--threshold must be greater than zero')
+ }
+ String headPath = options.get('head')
+ if (!headPath) {
+ throw new IllegalArgumentException('--head is required')
+ }
+ Map<String, Map<String, Benchmark>> head = JmhResults.readShards(headPath)
+ String basePath = options.get('base')
+ String report = basePath && Files.exists(Path.of(basePath))
+ ? ReportRenderer.render(BenchmarkComparator.compareShards(head, JmhResults.readShards(basePath), threshold,
+ options.getOrDefault('expected-shards', '').split(',').findAll { String value -> !value.trim().isEmpty() }))
+ : ReportRenderer.headOnly(JmhResults.poolShards(head))
+ System.out.println(report)
+ if (options.containsKey('output')) {
+ Files.writeString(Path.of(options.get('output')), report + '\n', StandardCharsets.UTF_8)
+ }
+ postWhenConfigured(report, options, poster, environment)
+ return 0
+ } catch (Exception error) {
+ System.err.println("error: ${error.message}")
+ return 2
+ }
+ }
+
+ private static int postFile(Map<String, String> options, CommentPoster poster, Map<String, String> environment) {
+ if (options.containsKey('head')) {
+ throw new IllegalArgumentException('--post-file cannot be used with --head')
+ }
+ String report = Files.readString(Path.of(options.get('post-file')), StandardCharsets.UTF_8)
+ postWhenConfigured(report, options, poster, environment)
+ return 0
+ }
+
+ private static void postWhenConfigured(String report, Map<String, String> options, CommentPoster poster, Map<String, String> environment) {
+ String pr = options.getOrDefault('pr-number', '').trim()
+ String repo = options.get('repo')
+ String token = environment.get('GITHUB_TOKEN')
+ if (!pr || pr == 'null' || !repo || !token) {
+ if (options.containsKey('post-file')) {
+ System.err.println('warning: --post-file requires --repo, --pr-number, and GITHUB_TOKEN; skipping comment post')
+ }
+ return
+ }
+ try {
+ poster.post(report, repo, pr, token)
+ } catch (Exception error) {
+ System.err.println("warning: unable to post JMH report: ${error.message}")
+ }
+ }
+
+ private static Map<String, String> parse(String[] args) {
+ Set<String> values = ['head', 'base', 'threshold', 'repo', 'pr-number', 'expected-shards', 'output', 'post-file'] as Set<String>
+ Map<String, String> options = new LinkedHashMap<>()
+ for (int index = 0; index < args.length; index++) {
+ String option = args[index]
+ if (!option.startsWith('--')) {
+ throw new IllegalArgumentException("unknown option: ${option}")
+ }
+ String key = option.substring(2)
+ if (key == 'fail-on-regression') {
Review Comment:
Fixed in 202d206a6a.
Removed the `--fail-on-regression` parse branch and the test that locked in the no-op success path. Passing the flag now fails CLI parsing (missing value / unknown option) instead of silently accepting it.
##########
grails-benchmarks/src/jmh/java/org/apache/grails/benchmarks/interceptors/UrlMappingMatcherBenchmark.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.grails.benchmarks.interceptors;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import grails.web.mapping.UrlMappingData;
+import grails.web.mapping.UrlMappingInfo;
+import org.grails.plugins.web.interceptors.UrlMappingMatcher;
+import org.grails.web.servlet.mvc.GrailsWebRequest;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Threads;
+import org.openjdk.jmh.annotations.Warmup;
+
+/**
+ * Measures URI matcher decisions that select Grails interceptors during request dispatch. Both
+ * matching and rejected paths matter because every interceptor evaluates incoming requests.
+ */
+@State(Scope.Benchmark)
+@Threads(1)
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
+@Fork(value = 2, jvmArgsAppend = {"-Xms1g", "-Xmx1g", "-XX:+UseG1GC"})
+public class UrlMappingMatcherBenchmark {
+
+ private UrlMappingMatcher matcher;
+ private UrlMappingInfo mappingInfo;
+
+ @Setup
+ public void setup() {
Review Comment:
Fixed in 202d206a6a.
Added `assertFixtureMatches()` guards called from `@Setup` in:
- `UrlMappingMatcherBenchmark` - match `/orders/42`, reject `/catalog/42`
- `SimpleDataBinderBenchmark` - both flat and type-converted binds populate all fields
- `ViewTemplateRenderingBenchmark` - JSON and markup renders contain expected content
Same fail-loud pattern as `UrlMappingsBenchmark`.
##########
grails-benchmarks/build.gradle:
##########
@@ -0,0 +1,268 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.gradle.api.file.DuplicatesStrategy
+import org.gradle.api.tasks.PathSensitivity
+
+import java.util.Properties
+import java.util.zip.ZipFile
+
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'me.champeau.jmh' version '0.7.3'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.compile'
+}
+
+version = projectVersion
+group = 'org.apache.grails'
+
+// This build-time-only benchmark harness is never published, so it deliberately omits
+// org.apache.grails.buildsrc.publish, org.apache.grails.buildsrc.sbom,
+// org.apache.grails.buildsrc.vulnerability-scan, org.apache.grails.gradle.grails-jacoco,
+// org.apache.grails.buildsrc.dependency-validator, and org.apache.grails.gradle.grails-code-style.
Review Comment:
Fixed in 202d206a6a.
Enabled `org.apache.grails.gradle.grails-code-style` and `org.apache.grails.buildsrc.vulnerability-scan`. Import-order / formatting issues in the module were cleaned so style checks pass. Publish/sbom/jacoco/dependency-validator stay off because this harness is build-time-only and JMH is Category X.
##########
grails-benchmarks/build.gradle:
##########
@@ -0,0 +1,268 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.gradle.api.file.DuplicatesStrategy
+import org.gradle.api.tasks.PathSensitivity
+
+import java.util.Properties
+import java.util.zip.ZipFile
+
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'me.champeau.jmh' version '0.7.3'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.compile'
+}
+
+version = projectVersion
+group = 'org.apache.grails'
+
+// This build-time-only benchmark harness is never published, so it deliberately omits
+// org.apache.grails.buildsrc.publish, org.apache.grails.buildsrc.sbom,
+// org.apache.grails.buildsrc.vulnerability-scan, org.apache.grails.gradle.grails-jacoco,
+// org.apache.grails.buildsrc.dependency-validator, and org.apache.grails.gradle.grails-code-style.
+// The latter remains omitted until existing JMH source import-order and fixture-formatting
+// violations can be remediated together.
+
+sourceSets {
+ report {
+ groovy.srcDirs = ['src/report/groovy']
+ }
+}
+
+// The Groovy fixtures under src/main/groovy build the framework objects whose APIs are
+// closure-based (the URL mappings DSL, the Validateable trait, view templates). The JMH
+// benchmarks themselves live in src/jmh/java and only *call* those fixtures, so that the
+// measured code path is plain Java and does not include Groovy's dynamic dispatch.
+// me.champeau.jmh puts the main source set's output on the jmh compile classpath, which is
+// why the fixtures are in main rather than src/jmh/groovy: keeping the benchmarks in a pure
+// Java source set preserves normal JMH annotation processing.
+dependencies {
+ implementation platform(project(':grails-bom'))
+
+ implementation project(':grails-web-url-mappings')
+ implementation project(':grails-databinding-core')
+ implementation project(':grails-gsp-core')
+ implementation project(':grails-interceptors')
+ implementation project(':grails-views-gson')
+ implementation project(':grails-views-markup')
+ implementation project(':grails-core')
+ implementation 'org.apache.groovy:groovy'
+
+ // The framework modules declare these as compileOnly, so they are absent from the runtime
+ // classpath a benchmark actually executes on. The modules' own test suites add them back
+ // the same way. Without them, URL mapping, interceptor and view benchmarks fail at @Setup
+ // with NoClassDefFoundError: jakarta/servlet/ServletContext.
+ implementation 'jakarta.servlet:jakarta.servlet-api'
+ implementation 'org.springframework:spring-test'
+
+ reportImplementation platform(project(':grails-bom'))
+ reportImplementation 'org.apache.groovy:groovy'
+ reportImplementation 'org.apache.groovy:groovy-json'
+
+ // Pull the report source set's full runtime classpath (groovy-json etc.), not only its
+ // compiled classes - the Spock suite exercises JsonSlurper and HttpClient paths.
+ testImplementation sourceSets.report.runtimeClasspath
+ testImplementation 'org.apache.groovy:groovy-test-junit5'
+ testImplementation 'org.junit.jupiter:junit-jupiter-api'
+ testImplementation 'org.junit.platform:junit-platform-suite'
+ testImplementation 'org.spockframework:spock-core'
+ testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine'
+}
+
+apply from: rootProject.layout.projectDirectory.file('gradle/test-config.gradle')
+
+tasks.register('jmhCompare', JavaExec) {
+ group = 'verification'
+ description = 'Compares JMH JSON results and renders a pull request report.'
+ classpath = sourceSets.report.runtimeClasspath
+ mainClass = 'org.apache.grails.benchmarks.report.JmhCompare'
+}
+
+// The jmh source set compiles and runs against everything main depends on.
+configurations.jmh.extendsFrom(configurations.implementation)
+
+// Every JMH knob the CI workflow needs to override is exposed as a Gradle property so that
+// .github/workflows/benchmark.yml can run a fast PR profile and a slower scheduled profile
+// from the same build. Defaults here are the quick local-development profile.
+jmh {
+ jmhVersion = '1.37'
+ includes = providers.gradleProperty('jmh.include')
+ .map { it.split(',') as List<String> }
+ .orElse(['.*'])
+ .get()
+ fork = providers.gradleProperty('jmh.forks').map { it as Integer }.orElse(1).get()
+ warmupIterations = providers.gradleProperty('jmh.warmupIterations').map { it as Integer }.orElse(1).get()
+ iterations = providers.gradleProperty('jmh.iterations').map { it as Integer }.orElse(1).get()
+ warmup = providers.gradleProperty('jmh.warmupTime').orElse('1s').get()
+ timeOnIteration = providers.gradleProperty('jmh.iterationTime').orElse('1s').get()
+ resultFormat = providers.gradleProperty('jmh.resultFormat').orElse('JSON').get()
+ // A benchmark that throws during @Setup is otherwise dropped from the results file while
+ // the build still reports success, which would silently shrink the comparison set.
+ failOnError = providers.gradleProperty('jmh.failOnError').map { it.toBoolean() }.orElse(true).get()
+ resultsFile = file(providers.gradleProperty('jmh.resultFile')
+ .orElse(layout.buildDirectory.file('results/jmh/results.json').get().asFile.absolutePath)
+ .get())
+ // Comma-separated JMH profilers, e.g. -Pjmh.profilers=gc for allocation reporting.
+ profilers = providers.gradleProperty('jmh.profilers')
+ .map { it.split(',') as List<String> }
+ .orElse([])
+ .get()
+}
+
+def mergeJmhClasspathMetadata = tasks.register('mergeJmhClasspathMetadata') {
+ def mergedMetadataDirectory = layout.buildDirectory.dir('generated/jmh-classpath-metadata')
+ def lineMetadataPrefixes = ['META-INF/services/', 'META-INF/groovy/']
+ def extensionModuleSuffix = 'org.codehaus.groovy.runtime.ExtensionModule'
+ def propertiesMetadataPaths = [
+ 'META-INF/spring.factories',
+ 'META-INF/spring.handlers',
+ 'META-INF/spring.schemas'
+ ]
+
+ inputs.files(configurations.jmhRuntimeClasspath)
+ .withPropertyName('jmhRuntimeClasspath')
+ .withPathSensitivity(PathSensitivity.RELATIVE)
+ outputs.dir(mergedMetadataDirectory)
+ outputs.cacheIf { true }
+
+ doLast {
+ File outputDirectory = mergedMetadataDirectory.get().asFile
+ project.delete(outputDirectory)
+
+ Map<String, Set<String>> lineEntries = new TreeMap<>()
+ Map<String, Map<String, Set<String>>> extensionModuleEntries = new TreeMap<>()
+ Map<String, Map<String, Set<String>>> propertyEntries = new TreeMap<>()
+
+ configurations.jmhRuntimeClasspath.files
+ .findAll { File artifact -> artifact.name.endsWith('.jar') }
+ .sort { File artifact -> artifact.absolutePath }
+ .each { File artifact ->
+ new ZipFile(artifact).withCloseable { ZipFile zipFile ->
+ zipFile.entries().each { entry ->
+ if (entry.directory) {
+ return
+ }
+
+ String path = entry.name
+ if (path.endsWith(extensionModuleSuffix)) {
+ Properties properties = new Properties()
+ zipFile.getInputStream(entry).withCloseable { input ->
+ properties.load(input)
+ }
+ Map<String, Set<String>> entries = extensionModuleEntries.computeIfAbsent(path) {
+ new TreeMap<>()
+ }
+ ['extensionClasses', 'staticExtensionClasses'].each { String key ->
+ Set<String> values = entries.computeIfAbsent(key) { new TreeSet<>() }
+ properties.getProperty(key, '').split(',').each { String value ->
+ if (value) {
+ values.add(value.trim())
+ }
+ }
+ }
+ } else if (lineMetadataPrefixes.any { String prefix -> path.startsWith(prefix) }) {
+ Set<String> lines = lineEntries.computeIfAbsent(path) { new TreeSet<>() }
+ zipFile.getInputStream(entry).withCloseable { input ->
+ input.getText('UTF-8').readLines().each { String line ->
+ if (line) {
+ lines.add(line)
+ }
+ }
+ }
+ } else if (propertiesMetadataPaths.contains(path)) {
+ Properties properties = new Properties()
+ zipFile.getInputStream(entry).withCloseable { input ->
+ properties.load(input)
+ }
+ Map<String, Set<String>> entries = propertyEntries.computeIfAbsent(path) {
+ new TreeMap<>()
+ }
+ properties.stringPropertyNames().each { String key ->
+ Set<String> values = entries.computeIfAbsent(key) { new TreeSet<>() }
+ properties.getProperty(key).split(',').each { String value ->
+ if (value) {
+ values.add(value.trim())
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
+ lineEntries.each { String path, Set<String> lines ->
+ File outputFile = new File(outputDirectory, path)
+ outputFile.parentFile.mkdirs()
+ outputFile.setText("${lines.join('\n')}\n", 'UTF-8')
+ }
+ extensionModuleEntries.each { String path, Map<String, Set<String>> entries ->
+ File outputFile = new File(outputDirectory, path)
+ outputFile.parentFile.mkdirs()
+ String moduleName = path.startsWith('META-INF/groovy/')
+ ? 'grails-benchmark-groovy-extension-modules'
+ : 'grails-benchmark-service-extension-modules'
+ outputFile.withWriter('UTF-8') { writer ->
+ writer.write("moduleName=${moduleName}\n")
+ writer.write('moduleVersion=1.0\n')
+ entries.each { String key, Set<String> values ->
+ writer.write("${key}=${values.join(',')}\n")
+ }
+ }
+ }
+ propertyEntries.each { String path, Map<String, Set<String>> entries ->
+ File outputFile = new File(outputDirectory, path)
+ outputFile.parentFile.mkdirs()
+ outputFile.withWriter('UTF-8') { writer ->
+ entries.each { String key, Set<String> values ->
+ String escapedKey = key.replace('\\', '\\\\').replace('=', '\\=').replace(':', '\\:')
+ writer.write("${escapedKey}=${values.join(',')}\n")
+ }
+ }
+ }
+ }
+}
+
+tasks.named('jmhJar') {
+ dependsOn mergeJmhClasspathMetadata
+ def mergedMetadataDirectory = layout.buildDirectory.dir('generated/jmh-classpath-metadata')
+ def mergedMetadataPath = mergedMetadataDirectory.get().asFile.absolutePath + File.separator
+ def lineMetadataPrefixes = ['META-INF/services/', 'META-INF/groovy/']
+ def propertiesMetadataPaths = [
+ 'META-INF/spring.factories',
+ 'META-INF/spring.handlers',
+ 'META-INF/spring.schemas'
+ ]
+
+ from(mergedMetadataDirectory)
+ eachFile { details ->
+ boolean mergeable = lineMetadataPrefixes.any { String prefix -> details.path.startsWith(prefix) } ||
+ propertiesMetadataPaths.contains(details.path)
+ if (mergeable && !details.file.absolutePath.startsWith(mergedMetadataPath)) {
+ details.exclude()
+ }
+ }
+ duplicatesStrategy = DuplicatesStrategy.EXCLUDE
+}
+
+tasks.named('check') {
+ // Benchmarks are not otherwise compiled by the standard lifecycle tasks.
+ dependsOn tasks.named('jmhClasses')
Review Comment:
Fixed in 202d206a6a.
Documented the decision on `check.dependsOn jmhClasses`: compile-on-every-build is preferred over workflow-only compilation so the suite cannot silently bit-rot between labelled runs; `jmhJar` remains the packaging gate.
##########
grails-benchmarks/src/test/groovy/org/apache/grails/benchmarks/report/GoldenReportSpec.groovy:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.grails.benchmarks.report
+
+import spock.lang.Specification
+import spock.lang.TempDir
+import spock.lang.Unroll
+
+import java.nio.file.Files
+import java.nio.file.Path
+
+class GoldenReportSpec extends Specification {
+ @TempDir
+ Path temporaryDirectory
+
+ @Unroll
+ def "#name report exactly matches the Python golden file"() {
Review Comment:
Fixed in 202d206a6a.
Renamed the golden test to `#name report exactly matches the golden fixture` and the number-format test to `matches percent-g style rendering`. No Python references remain under `grails-benchmarks`. Also normalized CRLF in the golden comparison so Windows CI does not false-fail on line endings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#issuecomment-5135424958
## β
All tests passed β
π·οΈ Commit: 9cf6166eaa23895154eb9dd2a8ebe2c14d9ae956
βΆοΈ Tests: 45521 executed
βͺοΈ Checks: 63/63 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#issuecomment-5200032512
## β
All tests passed β
π·οΈ Commit: 22c45df65a9fa38da1bf4f082c25a2131787900d
βΆοΈ Tests: 54614 executed
βͺοΈ Checks: 62/62 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "jdaugherty (via GitHub)" <gi...@apache.org>.
jdaugherty commented on code in PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#discussion_r3699955916
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,381 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
Review Comment:
`labeled` fires for *any* label, and the job gate at line 46 only checks that `performance` is **present**, not that it was the label just added. So on a PR that already carries `performance`, adding `bug`, `deps`, or anything else kicks off another full two-shard run β two framework builds and two JMH suites per shard.
Given the CI budget argument in the description, worth narrowing:
```yaml
if: >-
github.event_name == 'workflow_dispatch' ||
(contains(github.event.pull_request.labels.*.name, 'performance') &&
(github.event.action != 'labeled' || github.event.label.name == 'performance'))
```
##########
grails-benchmarks/build.gradle:
##########
@@ -0,0 +1,268 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.gradle.api.file.DuplicatesStrategy
+import org.gradle.api.tasks.PathSensitivity
+
+import java.util.Properties
+import java.util.zip.ZipFile
+
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'me.champeau.jmh' version '0.7.3'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.compile'
+}
+
+version = projectVersion
+group = 'org.apache.grails'
+
+// This build-time-only benchmark harness is never published, so it deliberately omits
+// org.apache.grails.buildsrc.publish, org.apache.grails.buildsrc.sbom,
+// org.apache.grails.buildsrc.vulnerability-scan, org.apache.grails.gradle.grails-jacoco,
+// org.apache.grails.buildsrc.dependency-validator, and org.apache.grails.gradle.grails-code-style.
Review Comment:
Two of these omissions read differently from the rest. `publish`/`sbom`/`jacoco` follow straightforwardly from "never published".
`grails-code-style` doesn't. The trailing note says it stays off "until existing JMH source import-order and fixture-formatting violations can be remediated together", which leaves a new module permanently invisible to `./gradlew codeStyle` and to the four aggregate violation reports the contributor guide asks for before every commit. It's about ten files β fixing them now avoids establishing the precedent that a new module can opt out.
`vulnerability-scan` is worth a second look too: this module pulls JMH and its transitives (jopt-simple, commons-math3) into the default build graph, and nothing else in the repo scans them.
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,381 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
+ paths-ignore:
+ - '**/*.md'
+ - '**/*.adoc'
+ - 'grails-doc/**'
+ workflow_dispatch:
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+permissions:
+ contents: read
+
+jobs:
+ # Each shard builds both revisions before measuring either one on the same runner.
+ # Building first avoids CPU, IO, thermal, cache, and frequency state biasing a measurement.
+ # Shard a measures BASE then HEAD, while shard b measures HEAD then BASE.
+ # Alternating the order cancels first-versus-second ordering bias without doubling runtime.
+ # Both shards use the PR merge commit as HEAD, so they measure what would actually land.
+ # Results are advisory: a detected regression never fails this workflow.
+ benchmark:
+ name: "Paired JMH benchmarks (${{ matrix.shard }})"
+ if: ${{ github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'performance') }}
+ runs-on: ubuntu-24.04
+ strategy:
+ fail-fast: false
+ matrix:
+ shard: [a, b]
+ env:
+ BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ HEAD_SHA: ${{ github.sha }}
+ JMH_INCLUDE: '.*'
+ PR_NUMBER: ${{ github.event.pull_request.number || 0 }}
+ REPOSITORY: ${{ github.repository }}
+ RESULT_DIR: ${{ github.workspace }}/jmh-results/${{ matrix.shard }}
+ REPORT_DIR: ${{ github.workspace }}/jmh-reports/${{ matrix.shard }}
+ SHARD: ${{ matrix.shard }}
+ steps:
+ - name: "π₯ Checkout repository"
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+ with:
+ fetch-depth: 0
+ - name: "βοΈ Setup JDK"
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
+ with:
+ distribution: liberica
+ java-version: 21
+ - name: "π Setup Gradle"
+ uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0
+ with:
+ cache-provider: basic # 'basic' uses the MIT-licensed, open-source cache provider; the default 'enhanced' provider (v6+) is proprietary (Gradle commercial Terms of Use)
+ develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+ # Deliberately allowed to fail the job, and placed before the expensive work: a broken
+ # comparison tool 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 tool"
+ run: ./gradlew :grails-benchmarks:test --max-workers=4
+ - name: "π³ Prepare paired worktrees"
+ run: |
+ WORKTREE_ROOT="$RUNNER_TEMP/jmh-worktrees/$SHARD"
+ mkdir -p "$WORKTREE_ROOT" "$RESULT_DIR" "$REPORT_DIR"
+ if [ -n "$BASE_SHA" ] && git cat-file -e "$BASE_SHA^{commit}" 2>/dev/null; then
+ RESOLVED_BASE_SHA="$BASE_SHA"
+ elif RESOLVED_BASE_SHA="$(git merge-base "$HEAD_SHA^1" "$HEAD_SHA^2" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using merge-base $RESOLVED_BASE_SHA."
+ elif RESOLVED_BASE_SHA="$(git rev-parse "$HEAD_SHA^" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using HEAD parent $RESOLVED_BASE_SHA."
+ else
+ RESOLVED_BASE_SHA=""
+ echo "No base commit could be resolved; comparison will use HEAD-only mode."
+ fi
+ git worktree add --detach "$WORKTREE_ROOT/head" "$HEAD_SHA"
+ {
+ printf 'HEAD_DIR=%s\n' "$WORKTREE_ROOT/head"
+ printf 'WORKTREE_ROOT=%s\n' "$WORKTREE_ROOT"
+ } >> "$GITHUB_ENV"
+ if [ -n "$RESOLVED_BASE_SHA" ] && git worktree add --detach "$WORKTREE_ROOT/base" "$RESOLVED_BASE_SHA"; then
+ {
+ printf 'BASE_DIR=%s\n' "$WORKTREE_ROOT/base"
+ printf 'RESOLVED_BASE_SHA=%s\n' "$RESOLVED_BASE_SHA"
+ } >> "$GITHUB_ENV"
+ echo "Using base commit $RESOLVED_BASE_SHA."
+ else
+ echo "BASE_BENCHMARKS_AVAILABLE=false" >> "$GITHUB_ENV"
+ fi
+ # Build both JMH jars before measuring either revision. Builds are CPU- and IO-heavy,
+ # so building and measuring one revision at a time would bias results with different
+ # thermal, cache, and CPU-frequency state on the shared runner.
+ - name: "π¨ Build paired JMH jars"
+ timeout-minutes: 60
+ run: |
+ base_build_ok=false
+ head_build_ok=false
+ if [ "${BASE_BENCHMARKS_AVAILABLE:-true}" = "true" ] && [ -f "$BASE_DIR/grails-benchmarks/build.gradle" ]; then
+ if (
+ cd "$BASE_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ base_build_ok=true
+ echo "Built base benchmark at $RESOLVED_BASE_SHA."
+ else
+ echo "BASE benchmark JAR build failed."
+ fi
+ else
+ echo "BASE does not contain grails-benchmarks; comparison will use HEAD-only mode."
+ fi
+ if (
+ cd "$HEAD_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ head_build_ok=true
+ echo "Built HEAD benchmark at $HEAD_SHA."
+ else
+ echo "HEAD benchmark JAR build failed."
+ fi
+ {
+ printf 'BASE_BUILD_OK=%s\n' "$base_build_ok"
+ printf 'HEAD_BUILD_OK=%s\n' "$head_build_ok"
+ } >> "$GITHUB_ENV"
+ # Two forks, three warmup iterations, and five measurement iterations balance PR latency
+ # against confidence. Reversing shard order cancels first-versus-second runner-state bias.
+ - name: "π‘οΈ Run paired JMH benchmarks"
+ timeout-minutes: 60
+ run: |
+ # JMH writes results incrementally, so a run that dies partway can leave a file that
+ # parses perfectly while describing only some of the benchmarks. The report job decides
+ # completeness from which files exist, so a partial file would be indistinguishable from
+ # a good one. Write to a staging path and publish it only on success, so a failed run
+ # leaves NO file rather than a plausible one.
+ run_benchmark() {
+ local revision_dir="$1"
+ local result_file="$2"
+ local staging_file="$result_file.partial"
+ rm -f "$staging_file" "$result_file"
+ if (
+ cd "$revision_dir" && ./gradlew :grails-benchmarks:jmh \
+ -Pjmh.include="$JMH_INCLUDE" \
+ -Pjmh.forks=2 \
+ -Pjmh.warmupIterations=3 \
+ -Pjmh.iterations=5 \
+ -Pjmh.resultFile="$staging_file" \
+ -Pjmh.profilers=gc \
+ --max-workers=4
+ ) && [ -s "$staging_file" ] && mv "$staging_file" "$result_file"; then
+ return 0
+ fi
+ rm -f "$staging_file"
+ return 1
+ }
+
+ base_run_failed=false
+ head_run_failed=false
+ run_base_benchmark() {
+ if [ "${BASE_BUILD_OK:-false}" != "true" ]; then
+ echo "BASE benchmark execution skipped because its JAR was not built."
+ elif ! run_benchmark "$BASE_DIR" "$RESULT_DIR/base.json"; then
+ base_run_failed=true
+ echo "BASE benchmark execution failed."
+ fi
+ }
+ run_head_benchmark() {
+ if [ "${HEAD_BUILD_OK:-false}" != "true" ]; then
+ echo "HEAD benchmark execution skipped because its JAR was not built."
+ elif ! run_benchmark "$HEAD_DIR" "$RESULT_DIR/head.json"; then
+ head_run_failed=true
+ echo "HEAD benchmark execution failed."
+ fi
+ }
+
+ if [ "$SHARD" = "a" ]; then
+ run_base_benchmark
+ run_head_benchmark
+ else
+ run_head_benchmark
+ run_base_benchmark
+ fi
+ {
+ printf 'BASE_RUN_FAILED=%s\n' "$base_run_failed"
+ printf 'HEAD_RUN_FAILED=%s\n' "$head_run_failed"
+ } >> "$GITHUB_ENV"
+ # The comparison is advisory. The comparison tool exits successfully for regressions, and this
+ # step is non-blocking even if results are incomplete because a benchmark execution failed.
+ # No --pr-number is passed here on purpose: this job renders the report only. The separate
+ # report job owns comment posting, so a two-shard matrix cannot produce duplicate comments.
+ - name: "π Compare JMH results"
+ if: always()
+ continue-on-error: true
+ run: |
+ report_file="$REPORT_DIR/comparison.md"
+ {
+ printf '### JMH shard `%s`\n\n' "$SHARD"
+ if [ "${BASE_RUN_FAILED:-false}" = "true" ]; then
+ printf 'BASE benchmark execution failed.\n\n'
+ fi
+ if [ "${BASE_BUILD_OK:-false}" != "true" ]; then
+ printf 'BASE benchmark JAR was not built.\n\n'
+ fi
+ if [ "${HEAD_RUN_FAILED:-false}" = "true" ]; then
+ printf 'HEAD benchmark execution failed.\n\n'
+ fi
+ if [ "${HEAD_BUILD_OK:-false}" != "true" ]; then
+ printf 'HEAD benchmark JAR was not built.\n\n'
+ fi
+ } > "$report_file"
+ comparison_file="$RUNNER_TEMP/jmh-comparison-$SHARD.md"
+ if [ ! -f "$RESULT_DIR/head.json" ]; then
+ printf 'HEAD benchmark result was not produced.\n' >> "$report_file"
+ elif [ "${BASE_BUILD_OK:-false}" = "true" ] && [ -f "$RESULT_DIR/base.json" ]; then
+ ./gradlew -q --console=plain :grails-benchmarks:jmhCompare --args="--head $RESULT_DIR/head.json --base $RESULT_DIR/base.json --output $comparison_file"
+ cat "$comparison_file" >> "$report_file"
+ else
+ ./gradlew -q --console=plain :grails-benchmarks:jmhCompare --args="--head $RESULT_DIR/head.json --output $comparison_file"
+ cat "$comparison_file" >> "$report_file"
+ fi
+ - name: "π Publish JMH report in job summary"
+ if: always()
+ run: |
+ if [ -f "$REPORT_DIR/comparison.md" ]; then
+ cat "$REPORT_DIR/comparison.md" >> "$GITHUB_STEP_SUMMARY"
+ else
+ printf '## JMH benchmark comparison\n\nNo comparison report was produced.\n' >> "$GITHUB_STEP_SUMMARY"
+ fi
+ - name: "π€ Upload JMH artifacts"
+ if: always()
+ uses: actions/upload-artifact@043fb46d1a93c77aae656e7c1c64a875d1fc6a0a # v7.0.1
+ with:
+ name: jmh-results-${{ matrix.shard }}
+ path: |
+ jmh-results/${{ matrix.shard }}/
+ jmh-reports/${{ matrix.shard }}/
+ if-no-files-found: warn
+ - name: "π§Ή Remove paired worktrees"
+ if: always()
+ run: |
+ WORKTREE_ROOT="$RUNNER_TEMP/jmh-worktrees/$SHARD"
+ git worktree remove --force "$WORKTREE_ROOT/base" || true
+ git worktree remove --force "$WORKTREE_ROOT/head" || true
+
+ report:
+ name: "Publish JMH benchmark comparison"
+ needs: benchmark
+ if: ${{ always() && github.event_name == 'pull_request' && contains(github.event.pull_request.labels.*.name, 'performance') && github.event.pull_request.head.repo.full_name == github.repository }}
Review Comment:
`always()` also runs this job when the `benchmark` jobs failed outright. Concrete case: the "π§ͺ Verify JMH comparison tool" gate fails, every later step in the shard job is skipped, so no results are uploaded. The pooling step then falls through to the final `else` and posts *"No HEAD benchmark results were produced"* β which, because the comment is sticky, **replaces the previous good report on that PR with an empty one**. A tooling failure silently destroys the last known-good numbers.
Worth either skipping the post step when there are zero results (leaving the prior comment intact), or making the message say the run failed and the earlier report still stands.
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,381 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
+ paths-ignore:
+ - '**/*.md'
+ - '**/*.adoc'
+ - 'grails-doc/**'
+ workflow_dispatch:
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+permissions:
+ contents: read
+
+jobs:
+ # Each shard builds both revisions before measuring either one on the same runner.
+ # Building first avoids CPU, IO, thermal, cache, and frequency state biasing a measurement.
+ # Shard a measures BASE then HEAD, while shard b measures HEAD then BASE.
+ # Alternating the order cancels first-versus-second ordering bias without doubling runtime.
+ # Both shards use the PR merge commit as HEAD, so they measure what would actually land.
+ # Results are advisory: a detected regression never fails this workflow.
+ benchmark:
+ name: "Paired JMH benchmarks (${{ matrix.shard }})"
+ if: ${{ github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'performance') }}
+ runs-on: ubuntu-24.04
+ strategy:
+ fail-fast: false
+ matrix:
+ shard: [a, b]
+ env:
+ BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ HEAD_SHA: ${{ github.sha }}
+ JMH_INCLUDE: '.*'
+ PR_NUMBER: ${{ github.event.pull_request.number || 0 }}
Review Comment:
`PR_NUMBER` and `REPOSITORY` are never referenced in this job β the compare step deliberately doesn't post, and the `report` job declares its own copies at lines 271-272. Dead as written; dropping them keeps it clear that this job has no comment-posting path.
##########
grails-benchmarks/src/jmh/java/org/apache/grails/benchmarks/interceptors/UrlMappingMatcherBenchmark.java:
##########
@@ -0,0 +1,144 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.grails.benchmarks.interceptors;
+
+import java.util.Collections;
+import java.util.Map;
+import java.util.concurrent.TimeUnit;
+
+import grails.web.mapping.UrlMappingData;
+import grails.web.mapping.UrlMappingInfo;
+import org.grails.plugins.web.interceptors.UrlMappingMatcher;
+import org.grails.web.servlet.mvc.GrailsWebRequest;
+import org.openjdk.jmh.annotations.Benchmark;
+import org.openjdk.jmh.annotations.BenchmarkMode;
+import org.openjdk.jmh.annotations.Fork;
+import org.openjdk.jmh.annotations.Measurement;
+import org.openjdk.jmh.annotations.Mode;
+import org.openjdk.jmh.annotations.OutputTimeUnit;
+import org.openjdk.jmh.annotations.Scope;
+import org.openjdk.jmh.annotations.Setup;
+import org.openjdk.jmh.annotations.State;
+import org.openjdk.jmh.annotations.Threads;
+import org.openjdk.jmh.annotations.Warmup;
+
+/**
+ * Measures URI matcher decisions that select Grails interceptors during request dispatch. Both
+ * matching and rejected paths matter because every interceptor evaluates incoming requests.
+ */
+@State(Scope.Benchmark)
+@Threads(1)
+@BenchmarkMode(Mode.AverageTime)
+@OutputTimeUnit(TimeUnit.NANOSECONDS)
+@Warmup(iterations = 3, time = 1, timeUnit = TimeUnit.SECONDS)
+@Measurement(iterations = 5, time = 1, timeUnit = TimeUnit.SECONDS)
+@Fork(value = 2, jvmArgsAppend = {"-Xms1g", "-Xmx1g", "-XX:+UseG1GC"})
+public class UrlMappingMatcherBenchmark {
+
+ private UrlMappingMatcher matcher;
+ private UrlMappingInfo mappingInfo;
+
+ @Setup
+ public void setup() {
Review Comment:
`UrlMappingsBenchmark` guards its fixture in `@Setup` and fails the run rather than publish wrong numbers, and the description makes a strong case for why. This benchmark has the same failure mode with no guard: if `doesMatch` ever stops returning `true` for `/orders/42`, `matchUriPattern` silently becomes a second copy of `rejectNonMatchingUriPattern` and the report shows two plausible, nearly identical numbers with no error anywhere.
I ran a probe on this branch against `sourceSets.jmh.runtimeClasspath` and confirmed the fixtures are all fine today β `doesMatch` returns `true` then `false`, both binder targets come back fully populated, both templates render non-empty β so this is not a live defect. But the same one-line assertion applies here, in `SimpleDataBinderBenchmark` (`SimpleDataBinder.bind` skips properties it can't bind, silently), and in `ViewTemplateRenderingBenchmark`. Given the silent-no-op story is the lesson the description leads with, the guard is worth applying uniformly rather than only where the bug happened to be found.
##########
grails-benchmarks/src/test/groovy/org/apache/grails/benchmarks/report/GoldenReportSpec.groovy:
##########
@@ -0,0 +1,71 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.grails.benchmarks.report
+
+import spock.lang.Specification
+import spock.lang.TempDir
+import spock.lang.Unroll
+
+import java.nio.file.Files
+import java.nio.file.Path
+
+class GoldenReportSpec extends Specification {
+ @TempDir
+ Path temporaryDirectory
+
+ @Unroll
+ def "#name report exactly matches the Python golden file"() {
Review Comment:
Leftover from the Python implementation this replaced β "the Python golden file" here, and "matches Python percent g rendering" in `JmhCompareSpec` line 81. There's no Python anywhere in the PR now, so these read as references to something that doesn't exist. The fixtures are still worth keeping as byte-exact locks; they're just locking the Groovy renderer's output.
##########
.github/workflows/benchmark.yml:
##########
@@ -0,0 +1,381 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership.
+# The ASF licenses this file to You under the Apache License, Version 2.0
+# (the "License"); you may not use this file except in compliance with
+# the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+name: "JMH Benchmark Comparison"
+
+# SECURITY: This workflow deliberately uses pull_request, never pull_request_target.
+# Pull requests run untrusted code, and Apache Infra policy forbids exposing tokens
+# to that code through a privileged pull_request_target workflow.
+on:
+ pull_request:
+ types: [opened, synchronize, reopened, labeled]
+ paths-ignore:
+ - '**/*.md'
+ - '**/*.adoc'
+ - 'grails-doc/**'
+ workflow_dispatch:
+
+concurrency:
+ group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.run_id }}
+ cancel-in-progress: ${{ github.event_name == 'pull_request' }}
+
+permissions:
+ contents: read
+
+jobs:
+ # Each shard builds both revisions before measuring either one on the same runner.
+ # Building first avoids CPU, IO, thermal, cache, and frequency state biasing a measurement.
+ # Shard a measures BASE then HEAD, while shard b measures HEAD then BASE.
+ # Alternating the order cancels first-versus-second ordering bias without doubling runtime.
+ # Both shards use the PR merge commit as HEAD, so they measure what would actually land.
+ # Results are advisory: a detected regression never fails this workflow.
+ benchmark:
+ name: "Paired JMH benchmarks (${{ matrix.shard }})"
+ if: ${{ github.event_name == 'workflow_dispatch' || contains(github.event.pull_request.labels.*.name, 'performance') }}
+ runs-on: ubuntu-24.04
+ strategy:
+ fail-fast: false
+ matrix:
+ shard: [a, b]
+ env:
+ BASE_SHA: ${{ github.event.pull_request.base.sha }}
+ HEAD_SHA: ${{ github.sha }}
+ JMH_INCLUDE: '.*'
+ PR_NUMBER: ${{ github.event.pull_request.number || 0 }}
+ REPOSITORY: ${{ github.repository }}
+ RESULT_DIR: ${{ github.workspace }}/jmh-results/${{ matrix.shard }}
+ REPORT_DIR: ${{ github.workspace }}/jmh-reports/${{ matrix.shard }}
+ SHARD: ${{ matrix.shard }}
+ steps:
+ - name: "π₯ Checkout repository"
+ uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
+ with:
+ fetch-depth: 0
+ - name: "βοΈ Setup JDK"
+ uses: actions/setup-java@be666c2fcd27ec809703dec50e508c2fdc7f6654 # v5.2.0
+ with:
+ distribution: liberica
+ java-version: 21
+ - name: "π Setup Gradle"
+ uses: gradle/actions/setup-gradle@50e97c2cd7a37755bbfafc9c5b7cafaece252f6e # v6.1.0
+ with:
+ cache-provider: basic # 'basic' uses the MIT-licensed, open-source cache provider; the default 'enhanced' provider (v6+) is proprietary (Gradle commercial Terms of Use)
+ develocity-access-key: ${{ secrets.DEVELOCITY_ACCESS_KEY }}
+ # Deliberately allowed to fail the job, and placed before the expensive work: a broken
+ # comparison tool 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 tool"
+ run: ./gradlew :grails-benchmarks:test --max-workers=4
+ - name: "π³ Prepare paired worktrees"
+ run: |
+ WORKTREE_ROOT="$RUNNER_TEMP/jmh-worktrees/$SHARD"
+ mkdir -p "$WORKTREE_ROOT" "$RESULT_DIR" "$REPORT_DIR"
+ if [ -n "$BASE_SHA" ] && git cat-file -e "$BASE_SHA^{commit}" 2>/dev/null; then
+ RESOLVED_BASE_SHA="$BASE_SHA"
+ elif RESOLVED_BASE_SHA="$(git merge-base "$HEAD_SHA^1" "$HEAD_SHA^2" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using merge-base $RESOLVED_BASE_SHA."
+ elif RESOLVED_BASE_SHA="$(git rev-parse "$HEAD_SHA^" 2>/dev/null)"; then
+ echo "Configured base commit is unreachable; using HEAD parent $RESOLVED_BASE_SHA."
+ else
+ RESOLVED_BASE_SHA=""
+ echo "No base commit could be resolved; comparison will use HEAD-only mode."
+ fi
+ git worktree add --detach "$WORKTREE_ROOT/head" "$HEAD_SHA"
+ {
+ printf 'HEAD_DIR=%s\n' "$WORKTREE_ROOT/head"
+ printf 'WORKTREE_ROOT=%s\n' "$WORKTREE_ROOT"
+ } >> "$GITHUB_ENV"
+ if [ -n "$RESOLVED_BASE_SHA" ] && git worktree add --detach "$WORKTREE_ROOT/base" "$RESOLVED_BASE_SHA"; then
+ {
+ printf 'BASE_DIR=%s\n' "$WORKTREE_ROOT/base"
+ printf 'RESOLVED_BASE_SHA=%s\n' "$RESOLVED_BASE_SHA"
+ } >> "$GITHUB_ENV"
+ echo "Using base commit $RESOLVED_BASE_SHA."
+ else
+ echo "BASE_BENCHMARKS_AVAILABLE=false" >> "$GITHUB_ENV"
+ fi
+ # Build both JMH jars before measuring either revision. Builds are CPU- and IO-heavy,
+ # so building and measuring one revision at a time would bias results with different
+ # thermal, cache, and CPU-frequency state on the shared runner.
+ - name: "π¨ Build paired JMH jars"
+ timeout-minutes: 60
+ run: |
+ base_build_ok=false
+ head_build_ok=false
+ if [ "${BASE_BENCHMARKS_AVAILABLE:-true}" = "true" ] && [ -f "$BASE_DIR/grails-benchmarks/build.gradle" ]; then
+ if (
+ cd "$BASE_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ base_build_ok=true
+ echo "Built base benchmark at $RESOLVED_BASE_SHA."
+ else
+ echo "BASE benchmark JAR build failed."
+ fi
+ else
+ echo "BASE does not contain grails-benchmarks; comparison will use HEAD-only mode."
+ fi
+ if (
+ cd "$HEAD_DIR"
+ ./gradlew :grails-benchmarks:jmhJar --max-workers=4
+ ); then
+ head_build_ok=true
+ echo "Built HEAD benchmark at $HEAD_SHA."
+ else
+ echo "HEAD benchmark JAR build failed."
+ fi
+ {
+ printf 'BASE_BUILD_OK=%s\n' "$base_build_ok"
+ printf 'HEAD_BUILD_OK=%s\n' "$head_build_ok"
+ } >> "$GITHUB_ENV"
+ # Two forks, three warmup iterations, and five measurement iterations balance PR latency
+ # against confidence. Reversing shard order cancels first-versus-second runner-state bias.
+ - name: "π‘οΈ Run paired JMH benchmarks"
+ timeout-minutes: 60
+ run: |
+ # JMH writes results incrementally, so a run that dies partway can leave a file that
+ # parses perfectly while describing only some of the benchmarks. The report job decides
+ # completeness from which files exist, so a partial file would be indistinguishable from
+ # a good one. Write to a staging path and publish it only on success, so a failed run
+ # leaves NO file rather than a plausible one.
+ run_benchmark() {
+ local revision_dir="$1"
+ local result_file="$2"
+ local staging_file="$result_file.partial"
+ rm -f "$staging_file" "$result_file"
+ if (
+ cd "$revision_dir" && ./gradlew :grails-benchmarks:jmh \
Review Comment:
Both revisions were built in the preceding step, so by the time measurement starts there are up to two Gradle daemons resident (3h default expiry) alongside this build's own, all on a 4-core runner, for the whole measurement window. Idle daemons still hold heap and run background GC.
The rulers exist to *detect* exactly this class of noise β cheaper to remove a known source of it. Either `--no-daemon` on the two measurement invocations, or a `./gradlew --stop` between the build step and this one, would leave the JMH fork as the only significant JVM competing for the runner.
##########
grails-benchmarks/src/report/groovy/org/apache/grails/benchmarks/report/JmhCompare.groovy:
##########
@@ -0,0 +1,116 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.grails.benchmarks.report
+
+import groovy.transform.CompileStatic
+
+import java.nio.charset.StandardCharsets
+import java.nio.file.Files
+import java.nio.file.Path
+
+@CompileStatic
+class JmhCompare {
+ static void main(String[] args) {
+ int exit = run(args, new GitHubComments())
+ if (exit != 0) {
+ System.exit(exit)
+ }
+ }
+
+ static int run(String[] args, CommentPoster poster) {
+ return run(args, poster, System.getenv())
+ }
+
+ static int run(String[] args, CommentPoster poster, Map<String, String> environment) {
+ try {
+ Map<String, String> options = parse(args)
+ if (options.containsKey('post-file')) {
+ return postFile(options, poster, environment)
+ }
+ double threshold = options.containsKey('threshold') ? Double.parseDouble(options.get('threshold')) : .10D
+ if (!Double.isFinite(threshold) || threshold <= 0D) {
+ throw new IllegalArgumentException('--threshold must be greater than zero')
+ }
+ String headPath = options.get('head')
+ if (!headPath) {
+ throw new IllegalArgumentException('--head is required')
+ }
+ Map<String, Map<String, Benchmark>> head = JmhResults.readShards(headPath)
+ String basePath = options.get('base')
+ String report = basePath && Files.exists(Path.of(basePath))
+ ? ReportRenderer.render(BenchmarkComparator.compareShards(head, JmhResults.readShards(basePath), threshold,
+ options.getOrDefault('expected-shards', '').split(',').findAll { String value -> !value.trim().isEmpty() }))
+ : ReportRenderer.headOnly(JmhResults.poolShards(head))
+ System.out.println(report)
+ if (options.containsKey('output')) {
+ Files.writeString(Path.of(options.get('output')), report + '\n', StandardCharsets.UTF_8)
+ }
+ postWhenConfigured(report, options, poster, environment)
+ return 0
+ } catch (Exception error) {
+ System.err.println("error: ${error.message}")
+ return 2
+ }
+ }
+
+ private static int postFile(Map<String, String> options, CommentPoster poster, Map<String, String> environment) {
+ if (options.containsKey('head')) {
+ throw new IllegalArgumentException('--post-file cannot be used with --head')
+ }
+ String report = Files.readString(Path.of(options.get('post-file')), StandardCharsets.UTF_8)
+ postWhenConfigured(report, options, poster, environment)
+ return 0
+ }
+
+ private static void postWhenConfigured(String report, Map<String, String> options, CommentPoster poster, Map<String, String> environment) {
+ String pr = options.getOrDefault('pr-number', '').trim()
+ String repo = options.get('repo')
+ String token = environment.get('GITHUB_TOKEN')
+ if (!pr || pr == 'null' || !repo || !token) {
+ if (options.containsKey('post-file')) {
+ System.err.println('warning: --post-file requires --repo, --pr-number, and GITHUB_TOKEN; skipping comment post')
+ }
+ return
+ }
+ try {
+ poster.post(report, repo, pr, token)
+ } catch (Exception error) {
+ System.err.println("warning: unable to post JMH report: ${error.message}")
+ }
+ }
+
+ private static Map<String, String> parse(String[] args) {
+ Set<String> values = ['head', 'base', 'threshold', 'repo', 'pr-number', 'expected-shards', 'output', 'post-file'] as Set<String>
+ Map<String, String> options = new LinkedHashMap<>()
+ for (int index = 0; index < args.length; index++) {
+ String option = args[index]
+ if (!option.startsWith('--')) {
+ throw new IllegalArgumentException("unknown option: ${option}")
+ }
+ String key = option.substring(2)
+ if (key == 'fail-on-regression') {
Review Comment:
`--fail-on-regression` is parsed and stored but never read β `run()` returns 0 regardless, and `JmhCompareSpec` line 113 ("fail on regression remains successful") locks that in.
Since gating is explicitly out of scope for this PR, I'd drop the flag and that test rather than ship a CLI option whose name promises behaviour it doesn't have. As it stands someone reaches for it later, sees it accepted without error, and assumes the build is gated.
##########
grails-benchmarks/build.gradle:
##########
@@ -0,0 +1,268 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import org.gradle.api.file.DuplicatesStrategy
+import org.gradle.api.tasks.PathSensitivity
+
+import java.util.Properties
+import java.util.zip.ZipFile
+
+plugins {
+ id 'groovy'
+ id 'java-library'
+ id 'me.champeau.jmh' version '0.7.3'
+ id 'org.apache.grails.buildsrc.properties'
+ id 'org.apache.grails.buildsrc.compile'
+}
+
+version = projectVersion
+group = 'org.apache.grails'
+
+// This build-time-only benchmark harness is never published, so it deliberately omits
+// org.apache.grails.buildsrc.publish, org.apache.grails.buildsrc.sbom,
+// org.apache.grails.buildsrc.vulnerability-scan, org.apache.grails.gradle.grails-jacoco,
+// org.apache.grails.buildsrc.dependency-validator, and org.apache.grails.gradle.grails-code-style.
+// The latter remains omitted until existing JMH source import-order and fixture-formatting
+// violations can be remediated together.
+
+sourceSets {
+ report {
+ groovy.srcDirs = ['src/report/groovy']
+ }
+}
+
+// The Groovy fixtures under src/main/groovy build the framework objects whose APIs are
+// closure-based (the URL mappings DSL, the Validateable trait, view templates). The JMH
+// benchmarks themselves live in src/jmh/java and only *call* those fixtures, so that the
+// measured code path is plain Java and does not include Groovy's dynamic dispatch.
+// me.champeau.jmh puts the main source set's output on the jmh compile classpath, which is
+// why the fixtures are in main rather than src/jmh/groovy: keeping the benchmarks in a pure
+// Java source set preserves normal JMH annotation processing.
+dependencies {
+ implementation platform(project(':grails-bom'))
+
+ implementation project(':grails-web-url-mappings')
+ implementation project(':grails-databinding-core')
+ implementation project(':grails-gsp-core')
+ implementation project(':grails-interceptors')
+ implementation project(':grails-views-gson')
+ implementation project(':grails-views-markup')
+ implementation project(':grails-core')
+ implementation 'org.apache.groovy:groovy'
+
+ // The framework modules declare these as compileOnly, so they are absent from the runtime
+ // classpath a benchmark actually executes on. The modules' own test suites add them back
+ // the same way. Without them, URL mapping, interceptor and view benchmarks fail at @Setup
+ // with NoClassDefFoundError: jakarta/servlet/ServletContext.
+ implementation 'jakarta.servlet:jakarta.servlet-api'
+ implementation 'org.springframework:spring-test'
+
+ reportImplementation platform(project(':grails-bom'))
+ reportImplementation 'org.apache.groovy:groovy'
+ reportImplementation 'org.apache.groovy:groovy-json'
+
+ // Pull the report source set's full runtime classpath (groovy-json etc.), not only its
+ // compiled classes - the Spock suite exercises JsonSlurper and HttpClient paths.
+ testImplementation sourceSets.report.runtimeClasspath
+ testImplementation 'org.apache.groovy:groovy-test-junit5'
+ testImplementation 'org.junit.jupiter:junit-jupiter-api'
+ testImplementation 'org.junit.platform:junit-platform-suite'
+ testImplementation 'org.spockframework:spock-core'
+ testRuntimeOnly 'org.junit.jupiter:junit-jupiter-engine'
+}
+
+apply from: rootProject.layout.projectDirectory.file('gradle/test-config.gradle')
+
+tasks.register('jmhCompare', JavaExec) {
+ group = 'verification'
+ description = 'Compares JMH JSON results and renders a pull request report.'
+ classpath = sourceSets.report.runtimeClasspath
+ mainClass = 'org.apache.grails.benchmarks.report.JmhCompare'
+}
+
+// The jmh source set compiles and runs against everything main depends on.
+configurations.jmh.extendsFrom(configurations.implementation)
+
+// Every JMH knob the CI workflow needs to override is exposed as a Gradle property so that
+// .github/workflows/benchmark.yml can run a fast PR profile and a slower scheduled profile
+// from the same build. Defaults here are the quick local-development profile.
+jmh {
+ jmhVersion = '1.37'
+ includes = providers.gradleProperty('jmh.include')
+ .map { it.split(',') as List<String> }
+ .orElse(['.*'])
+ .get()
+ fork = providers.gradleProperty('jmh.forks').map { it as Integer }.orElse(1).get()
+ warmupIterations = providers.gradleProperty('jmh.warmupIterations').map { it as Integer }.orElse(1).get()
+ iterations = providers.gradleProperty('jmh.iterations').map { it as Integer }.orElse(1).get()
+ warmup = providers.gradleProperty('jmh.warmupTime').orElse('1s').get()
+ timeOnIteration = providers.gradleProperty('jmh.iterationTime').orElse('1s').get()
+ resultFormat = providers.gradleProperty('jmh.resultFormat').orElse('JSON').get()
+ // A benchmark that throws during @Setup is otherwise dropped from the results file while
+ // the build still reports success, which would silently shrink the comparison set.
+ failOnError = providers.gradleProperty('jmh.failOnError').map { it.toBoolean() }.orElse(true).get()
+ resultsFile = file(providers.gradleProperty('jmh.resultFile')
+ .orElse(layout.buildDirectory.file('results/jmh/results.json').get().asFile.absolutePath)
+ .get())
+ // Comma-separated JMH profilers, e.g. -Pjmh.profilers=gc for allocation reporting.
+ profilers = providers.gradleProperty('jmh.profilers')
+ .map { it.split(',') as List<String> }
+ .orElse([])
+ .get()
+}
+
+def mergeJmhClasspathMetadata = tasks.register('mergeJmhClasspathMetadata') {
+ def mergedMetadataDirectory = layout.buildDirectory.dir('generated/jmh-classpath-metadata')
+ def lineMetadataPrefixes = ['META-INF/services/', 'META-INF/groovy/']
+ def extensionModuleSuffix = 'org.codehaus.groovy.runtime.ExtensionModule'
+ def propertiesMetadataPaths = [
+ 'META-INF/spring.factories',
+ 'META-INF/spring.handlers',
+ 'META-INF/spring.schemas'
+ ]
+
+ inputs.files(configurations.jmhRuntimeClasspath)
+ .withPropertyName('jmhRuntimeClasspath')
+ .withPathSensitivity(PathSensitivity.RELATIVE)
+ outputs.dir(mergedMetadataDirectory)
+ outputs.cacheIf { true }
+
+ doLast {
+ File outputDirectory = mergedMetadataDirectory.get().asFile
+ project.delete(outputDirectory)
+
+ Map<String, Set<String>> lineEntries = new TreeMap<>()
+ Map<String, Map<String, Set<String>>> extensionModuleEntries = new TreeMap<>()
+ Map<String, Map<String, Set<String>>> propertyEntries = new TreeMap<>()
+
+ configurations.jmhRuntimeClasspath.files
+ .findAll { File artifact -> artifact.name.endsWith('.jar') }
+ .sort { File artifact -> artifact.absolutePath }
+ .each { File artifact ->
+ new ZipFile(artifact).withCloseable { ZipFile zipFile ->
+ zipFile.entries().each { entry ->
+ if (entry.directory) {
+ return
+ }
+
+ String path = entry.name
+ if (path.endsWith(extensionModuleSuffix)) {
+ Properties properties = new Properties()
+ zipFile.getInputStream(entry).withCloseable { input ->
+ properties.load(input)
+ }
+ Map<String, Set<String>> entries = extensionModuleEntries.computeIfAbsent(path) {
+ new TreeMap<>()
+ }
+ ['extensionClasses', 'staticExtensionClasses'].each { String key ->
+ Set<String> values = entries.computeIfAbsent(key) { new TreeSet<>() }
+ properties.getProperty(key, '').split(',').each { String value ->
+ if (value) {
+ values.add(value.trim())
+ }
+ }
+ }
+ } else if (lineMetadataPrefixes.any { String prefix -> path.startsWith(prefix) }) {
+ Set<String> lines = lineEntries.computeIfAbsent(path) { new TreeSet<>() }
+ zipFile.getInputStream(entry).withCloseable { input ->
+ input.getText('UTF-8').readLines().each { String line ->
+ if (line) {
+ lines.add(line)
+ }
+ }
+ }
+ } else if (propertiesMetadataPaths.contains(path)) {
+ Properties properties = new Properties()
+ zipFile.getInputStream(entry).withCloseable { input ->
+ properties.load(input)
+ }
+ Map<String, Set<String>> entries = propertyEntries.computeIfAbsent(path) {
+ new TreeMap<>()
+ }
+ properties.stringPropertyNames().each { String key ->
+ Set<String> values = entries.computeIfAbsent(key) { new TreeSet<>() }
+ properties.getProperty(key).split(',').each { String value ->
+ if (value) {
+ values.add(value.trim())
+ }
+ }
+ }
+ }
+ }
+ }
+ }
+
+ lineEntries.each { String path, Set<String> lines ->
+ File outputFile = new File(outputDirectory, path)
+ outputFile.parentFile.mkdirs()
+ outputFile.setText("${lines.join('\n')}\n", 'UTF-8')
+ }
+ extensionModuleEntries.each { String path, Map<String, Set<String>> entries ->
+ File outputFile = new File(outputDirectory, path)
+ outputFile.parentFile.mkdirs()
+ String moduleName = path.startsWith('META-INF/groovy/')
+ ? 'grails-benchmark-groovy-extension-modules'
+ : 'grails-benchmark-service-extension-modules'
+ outputFile.withWriter('UTF-8') { writer ->
+ writer.write("moduleName=${moduleName}\n")
+ writer.write('moduleVersion=1.0\n')
+ entries.each { String key, Set<String> values ->
+ writer.write("${key}=${values.join(',')}\n")
+ }
+ }
+ }
+ propertyEntries.each { String path, Map<String, Set<String>> entries ->
+ File outputFile = new File(outputDirectory, path)
+ outputFile.parentFile.mkdirs()
+ outputFile.withWriter('UTF-8') { writer ->
+ entries.each { String key, Set<String> values ->
+ String escapedKey = key.replace('\\', '\\\\').replace('=', '\\=').replace(':', '\\:')
+ writer.write("${escapedKey}=${values.join(',')}\n")
+ }
+ }
+ }
+ }
+}
+
+tasks.named('jmhJar') {
+ dependsOn mergeJmhClasspathMetadata
+ def mergedMetadataDirectory = layout.buildDirectory.dir('generated/jmh-classpath-metadata')
+ def mergedMetadataPath = mergedMetadataDirectory.get().asFile.absolutePath + File.separator
+ def lineMetadataPrefixes = ['META-INF/services/', 'META-INF/groovy/']
+ def propertiesMetadataPaths = [
+ 'META-INF/spring.factories',
+ 'META-INF/spring.handlers',
+ 'META-INF/spring.schemas'
+ ]
+
+ from(mergedMetadataDirectory)
+ eachFile { details ->
+ boolean mergeable = lineMetadataPrefixes.any { String prefix -> details.path.startsWith(prefix) } ||
+ propertiesMetadataPaths.contains(details.path)
+ if (mergeable && !details.file.absolutePath.startsWith(mergedMetadataPath)) {
+ details.exclude()
+ }
+ }
+ duplicatesStrategy = DuplicatesStrategy.EXCLUDE
+}
+
+tasks.named('check') {
+ // Benchmarks are not otherwise compiled by the standard lifecycle tasks.
+ dependsOn tasks.named('jmhClasses')
Review Comment:
Wiring `jmhClasses` into `check` means every root `./gradlew build`/`check` now compiles the JMH source set and resolves JMH, plus the six `implementation project(...)` dependencies, for a module only exercised by an opt-in workflow. That's a cost on every developer and CI build, which sits a little oddly beside the budget discipline argued for the workflow itself.
The alternative β letting the workflow's `jmhJar` be the only compile gate β risks the benchmarks rotting silently between labelled runs, so I don't think the current choice is wrong. But it deserves to be a recorded decision rather than a bare `dependsOn`; could the comment say why compile-on-every-build was preferred?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "testlens-app[bot] (via GitHub)" <gi...@apache.org>.
testlens-app[bot] commented on PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#issuecomment-5136517468
## β
All tests passed β
π·οΈ Commit: b06d51784c18781c93dda2391d5f72e95aa305ff
βΆοΈ Tests: 45531 executed
βͺοΈ Checks: 62/62 completed
---
_Learn more about TestLens at [testlens.app](https://testlens.app)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add JMH performance regression reporting for pull requests [grails-core]
Posted by "jamesfredley (via GitHub)" <gi...@apache.org>.
jamesfredley commented on PR #16071:
URL: https://github.com/apache/grails-core/pull/16071#issuecomment-5302953995
Merging this so that we can use it on a number of open Grails 8 PRs where we need to evaluate performance.
Adding 'performance' label triggers it.
Can iterate on this later.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@grails.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org