7903596: JMH: Explicitly enable annotation processors #125

jaikiran · 2023-11-30T06:43:21Z

Can I please get a review of this change which proposes to address the issue noted in https://bugs.openjdk.org/browse/CODETOOLS-7903596?

As noted in that issue, when a benchmark generated using Java 22 is run, at runtime it fails with:

java -jar target/benchmarks.jar
Exception in thread "main" java.lang.RuntimeException: ERROR: Unable to find the resource: /META-INF/BenchmarkList
	at org.openjdk.jmh.runner.AbstractResourceReader.getReaders(AbstractResourceReader.java:98)
	at org.openjdk.jmh.runner.BenchmarkList.find(BenchmarkList.java:124)
	at org.openjdk.jmh.runner.Runner.internalRun(Runner.java:252)
	at org.openjdk.jmh.runner.Runner.run(Runner.java:208)
	at org.openjdk.jmh.Main.main(Main.java:71)

This is due to annotation processors being disabled by default during compilation starting Java 22. The commit in this PR, explicitly enables the org.openjdk.jmh.generators.BenchmarkProcessor annotation processor to allow for it to generate the necessary benchmark resources.

I've tested this change locally, with Java 8, Java 21 as well as Java 22 and the newly generated benchmark is now functional on all these versions.

Progress

Change must not contain extraneous whitespace
Change must be properly reviewed (1 review required, with at least 1 Committer)

Issue

CODETOOLS-7903596: JMH: Explicitly enable annotation processors (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jmh.git pull/125/head:pull/125
$ git checkout pull/125

Update a local copy of the PR:
$ git checkout pull/125
$ git pull https://git.openjdk.org/jmh.git pull/125/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 125

View PR using the GUI difftool:
$ git pr show -t 125

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jmh/pull/125.diff

Using Webrev

Link to Webrev Comment

…ntime - ERROR: Unable to find the resource: /META-INF/BenchmarkList

bridgekeeper · 2023-11-30T06:44:06Z

👋 Welcome back jpai! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

mlbridge · 2023-11-30T06:49:03Z

Webrevs

01: Full - Incremental (29c7950c)
00: Full (ac965456)

shipilev · 2023-11-30T13:30:12Z

There are tests in JMH that test archetypes work, please run them. The easiest way is do this is enabling JMH GHA:
https://github.com/jaikiran/jmh/actions -- and triggering the run on your branch.

jaikiran · 2023-11-30T16:14:29Z

Hello Aleksey, thank you for that info. I've enabled the actions and the job is in progress. There are some Windows failures, but those don't appear related:

Tests run: 1, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 177.957 sec <<< FAILURE! - in org.openjdk.jmh.it.profilers.WinPerfAsmProfilerTest
test(org.openjdk.jmh.it.profilers.WinPerfAsmProfilerTest)  Time elapsed: 177.832 sec  <<< ERROR!
java.lang.IllegalStateException: Profile does not contain the required frame: PrintAssembly processed: 7 total address lines.

There's a macos run going on in that run for a while now, I'll check it out tomorrow on how it goes.

jaikiran · 2023-12-01T12:57:36Z

I had a look at the Windows failures reported in GitHub actions job - they all appear to be:

test(org.openjdk.jmh.it.profilers.WinPerfAsmProfilerTest)  Time elapsed: 185.743 sec  <<< ERROR!
java.lang.IllegalStateException: Profile does not contain the required frame: PrintAssembly processed: 7 total address lines.

I'm unfamiliar with those tests, but the error doesn't suggest it's due to the changes in this PR. Would you happen to know, Aleksey?

bridgekeeper · 2023-12-29T17:20:12Z

@jaikiran This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

jaikiran · 2023-12-30T01:41:58Z

Please keep open, hoping for a review.

bridgekeeper · 2024-01-27T04:00:27Z

@jaikiran This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

bridgekeeper · 2024-02-24T06:07:38Z

@jaikiran This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

jaikiran · 2024-10-08T07:25:14Z

/open

openjdk · 2024-10-08T07:25:53Z

@jaikiran This pull request is now open

openjdk · 2024-10-08T07:25:55Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

jaikiran · 2024-10-08T07:29:37Z

Re-opening this PR since the issue is still applicable.

Without the changes in this PR, generating a project using the archetype:

mvn archetype:generate \       
  -DinteractiveMode=false \
  -DarchetypeGroupId=org.openjdk.jmh \
  -DarchetypeArtifactId=jmh-java-benchmark-archetype -DarchetypeVersion=1.38-SNAPSHOT\
  -DgroupId=org.sample \
  -DartifactId=test \
  -Dversion=1.0

Then running the project:

cd test
mvn clean install
$JDK23/bin/java -jar target/benchmarks.jar

results in:

Exception in thread "main" java.lang.RuntimeException: ERROR: Unable to find the resource: /META-INF/BenchmarkList
	at org.openjdk.jmh.runner.AbstractResourceReader.getReaders(AbstractResourceReader.java:98)
	at org.openjdk.jmh.runner.BenchmarkList.find(BenchmarkList.java:124)
	at org.openjdk.jmh.runner.Runner.internalRun(Runner.java:252)
	at org.openjdk.jmh.runner.Runner.run(Runner.java:208)
	at org.openjdk.jmh.Main.main(Main.java:71)

With the changes in this PR, the newly generated projects from the archetype can successfully launch the benchmarks even against Java 23.

sigpwned · 2024-11-04T03:30:16Z

I just got bit by this issue and lost a couple of hours. Having this archetype up-to-date would have helped. Instead, my developer experience looked like this:

Try to add JMH benchmarks to a new project
Consistently get above error about /META-INF/BenchmarkList
Try to run existing JMH benchmarks on existing projects
Those fail with /META-INF/BenchmarkList
Generate a fresh project from the archetype
Try to run JMH benchmarks against the archetype
Those fail with /META-INF/BenchmarkList
Bang head against wall
Check JMH PRs and find this issue
Retry builds with Java 17, and everything works

First, I could have saved some time if this archetype had been up-to-date, since it would have given me the thread of the relevant difference in the POM to pull on. Second, that I'm only discovering this through a PR I had to track down semi-manually means that there are probably a lot of folks who are having this issue but don't find the cause.

In my opinion, for whatever that's worth, I think merging this PR would be very helpful, since it would at least put this PR in the main branch to help people find it. I also think adding a "gotcha" to the README would help, too.

galderz · 2024-11-05T08:39:03Z

Just bumped into this.

I also think adding a "gotcha" to the README would help, too.

+1

jaikiran · 2024-11-05T09:42:48Z

Any reviewers who can review this one, please?

shipilev

This looks okay, but shouldn't the fix also include adding -processor all around the JMH modules, like jmh-samples, jmh-core-it and such? Should probably be enough to add it to jmh-parent here:

jmh/pom.xml

Lines 175 to 179 in 7c6da95

    
           <configuration> 
        
               <compilerVersion>1.8</compilerVersion> 
        
               <source>1.8</source> 
        
               <target>1.8</target> 
        
           </configuration>

Please rebase from master and test full JMH build with JDK 23.

sigpwned · 2024-11-07T21:46:57Z

I'm having a look at this. I've rebased and have started compiling on JDK 23. It turns out there's a bit more to it than updating the root POM. Will update with progress here in a bit.

EDIT: Here is what I've tried and what I've learned, since I'm not sure the best way to PR a PR in GitHub, and just in case I don't end up having the time to run this to ground.

I'm running builds on ubuntu24.04. I can confirm that build runs happily on JDK 17 and 21. I can also confirm that the build does not run happily on JDK 23. Here is what I've learned regarding builds on JDK 23.

I'm merely commenting on what works below. I'm very, very open to the idea -- indeed, I suspect -- that there are better ways.

Edits to pom.xml

A build on master fails in jmh-core with many, many errors like ERROR: Unable to find the resource: /META-INF/BenchmarkList.

The following patch addresses these issue:

--- a/pom.xml
+++ b/pom.xml
@@ -176,6 +176,9 @@ questions.
                         <compilerVersion>1.8</compilerVersion>
                         <source>1.8</source>
                         <target>1.8</target>
+                        <annotationProcessors>
+                          <annotationProcessor>org.openjdk.jmh.generators.BenchmarkProcessor</annotationProcessor>
+                        </annotationProcessors>
                     </configuration>
                 </plugin>
                 <plugin>

Edits to jmh-core-it/pom.xml

The build now fails in jmh-core-it with one error about CustomBenchmark.

The following patch addresses this issue:

--- a/jmh-core-it/pom.xml
+++ b/jmh-core-it/pom.xml
@@ -64,6 +64,24 @@ questions.
 
     <build>
         <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-compiler-plugin</artifactId>
+                <executions>
+                    <!-- Configuration for the test compile phase -->
+                    <execution>
+                        <id>test-compile</id>
+                        <goals>
+                            <goal>testCompile</goal>
+                        </goals>
+                        <configuration>
+                            <annotationProcessors>
+                                <annotationProcessor>org.openjdk.jmh.it.annsteal.CustomBenchmarkProcessor</annotationProcessor>
+                            </annotationProcessors>
+                        </configuration>
+                    </execution>
+                </executions>
+            </plugin>
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-surefire-plugin</artifactId>

Edits to jmh-core-ct/pom.xml

The build now fails in jmh-core-ct with 98 test failures like the following:

Running org.openjdk.jmh.ct.other.GenericParamTest
Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 0.064 sec <<< FAILURE! - in org.openjdk.jmh.ct.other.GenericParamTest
compileTest(org.openjdk.jmh.ct.other.GenericParamTest)  Time elapsed: 0.063 sec  <<< FAILURE!
java.lang.AssertionError: Should have failed.
	at org.junit.Assert.fail(Assert.java:89)
	at org.openjdk.jmh.ct.CompileTest.assertFail(CompileTest.java:66)
	at org.openjdk.jmh.ct.other.GenericParamTest.compileTest(GenericParamTest.java:39)
	at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:103)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
	at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
	at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
	at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
	at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:264)
	at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:153)
	at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:124)
	at org.apache.maven.surefire.booter.ForkedBooter.invokeProviderInSameClassLoader(ForkedBooter.java:200)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:153)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:103)

The following patch DOES NOT address these issues:

--- a/jmh-core-ct/pom.xml
+++ b/jmh-core-ct/pom.xml
@@ -104,6 +104,20 @@ questions.
                 <configuration>
                     <compilerArgument>-proc:none</compilerArgument>
                 </configuration>
+                <executions>
+                    <!-- Configuration for the test compile phase -->
+                    <execution>
+                        <id>test-compile</id>
+                        <goals>
+                            <goal>testCompile</goal>
+                        </goals>
+                        <configuration>
+                            <annotationProcessors>
+                                <annotationProcessor>org.openjdk.jmh.generators.BenchmarkProcessor</annotationProcessor>
+                            </annotationProcessors>
+                        </configuration>
+                    </execution>
+                </executions>
             </plugin>
         </plugins>

This is where I am for now. I will update here if/as I make more progress.

7903596: jmh-java-benchmark-archetype generated benchmark fails at ru…

ac96545

…ntime - ERROR: Unable to find the resource: /META-INF/BenchmarkList

openjdk bot added the rfr Pull request is ready for review label Nov 30, 2023

bridgekeeper bot closed this Feb 24, 2024

merge latest from master branch

29c7950

openjdk bot reopened this Oct 8, 2024

shipilev changed the title ~~7903596: jmh-java-benchmark-archetype generated benchmark fails at runtime - ERROR: Unable to find the resource: /META-INF/BenchmarkList~~ 7903596: JMH: Explicitly enable annotation processors Nov 5, 2024

shipilev reviewed Nov 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

7903596: JMH: Explicitly enable annotation processors #125

7903596: JMH: Explicitly enable annotation processors #125

jaikiran commented Nov 30, 2023 •

edited by openjdk bot

Loading

bridgekeeper bot commented Nov 30, 2023

mlbridge bot commented Nov 30, 2023 •

edited

Loading

shipilev commented Nov 30, 2023

jaikiran commented Nov 30, 2023

jaikiran commented Dec 1, 2023

bridgekeeper bot commented Dec 29, 2023

jaikiran commented Dec 30, 2023

bridgekeeper bot commented Jan 27, 2024

bridgekeeper bot commented Feb 24, 2024

jaikiran commented Oct 8, 2024

openjdk bot commented Oct 8, 2024

openjdk bot commented Oct 8, 2024

jaikiran commented Oct 8, 2024

sigpwned commented Nov 4, 2024 •

edited

Loading

galderz commented Nov 5, 2024

jaikiran commented Nov 5, 2024

shipilev left a comment

sigpwned commented Nov 7, 2024 •

edited

Loading

	<configuration>
	<compilerVersion>1.8</compilerVersion>
	<source>1.8</source>
	<target>1.8</target>
	</configuration>

7903596: JMH: Explicitly enable annotation processors #125

Are you sure you want to change the base?

7903596: JMH: Explicitly enable annotation processors #125

Conversation

jaikiran commented Nov 30, 2023 • edited by openjdk bot Loading

Progress

Issue

Reviewing

bridgekeeper bot commented Nov 30, 2023

mlbridge bot commented Nov 30, 2023 • edited Loading

Webrevs

shipilev commented Nov 30, 2023

jaikiran commented Nov 30, 2023

jaikiran commented Dec 1, 2023

bridgekeeper bot commented Dec 29, 2023

jaikiran commented Dec 30, 2023

bridgekeeper bot commented Jan 27, 2024

bridgekeeper bot commented Feb 24, 2024

jaikiran commented Oct 8, 2024

openjdk bot commented Oct 8, 2024

openjdk bot commented Oct 8, 2024

jaikiran commented Oct 8, 2024

sigpwned commented Nov 4, 2024 • edited Loading

galderz commented Nov 5, 2024

jaikiran commented Nov 5, 2024

shipilev left a comment

Choose a reason for hiding this comment

sigpwned commented Nov 7, 2024 • edited Loading

Edits to pom.xml

Edits to jmh-core-it/pom.xml

Edits to jmh-core-ct/pom.xml

jaikiran commented Nov 30, 2023 •

edited by openjdk bot

Loading

mlbridge bot commented Nov 30, 2023 •

edited

Loading

sigpwned commented Nov 4, 2024 •

edited

Loading

sigpwned commented Nov 7, 2024 •

edited

Loading