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

[Magnus Ihse Bursie]

On 2015-08-13 00:24, Staffan Friberg wrote:
> 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

Looks good to me!

/Magnus
>
> 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
>>
>