Test Backports

[Joe Darcy]

On 04/08/10 08:20 AM, Andrew John Hughes wrote:
> On 8 April 2010 02:20, Joe Darcy <joe.darcy at oracle.com> wrote:
>   
>> Andrew John Hughes wrote:
>>     
>>> On 7 April 2010 00:25, Joe Darcy <joe.darcy at oracle.com> wrote:
>>>
>>>       
>>>> On 04/06/10 01:27 PM, Andrew John Hughes wrote:
>>>>
>>>> On 6 April 2010 01:43, Joe Darcy <joe.darcy at oracle.com> wrote:
>>>>
>>>>         
>> [snip]
>>     
>>>> On my queue, I have four more Zero patches and a set of backports I'd
>>>> like in (making the source/target explicit as we did in 7 already, and
>>>> Kelly's ant 1.8 patch).  Everything else can wait until b20
>>>>
>>>>
>>>>
>>>> Here's the backport:
>>>>
>>>> http://cr.openjdk.java.net/~andrew/6873059/webrev.01/jdk6.patch
>>>>
>>>> It's a replica of 6873059 as applied to the HotSpot, JDK and CORBA
>>>> trees in OpenJDK7, the only difference being that we use 5 instead of
>>>> 6 as the bootstrap version for OpenJDK6.  Ok to push?  Should I use
>>>> the same bug ID or do you want to allocate a fresh one?
>>>>
>>>>
>>>> Using the same bug id is fine, but I'd like Kelly to sanity check it
>>>> before
>>>> it goes back.
>>>>
>>>>
>>>>         
>>> Ok, waiting for Kelly's response.
>>>
>>>       
>> Acknowledged.
>>
>>     
>>>> On another note, there is now some code requiring source level 6 in
>>>> OpenJDK6 (due to use of the @Override annotation on interfaces):
>>>>
>>>>  src/share/classes/javax/swing/plaf/synth/SynthComboBoxUI.java
>>>>  src/share/classes/javax/swing/plaf/synth/SynthLookAndFeel.java
>>>>  src/share/classes/javax/swing/plaf/synth/SynthTreeUI.java
>>>>  src/share/classes/sun/security/provider/certpath/OCSPResponse.java
>>>>  src/share/classes/sun/swing/plaf/synth/SynthFileChooserUIImpl.java
>>>>
>>>>
>>>> There is an overly-long story behind -source 5 vs. -source 6 and
>>>> @Override.
>>>> The short answer is that javac in JDK 6 unconditionally applies the more
>>>> liberal (and more useful) semantics for @Override.  For the JDK sources,
>>>> a
>>>> compiler that does the same should be used.
>>>>
>>>>
>>>>         
>>> Exactly.  I know ecj throws it out for 5 but not for 6 and thus
>>> building using ecj (our method for bootstrapping) falls foul of the
>>> classes above.  I'm not sure if javac is allowing them through because
>>> its version of 1.5 allows it or it is simply defaulting to 6 because
>>> nothing else is specified.  I had a quick look but the version is
>>> currently set in more than one place, so I think this needs a more
>>> in-depth review and I'd prefer it waits until b20 to be on the safe
>>> side.
>>>
>>>       
>> The javac command in JDK 6, both open and proprietary, will allow use of
>> @Override on interfaces, even if "-source 5" is specified.
>>
>>     
>
> Ok, that explains why it works for javac and fails for ecj.  Is there
> any reason we're using 5 for a build of OpenJDK 6?  The bootstrapping
> classes are now explicitly 5, but surely the final JDK code should be
> 6.
>   

Not necessarily.  How rt.jar and tools.jar are compiled (and even if 
there is a rt.jar and tools.jar) is an implementation artifact of the 
JDK.  One of the main benefits of target 6 over target 5 is that 
stackmaps are supported in the class files which allows for faster class 
file verification.  However, files on the bootclasspath, such as rt.jar, 
are not verified anyway so using target 6 to compile the classes going 
into rt.jar would add to the size of rt.jar without conferring any speed 
benefit since verification is skipped anyway.

>>>> So we should look at bumping the generated code version to 6 (it still
>>>> seems to be 5 even though this is OpenJDK6).  I'd prefer to leave that
>>>> until b20 though.
>>>>
>>>> I see Kelly's patch went in.  It would be nice to also backport
>>>> http://hg.openjdk.java.net/jdk7/jdk7/hotspot/rev/5fdbe2cdf565 (a minor
>>>> warning fix) so IcedTea6's OpenJDK backport set is empty again.
>>>>
>>>>
>>>> I approve the warning fix being backported.
>>>>
>>>>
>>>>         
>>> Done: http://hg.openjdk.java.net/jdk6/jdk6/hotspot/rev/6ee696377676
>>>
>>>       
>> Thanks,
>>
>>     
>>> The first of the four Zero backports is 6903453: Zero build on ARM and
>>> IA-64.
>>>
>>> http://cr.openjdk.java.net/~andrew/6903453/webrev.01/
>>>
>>> It adds a few build conditionals for building on arm and ia64 platforms.
>>>
>>> Ok to push?
>>>
>>>       
>> Approved to go back.
>>
>>     
>
> Done:
>
> http://hg.openjdk.java.net/jdk6/jdk6/corba/rev/e83301fe4687
> http://hg.openjdk.java.net/jdk6/jdk6/jdk/rev/17a217fd1d49
>
> Next one is 6909153 which turns off a number of options that fail or
> are inappropriate on Zero:
>
> http://cr.openjdk.java.net/~andrew/6909153/webrev.01/
>
> Ok to push?
>   

Approved to go back.

I'm not sure of the HotSpot conventions, but the removal of the opening 
brace on the end of line 1824 of compileBroker.cpp looks suspect to me.

However, I see this was done in the JDK 7 HotSpot sources so it is more 
important to keep the patches the same.

>   
>> I've run some regression tests on a build from the current tip and the
>> result look good; in particular, all the rmid tests pass again :-)
>>
>>     
>
> These results look good.  Plenty of new tests :-)
>
> The only odd one seems to be
>
>   
>> pass   fail   com/sun/crypto/provider/KeyFactory/TestProviderLeak.java
>>     
>
> The rest are either new or disabled by the security update.
>   

It appears a port of 6923976: TestProviderLeak.java is using too small 
of an initial heap under newer Hotspot (b79+) is needed:
http://hg.openjdk.java.net/jdk7/tl/jdk/rev/7136683a33d2

I've applied the patch to a local repo, the test passes again, and I've 
pushed this changeset into the master.

Thanks for noticing this and prompting an investigation,

-Joe
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/jdk6-dev/attachments/20100408/9be67044/attachment.html