Looks good.
A small comment: make test TEST=micro always builds the native libraries
for the tests. It'd be
nice if running microbenchmarks does not depend on the target of
building native test libraries.
Perhaps you can file a RFE as a follow up.
Mandy
On 11/16/18 4:43 AM, Claes Redestad wrote:
Hi Mandy,
I've updated copyright headers of all microbenchmarks to use the
correct one, and fixed a few instances of long lines that stood out.
We can do more cleanups as a follow-up.
Currently no microbenchmark includes any resources, and as you say the
current convention is to keep resources co-located. I've removed the
classes and resources folders, and updated makefiles with the trivial
changes needed for this.
The JEP text also referred to `make run-test`, which is now just `make
test`. Updated.
To help set thing up JDK-8061281 includes examples and setup
instructions in docs/testing.md|html - I've added a reference to this
documentation in the JEP text. I think using the JEP as the
authoritative place to document usage wouldn't age well, as
instructions typically evolve and change over time.
Combined webrev for 8061281 and 8061282 here:
http://cr.openjdk.java.net/~redestad/8061281_8061282.01/
Thanks!
/Claes
On 2018-11-15 23:50, Mandy Chung wrote:
Hi Claes,
It's good to see this JEP targeted and integrate the microbenchmarks
to colocate with JDK. Overall the work looks good.
The copyright headers need update to GPL. There are some super long
lines (mostly looking up method handles), for example:
+ MethodHandle bodyNormal =
MethodHandles.lookup().findStatic(MethodHandlesCatchException.class,
"doWorkNormal", MethodType.methodType(void.class,
MethodHandlesCatchException.class));
+ MethodHandle bodyExceptional =
MethodHandles.lookup().findStatic(MethodHandlesCatchException.class,
"doWorkExceptional", MethodType.methodType(void.class,
MethodHandlesCatchException.class));
+ MethodHandle fallback =
MethodHandles.lookup().findStatic(MethodHandlesCatchException.class,
"fallback", MethodType.methodType(void.class, MyException.class,
MethodHandlesCatchException.class));
JEP 230 proposes to separate the resources from the source files in
micro/classes and micro/resources directories. What kinds of
resources are expected to be placed under micro/resources directory?
If they are java resources, then I would expect them follow the
consistent layout as JDK source tree where the java classes and
resources are placed together.
I was trying to experiment building and running the benchmarks. What
does configure --with-jmh expect to contain? I can't quite figure it
out from the error message.
The JEP describes make build-microbenchmark and run-test targets to
build and execute the microbenchmarks.
It'd also be helpful to update the JEP to include an example how to
run a specific set of benchmarks e.g.
org.openjdk.bench.java.lang.ObjectHashCode and how to run the
benchmarks with JDK n and JDK n-1 and compare the result (is there a
build target to do this)? We can reference this JEP to get started
running the microbenchmark and refer to JMH and other documentation
for other details like developing a JMH benchmark.
Mandy
On 10/18/18 2:03 PM, Claes Redestad wrote:
Hi,
as the final part of JEP 230: Microbenchmarks Suite, I propose
migrating all microbenchmarks from the codetools
jmh-jdk-microbenchmarks project into the JDK:
http://cr.openjdk.java.net/~redestad/8061282/jdk.00/
This is built on top of the patch for JDK-8061281, and makes the
entirety of this suite readily available to build, run and
experiment with from the main jdk repos.
While the future of the codetools jmh-jdk-microbenchmarks project is
out of scope for JEP 230, it has been suggested it could be kept
alive as a stabilization target and that stable microbenchmarks
should be kept out of the jdk. That discussion is partly out of
scope here, but I believe it makes sense to keep a copy in the JDK
suite precisely because the benchmark will be compiled with the
platform javac, meaning a different set of bugs, regressions and
improvements will be discoverable.
Two micros, org.openjdk.bench.java.lang.invoke.Indify and
org.opendjk.bench.java.lang.reflect.GetAnnotation need special build
treatment and will need to be dealt with in a follow-up.
Thanks!
/Claes
