JDK-8061281 (JEP-230): Push Makefiles, repository layout and small set of benchmarks

[Staffan Friberg]

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
>