Hi Erik,
Thank you for the quick review. I have fixed all your comments in this
new webrev, http://cr.openjdk.java.net/~sfriberg/JDK-8061281/webrev.02
Cheers,
Staffan
On 08/12/2015 03:15 AM, Erik Joelsson wrote:
Hello,
On 2015-08-12 01:34, Staffan Friberg wrote:
Hi,
Unfortunately we have not yet got the new repositories created in
OpenJDK for JEP-230 that was discussed earlier this year [1]. However
I wanted to make sure we are ready to push as soon as those
repositories are created.
These two webrevs contain updates to the build scripts to add the
build-microbenchmark target, and the new repository for the benchmarks.
Makefile change
The configure script now has a new flag that allows you to specifify
a directory where JMH is available, --with-jmh=<PATH>. If the build
environment has been setup correctly the benchmarks can be built
using "make build-microbenchmark". The directory must include the
following JARs commons-math3-3.2.jar, jopt-simple-4.6.jar,
jmh-core-1.10.2.jar, jmh-generator-annprocess-1.10.2.jar, which is
verified by the configure script.
webrev:
http://cr.openjdk.java.net/~sfriberg/JDK-8061281/webrev.01/webrev_jdk/webrev
Only a minor detail in Main.gmk line 288. I would like a + at the
start of the line to indicate to make that the command line is running
a submake. It likely works anyway, but we have this pattern in the
rest of the file.
Benchmarks Repository
Contains the new repository and the directory structure for
microbenchmarks and the specific microbenchmark Makefile. Maven POM
files have been added to make it easy to work with the benchmarks in
IDEs, but are not used as part of the Makefile build process.
webrev:
http://cr.openjdk.java.net/~sfriberg/JDK-8061281/webrev.01/webrev_benchmark/webrev
Only style issues which you may or may not change. These style changes
improves readability and maintainability and we have rather recently
improved the framework to support them. Examples:
Add a space after comma. This wasn't supported before but is now.
$(eval $(call SetupArchive, BUILD_OLDJDK_JAR, \
On the last named variable, finish with ", \" and put the double
parentheses on a new line, indented to the same level as the start of
the expression:
JAR := $(OLDJDK_JAR), \
))
In addition tot hat, in the rules with touch files I would recommend
using $(@D) to refer to the directory instead of repeating it several
times in the recipe to reduce code duplication.
I find this to be an interesting construct:
123 $(foreach jar, $(UNPACK_JARS), \
124 $$($(UNZIP) -oq $(jar) -d $(JMH_UNPACKED)))
I haven't tested, but I think this would work:
123 $(foreach jar, $(UNPACK_JARS), \
124 $(UNZIP) -oq $(jar) -d $(JMH_UNPACKED) $(NEWLINE))
/Erik
[1] -
http://mail.openjdk.java.net/pipermail/jdk9-dev/2015-January/001889.html
Thanks,
Staffan
