build: add jmh-generator-annprocess to annotationProcessorPaths - #587
build: add jmh-generator-annprocess to annotationProcessorPaths#587iamabhilaksh wants to merge 1 commit into
Conversation
…DK 21+ Without the jmh-generator-annprocess processor on the annotation processor path, META-INF/BenchmarkList is not generated under JDK 21+, so every JMH benchmark suite fails at runtime with 'Unable to find the resource: /META-INF/BenchmarkList'. jmh.version (1.37) and jmh-core are already parent-managed; this adds the missing generator path. Story: https://gus.lightning.force.com/lightning/r/ADM_Work__c/a07EE00002hP986YAC/view (W-23830175)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #587 +/- ##
=========================================
Coverage 83.61% 83.61%
Complexity 674 674
=========================================
Files 215 215
Lines 15010 15010
Branches 2076 2076
=========================================
Hits 12550 12550
Misses 1636 1636
Partials 824 824
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| <version>${auto-service.version}</version> | ||
| </path> | ||
| <path> | ||
| <groupId>org.openjdk.jmh</groupId> |
There was a problem hiding this comment.
I built both sides rather than reasoning about it. On current main, a full-reactor mvn -T 1C clean test-compile produces zero META-INF/BenchmarkList files. With this patch applied the same build produces six — 60 entries each for blob-aws/blob-gcp, 52 for pubsub-aws, 28 for the docstore providers, and so on. So this line isn't forward-compat hardening: it repairs benchmark suites that are already merged and currently cannot run at all.
That matters because the recorded reason points somewhere else. The title says "JDK 21+" and the comment two lines above says "JDK 23+ no longer runs annotation processors discovered only on the classpath by default". Neither is what's happening here. Once annotationProcessorPaths is specified at all, Maven passes --processor-path and javac stops scanning the compile/test classpath entirely, on every JDK. Controlled javac matrix on a one-method @Benchmark class:
| JDK | processor discovery | BenchmarkList |
|---|---|---|
| 11 | classpath only | generated |
| 11 | --processor-path lombok |
not generated |
| 21 | classpath only | generated |
| 21 | --processor-path lombok |
not generated |
Since the project targets <releaseTarget>17</releaseTarget>, the JDK framing would leave the next reader thinking this only matters on newer toolchains and is safe to drop. Suggested rationale instead: once annotationProcessorPaths is configured, javac uses --processor-path and never scans the compile/test classpath, so every processor the build relies on must be listed here regardless of JDK version.
Second, the Merge order section says "No hard dependency", while the Summary a few paragraphs above says that without this change "every JMH benchmark suite fails at runtime with Unable to find the resource: /META-INF/BenchmarkList". Both can't hold. I reproduced the failure on #588's branch with the exact command from its own body — it fails in 34 ms, before any network call, so it isn't credential-dependent:
java.lang.RuntimeException: ERROR: Unable to find the resource: /META-INF/BenchmarkList
at org.openjdk.jmh.runner.BenchmarkList.find(BenchmarkList.java:124)
#588–#591 each do say "Recommend the root-pom JMH fix merges first", so it's only this line that needs correcting. Worth correcting because CI cannot catch the ordering: the suites compile fine without this, and nothing in .github/workflows/ sets -DrunBenchmarks=true, so all six PRs go green either way.
For what it's worth I also checked the two things that could have made this risky, and both are clean: auto-service still emits all 27 META-INF/services files byte-identically (ServiceLoader provider discovery is unaffected), and full-reactor build time is unchanged within noise.
There was a problem hiding this comment.
Thanks for digging in on this — you're right on both counts and I've corrected the PR.
On merge order: agreed, "No hard dependency" was flat wrong given the Summary says the suites can't run without it. It's a hard runtime dependency (they compile fine, just can't run — your BenchmarkList.find repro shows it failing before any network call). Fixed in the body, and dropped the "JDK 21+" from the title since — as you say — once annotationProcessorPaths is configured, javac is on --processor-path and skips classpath scanning regardless of JDK.
I ran the javac matrix to be sure, and it lines up with your point with one extra wrinkle worth noting for the record:
- classpath-only, no
annotationProcessorPaths: JDK 11 generates, JDK 25 does not annotationProcessorPathsset without the JMH generator (this repo's pre-PR state): neither JDK 11 nor JDK 25 generatesannotationProcessorPathsset with the JMH generator (this PR): both generate
So the classpath-discovery cutoff is real but lands at JDK 23+, not 21 — the reason it bites us on every JDK is exactly your point: we already set annotationProcessorPaths, so classpath scanning is already off. The pre-existing pom comment framing it as a JDK-23 thing is a bit narrow, but since it predates this PR and isn't strictly false, I'll leave it out of scope here rather than expand the diff — happy to do a follow-up if you think it's worth clarifying.
Also — thanks for confirming auto-service's META-INF/services output is byte-identical. That was my main worry too.
Summary
Adds
jmh-generator-annprocessto the root pom's<annotationProcessorPaths>.This repo already configures
<annotationProcessorPaths>(lombok, auto-service),which puts javac on
--processor-pathand stops it from discovering processors onthe compile/test classpath. As a result the JMH generator is never picked up and
META-INF/BenchmarkListis not generated, so every JMH benchmark suite fails atruntime with
Unable to find the resource: /META-INF/BenchmarkList.jmh.version(1.37) and
jmh-coreare already parent-managed; this only adds the missinggenerator path.
Scope
Affects local benchmark runs and reactor builds. Downstream consumers that build
through Bazel (not this root pom) are unaffected, so this does not change their
build behavior.
Merge order
Hard runtime dependency for the benchmark suites: they compile without this change
but cannot run (the BenchmarkList is missing). Recommend merging this first so the
per-module benchmark suites are runnable locally and in CI.
Testing
mvn install -DskipTestsclean on the full reactor.jmh-generator-annprocess:1.37resolves from the parent-managed version.@Benchmarkclass: withannotationProcessorPathsconfigured,
BenchmarkListis generated only when the JMH generator is on theprocessor path — confirmed on JDK 11 and JDK 25.