jdaugherty commented on code in PR #16169: URL: https://github.com/apache/grails-core/pull/16169#discussion_r4264632704
########## grails-testing-support-http-client/src/testFixtures/groovy/org/apache/grails/testing/http/client/bench/AppHttpBench.groovy: ########## @@ -0,0 +1,316 @@ +/* + * 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.testing.http.client.bench + +import java.nio.charset.StandardCharsets +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.Paths + +import groovy.json.JsonOutput +import groovy.json.JsonException +import groovy.transform.CompileStatic + +/** + * App-level HTTP microbench helper that emits JMH-compatible JSON so results can be compared + * with {@code :grails-test-examples-benchmarks:jmhCompare} using the same methodology as the framework JMH suite. + * + * <p>This type lives in test fixtures, not the published main API of grails-testing-support-http-client. + * + * <p>Measurement model: + * <ul> + * <li>warm up the full Spring Boot stack with {@code warmup} requests (discarded)</li> + * <li>collect {@code samples} timed requests as one raw series</li> + * <li>split the series into {@code forks} equal chunks and report a t-interval over fork means</li> + * <li>also emit a ruler-equivalent tight loop so runner-health gates apply</li> + * </ul> + * + * <p>Enable gated specs with {@code -PappBench=true}. Optional properties: + * {@code appBenchWarmup}, {@code appBenchSamples}, {@code appBenchForks}, + * {@code appBenchOut}, {@code appBenchOutDir}. + */ +@CompileStatic +final class AppHttpBench { + + static final String RULER_BENCHMARK = 'org.apache.grails.benchmarks.ruler.AppBenchCpu.measure' + + private AppHttpBench() { + } + + static boolean enabled() { + Boolean.getBoolean('app.bench') || Boolean.parseBoolean(System.getProperty('appBench', 'false')) + } + + static int warmupCount() { + intProperty('app.bench.warmup', 'appBenchWarmup', 200, true) + } + + static int sampleCount() { + intProperty('app.bench.samples', 'appBenchSamples', 1000, false) + } + + static int forkCount() { + intProperty('app.bench.forks', 'appBenchForks', 2, false) + } + + static Path outputPath(String defaultFileName) { + String configured = System.getProperty('app.bench.out', System.getProperty('appBenchOut', '')) + if (configured) { + return Paths.get(configured) + } + String directory = System.getProperty('app.bench.out.dir', System.getProperty('appBenchOutDir', '')) + Path dir = directory ? Paths.get(directory) : Paths.get('build', 'app-bench') + Files.createDirectories(dir) + return dir.resolve(defaultFileName) + } + + /** + * Time a single request body. The closure must perform the HTTP call and assert success. + * + * @return elapsed nanoseconds + */ + static long timeNanos(Closure<?> request) { + long start = System.nanoTime() + request.call() + return System.nanoTime() - start + } + + /** + * Warm up, sample, and replace {@code out} with a JMH-shaped array containing the HTTP + * benchmark and a ruler-equivalent entry. + * + * @param benchmark fully-qualified-style name, e.g. {@code appbench.latency.FastPing.httpGet} + * @param request closure that performs one successful request + */ + static void measureAndWrite(String benchmark, Path out, Closure<?> request) { + int warmup = Math.max(0, warmupCount()) + int samples = sampleCount() + if (samples < 1) { + throw new IllegalArgumentException("app.bench.samples must be >= 1, was ${samples}") + } + + for (int i = 0; i < warmup; i++) { + request.call() + } + + double[] values = new double[samples] + for (int i = 0; i < samples; i++) { + values[i] = (double) timeNanos(request) + } + + int forks = forkCount() + writeEntries(out, [ + toJmhEntry(benchmark, values, forks), + toJmhEntry(RULER_BENCHMARK, rulerSamples(samples, warmup), forks) + ]) + } + + static Map<String, Object> toJmhEntry(String benchmark, double[] values, int forks) { + if (values == null || values.length == 0) { + throw new IllegalArgumentException('values must contain at least one sample') + } + + int forkCount = Math.min(Math.max(1, forks), values.length) + int perFork = Math.max(1, values.length.intdiv(forkCount)) + List<List<Double>> rawData = new ArrayList<>(forkCount) + int offset = 0 + for (int f = 0; f < forkCount; f++) { + int end = (f == forkCount - 1) ? values.length : Math.min(values.length, offset + perFork) + List<Double> chunk = new ArrayList<>(Math.max(0, end - offset)) + for (int i = offset; i < end; i++) { + chunk.add(values[i]) + } + rawData.add(chunk) + offset = end + } + + double[] forkMeans = new double[rawData.size()] + for (int f = 0; f < rawData.size(); f++) { + forkMeans[f] = mean(rawData.get(f)) + } + double score = mean(forkMeans) + double scoreError = tIntervalError(forkMeans) Review Comment: Done in 37d722d079. The knob is now `appBenchChunks` (`app.bench.chunks`, `--chunks`) and defaults to 10, in CI too. `AppHttpBenchSpec` pins the 10-chunk interval (nine degrees of freedom, 2.262 critical value) next to the existing 2-chunk case. ########## grails-test-examples/benchmarks/src/reportTest/groovy/org/apache/grails/benchmarks/report/AppIndyBenchSpec.groovy: ########## @@ -0,0 +1,275 @@ +/* + * 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 java.nio.charset.StandardCharsets +import java.nio.file.Files +import java.nio.file.Path + +class AppIndyBenchSpec extends Specification { + + @TempDir + Path temporaryDirectory + + void 'parse requires project-dir and applies defaults'() { + when: + AppIndyBench.Options options = AppIndyBench.parse(['--project-dir', temporaryDirectory.toString()] as String[]) + + then: + options.projectDir == temporaryDirectory.toAbsolutePath().normalize() + options.outputDir == options.projectDir.resolve('build').resolve('app-bench') + options.warmup == 200 + options.samples == 1000 + options.forks == 2 + options.maxWorkers == 4 + } + + void 'parse rejects a missing project-dir'() { + when: + AppIndyBench.parse(['--warmup', '10'] as String[]) + + then: + IllegalArgumentException error = thrown() + error.message.contains('--project-dir is required') + } + + void 'run alternates indy mode per app then compares directory results'() { + given: + Path outputDir = temporaryDirectory.resolve('out') + Path runDir = outputDir.resolve(AppIndyBench.OUTPUT_DIRECTORY_NAME) + Path summary = temporaryDirectory.resolve('summary.md') + List<List<String>> invocations = [] + Files.createDirectories(outputDir.resolve('noindy')) + Files.writeString(outputDir.resolve('noindy').resolve('caller-owned.json'), '[]', StandardCharsets.UTF_8) + Files.writeString(outputDir.resolve('indy-vs-noindy.md'), 'stale report', StandardCharsets.UTF_8) + AppIndyBench.GradleRunner runner = { Path projectDir, List<String> args -> + invocations.add(args) + writeDummyResult(args) + } as AppIndyBench.GradleRunner + + when: + int exit = AppIndyBench.run( + [ + '--project-dir', temporaryDirectory.toString(), + '--output-dir', outputDir.toString(), + '--warmup', '80', + '--samples', '300', + '--forks', '2', + '--max-workers', '3' + ] as String[], + runner, + new GitHubComments(), + [GITHUB_STEP_SUMMARY: summary.toString()] + ) + + then: + exit == 0 + invocations.size() == 6 + invocations[0].contains(':grails-test-examples-latency:integrationTest') + invocations[0].contains('latencyapp.AppBenchFastPingSpec') + invocations[0].contains('-PgrailsIndy=false') + invocations[0].contains('-PappBench=true') + invocations[0].contains('-PappBenchWarmup=80') + invocations[0].contains('-PappBenchSamples=300') + invocations[0].contains('-PappBenchForks=2') + invocations[0].contains('--no-daemon') + !invocations[0].contains('--rerun-tasks') + invocations[0].contains('--max-workers=3') + invocations[0].contains('--project-cache-dir') + invocations[0].any { String argument -> Path.of(argument).endsWith(Path.of(AppIndyBench.OUTPUT_DIRECTORY_NAME, 'project-cache', 'noindy', 'latency')) } + invocations[1].contains('-PgrailsIndy=true') + invocations[1].contains(':grails-test-examples-latency:integrationTest') + invocations[2].contains(':grails-test-examples-app1:integrationTest') + invocations[2].contains('functionaltests.AppBenchInterceptorDemoSpec') + invocations[2].contains('-PgrailsIndy=true') + invocations[3].contains('-PgrailsIndy=false') + invocations[3].contains(':grails-test-examples-app1:integrationTest') + invocations[4].contains('-PgrailsIndy=false') + invocations[4].contains(':grails-test-examples-gsp-layout:integrationTest') + invocations[5].contains('-PgrailsIndy=true') + invocations[5].contains(':grails-test-examples-gsp-layout:integrationTest') + invocations.each { List<String> arguments -> + boolean indy = arguments.contains('-PgrailsIndy=true') + String outputArgument = arguments.find { String argument -> argument.startsWith('-PappBenchOut=') } + Path result = Path.of(outputArgument.substring('-PappBenchOut='.length())) + result.parent == runDir.resolve(indy ? 'indy' : 'noindy') Review Comment: Done in 37d722d079: both comparisons are `assert`s now. Re-running with the `bogus-never-exists` mutation fails the spec. ########## grails-testing-support-http-client/src/testFixtures/groovy/org/apache/grails/testing/http/client/bench/AppHttpBench.groovy: ########## @@ -0,0 +1,316 @@ +/* + * 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.testing.http.client.bench + +import java.nio.charset.StandardCharsets +import java.nio.file.Files +import java.nio.file.Path +import java.nio.file.Paths + +import groovy.json.JsonOutput +import groovy.json.JsonException +import groovy.transform.CompileStatic + +/** + * App-level HTTP microbench helper that emits JMH-compatible JSON so results can be compared + * with {@code :grails-test-examples-benchmarks:jmhCompare} using the same methodology as the framework JMH suite. + * + * <p>This type lives in test fixtures, not the published main API of grails-testing-support-http-client. + * + * <p>Measurement model: + * <ul> + * <li>warm up the full Spring Boot stack with {@code warmup} requests (discarded)</li> + * <li>collect {@code samples} timed requests as one raw series</li> + * <li>split the series into {@code forks} equal chunks and report a t-interval over fork means</li> + * <li>also emit a ruler-equivalent tight loop so runner-health gates apply</li> + * </ul> + * + * <p>Enable gated specs with {@code -PappBench=true}. Optional properties: + * {@code appBenchWarmup}, {@code appBenchSamples}, {@code appBenchForks}, + * {@code appBenchOut}, {@code appBenchOutDir}. + */ +@CompileStatic +final class AppHttpBench { + + static final String RULER_BENCHMARK = 'org.apache.grails.benchmarks.ruler.AppBenchCpu.measure' + + private AppHttpBench() { + } + + static boolean enabled() { + Boolean.getBoolean('app.bench') || Boolean.parseBoolean(System.getProperty('appBench', 'false')) + } + + static int warmupCount() { + intProperty('app.bench.warmup', 'appBenchWarmup', 200, true) + } + + static int sampleCount() { + intProperty('app.bench.samples', 'appBenchSamples', 1000, false) + } + + static int forkCount() { + intProperty('app.bench.forks', 'appBenchForks', 2, false) + } + + static Path outputPath(String defaultFileName) { + String configured = System.getProperty('app.bench.out', System.getProperty('appBenchOut', '')) + if (configured) { + return Paths.get(configured) + } + String directory = System.getProperty('app.bench.out.dir', System.getProperty('appBenchOutDir', '')) + Path dir = directory ? Paths.get(directory) : Paths.get('build', 'app-bench') + Files.createDirectories(dir) + return dir.resolve(defaultFileName) + } + + /** + * Time a single request body. The closure must perform the HTTP call and assert success. + * + * @return elapsed nanoseconds + */ + static long timeNanos(Closure<?> request) { + long start = System.nanoTime() + request.call() + return System.nanoTime() - start + } + + /** + * Warm up, sample, and replace {@code out} with a JMH-shaped array containing the HTTP + * benchmark and a ruler-equivalent entry. + * + * @param benchmark fully-qualified-style name, e.g. {@code appbench.latency.FastPing.httpGet} + * @param request closure that performs one successful request + */ + static void measureAndWrite(String benchmark, Path out, Closure<?> request) { + int warmup = Math.max(0, warmupCount()) + int samples = sampleCount() + if (samples < 1) { + throw new IllegalArgumentException("app.bench.samples must be >= 1, was ${samples}") + } + + for (int i = 0; i < warmup; i++) { + request.call() + } + + double[] values = new double[samples] + for (int i = 0; i < samples; i++) { + values[i] = (double) timeNanos(request) + } + + int forks = forkCount() + writeEntries(out, [ + toJmhEntry(benchmark, values, forks), + toJmhEntry(RULER_BENCHMARK, rulerSamples(samples, warmup), forks) + ]) + } + + static Map<String, Object> toJmhEntry(String benchmark, double[] values, int forks) { + if (values == null || values.length == 0) { + throw new IllegalArgumentException('values must contain at least one sample') + } + + int forkCount = Math.min(Math.max(1, forks), values.length) + int perFork = Math.max(1, values.length.intdiv(forkCount)) + List<List<Double>> rawData = new ArrayList<>(forkCount) + int offset = 0 + for (int f = 0; f < forkCount; f++) { + int end = (f == forkCount - 1) ? values.length : Math.min(values.length, offset + perFork) + List<Double> chunk = new ArrayList<>(Math.max(0, end - offset)) + for (int i = offset; i < end; i++) { + chunk.add(values[i]) + } + rawData.add(chunk) + offset = end + } + + double[] forkMeans = new double[rawData.size()] + for (int f = 0; f < rawData.size(); f++) { + forkMeans[f] = mean(rawData.get(f)) + } + double score = mean(forkMeans) + double scoreError = tIntervalError(forkMeans) + + Map<String, Object> percentiles = new LinkedHashMap<>() + double[] sorted = Arrays.copyOf(values, values.length) + Arrays.sort(sorted) + percentiles.put('0.0', sorted[0]) + percentiles.put('50.0', percentile(sorted, 0.50d)) + percentiles.put('90.0', percentile(sorted, 0.90d)) + percentiles.put('95.0', percentile(sorted, 0.95d)) + percentiles.put('99.0', percentile(sorted, 0.99d)) + percentiles.put('100.0', sorted[sorted.length - 1]) + + Map<String, Object> primary = new LinkedHashMap<>() + primary.put('score', score) + primary.put('scoreError', scoreError) + primary.put('scoreConfidence', [score - scoreError, score + scoreError]) + primary.put('scorePercentiles', percentiles) + primary.put('scoreUnit', 'ns/op') + primary.put('rawData', rawData) + + Map<String, Object> entry = new LinkedHashMap<>() + entry.put('jmhVersion', 'app-bench-1.0') + entry.put('benchmark', benchmark) + entry.put('mode', 'avgt') + entry.put('threads', 1) + entry.put('forks', forkCount) + entry.put('jdkVersion', System.getProperty('java.version', 'unknown')) + entry.put('vmName', System.getProperty('java.vm.name', 'unknown')) + entry.put('vmVersion', System.getProperty('java.vm.version', 'unknown')) + entry.put('warmupIterations', 1) + entry.put('warmupTime', "${warmupCount()} reqs") + entry.put('measurementIterations', values.length) + entry.put('measurementTime', '1 req') + entry.put('primaryMetric', primary) + entry.put('secondaryMetrics', Collections.emptyMap()) + return entry + } + + static void writeEntries(Path out, List<Map<String, Object>> entries) { + if (Files.exists(out)) { + String existing = Files.readString(out, StandardCharsets.UTF_8).trim() + if (!existing) { + throw new IllegalStateException("Truncated or invalid bench output: ${out}") + } + Object parsed + try { + parsed = new groovy.json.JsonSlurper().parseText(existing) + } catch (JsonException error) { + throw new IllegalStateException("Truncated or invalid bench output: ${out}", error) + } + if (!(parsed instanceof List)) { + throw new IllegalStateException("Refusing to overwrite non-JSON-array bench output: ${out}") + } + } + Path parent = out.getParent() + if (parent != null) { + Files.createDirectories(parent) + } + Files.writeString(out, JsonOutput.prettyPrint(JsonOutput.toJson(entries)), StandardCharsets.UTF_8) + } + + static int intProperty(String primary, String fallback, int defaultValue, boolean allowZero) { + String raw = System.getProperty(primary) + String name = primary + if (raw == null) { + raw = System.getProperty(fallback) + name = fallback + } + if (raw == null) { + return defaultValue + } + return parseDecimalInt(raw, name, allowZero) + } + + private static int parseDecimalInt(String raw, String name, boolean allowZero) { + int value + try { + value = Integer.parseInt(raw) + } catch (NumberFormatException ignored) { + throw new IllegalArgumentException("${name} must be a decimal integer, was '${raw}'") + } + if (value < 0 || (!allowZero && value < 1)) { + throw new IllegalArgumentException("${name} must be ${allowZero ? '>= 0' : '>= 1'}, was ${value}") + } + return value + } + + private static double[] rulerSamples(int samples, int warmup) { + for (int i = 0; i < warmup; i++) { + rulerNanos() + } + double[] values = new double[samples] + for (int i = 0; i < samples; i++) { + values[i] = (double) rulerNanos() + } + return values + } + + private static long rulerNanos() { + long start = System.nanoTime() + long acc = 0L + for (int i = 0; i < 64; i++) { Review Comment: Done in 37d722d079. The ruler now runs the `CpuRulerBenchmark.integerArithmetic` recurrence, 1,024 rounds with the state carried in a static field, and the `measureAndWrite` spec checks that no ruler sample is zero. ########## grails-test-examples/benchmarks/build.gradle: ########## @@ -131,6 +131,27 @@ tasks.register('jmhCompare', JavaExec) { mainClass = 'org.apache.grails.benchmarks.report.JmhCompare' } +tasks.register('appIndyBench', JavaExec) { + group = 'verification' + description = 'Runs app-level HTTP benches for grailsIndy=false then true and compares JMH-shaped JSON.' + classpath = sourceSets.report.runtimeClasspath + mainClass = 'org.apache.grails.benchmarks.report.AppIndyBench' + def warmup = providers.gradleProperty('appBenchWarmup').orElse('200') + def samples = providers.gradleProperty('appBenchSamples').orElse('1000') + def forks = providers.gradleProperty('appBenchForks').orElse('2') + def maxWorkers = providers.gradleProperty('appBenchMaxWorkers').orElse('4') + def outputDir = providers.gradleProperty('appBenchOutDir') + .orElse(rootProject.layout.projectDirectory.dir('build/app-bench').asFile.absolutePath) Review Comment: Done in 37d722d079. `appBenchOutDir` is resolved through `rootProject.layout.projectDirectory.dir(...)`, so a relative value lands under the repository root like the default does, and the README row now says the results go to `run/`. ########## grails-test-examples/benchmarks/src/report/groovy/org/apache/grails/benchmarks/report/AppIndyBench.groovy: ########## @@ -0,0 +1,337 @@ +/* + * 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 +import java.nio.file.StandardOpenOption +import java.util.concurrent.TimeUnit + +@CompileStatic +class AppIndyBench { + + static final String OUTPUT_DIRECTORY_NAME = 'run' + + static final List<App> APPS = [ + new App('latency', ':grails-test-examples-latency:integrationTest', 'latencyapp.AppBenchFastPingSpec'), + new App('app1', ':grails-test-examples-app1:integrationTest', 'functionaltests.AppBenchInterceptorDemoSpec'), + new App('gsp-layout', ':grails-test-examples-gsp-layout:integrationTest', 'org.example.grails.layout.AppBenchDemoRenderTextSpec') + ].asImmutable() + + static void main(String[] args) { + int exit = run(args, new WrapperGradleRunner()) + if (exit != 0) { + System.exit(exit) + } + } + + static int run(String[] args, GradleRunner runner) { + return run(args, runner, new GitHubComments(), System.getenv()) + } + + static int run(String[] args, GradleRunner runner, CommentPoster poster, Map<String, String> environment) { + try { + Options options = parse(args) + Path runDir = recreateOwnedDirectory(options.outputDir) + Path noindyDir = Files.createDirectories(runDir.resolve('noindy')) + Path indyDir = Files.createDirectories(runDir.resolve('indy')) + Path report = runDir.resolve('indy-vs-noindy.md') + boolean nestedFailed = false + + APPS.eachWithIndex { App app, int index -> + List<String> modes = index % 2 == 0 ? ['false', 'true'] : ['true', 'false'] + modes.each { String indy -> + Path modeDir = indy == 'true' ? indyDir : noindyDir + Path out = modeDir.resolve(app.name + '.json') + try { + runner.run(options.projectDir, gradleArgs(options, app, indy, out)) + } catch (Exception error) { + nestedFailed = true + System.err.println("Nested Gradle failed for ${app.name} with grailsIndy=${indy}: ${error.message}") + error.printStackTrace(System.err) + } + if (!Files.isRegularFile(out)) { + nestedFailed = true + System.err.println(missingResultMessage(out)) + } + } + } + + int compareExit = JmhCompare.run( + ['--base', noindyDir.toString(), '--head', indyDir.toString(), '--output', report.toString()] as String[], + poster, + environment + ) + if (compareExit != 0 || nestedFailed) { + appendFallbackStepSummary(environment, 'JmhCompare could not produce an app indy benchmark comparison. Nested result files were retained as artifacts for diagnosis.') + } else { + appendStepSummary(report, environment) Review Comment: Done in 37d722d079. The success path writes the same `## App indy benchmark comparison` heading ahead of the JmhCompare report; both paths share `STEP_SUMMARY_HEADING` and the spec checks the heading on each. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
