HotSpot 16 and OpenJDK6

[Joseph D. Darcy]

Joseph D. Darcy wrote:
> Joe Darcy wrote:
>> Andrew John Hughes wrote:
>>> On 24 February 2010 00:28, Joe Darcy <Joe.Darcy at sun.com> wrote:
>>>  
>>>> Andrew John Hughes wrote:
>>>>   
>>>>> On 23 February 2010 20:10, Joseph D. Darcy <Joe.Darcy at sun.com> wrote:
>>>>>
>>>>>     
>>>>>> Hello.
>>>>>>
>>>>>> Andrew John Hughes wrote:
>>>>>>
>>>>>>       
>>>>>>> Here's the merge:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~andrew/jdk6-hs16-merge/webrev.01/
>>>>>>>
>>>>>>> which now builds, thanks to this additional changeset from Daniel
>>>>>>> Daugherty in baseline:
>>>>>>> http://hg.openjdk.java.net/hsx/hsx16/baseline/rev/c9740f5ed5b4
>>>>>>>
>>>>>>>
>>>>>>>           
>>>>>> Good.
>>>>>>
>>>>>>
>>>>>>       
>>>>>>> It also includes an additional fix, also listed separately at:
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~andrew/jdk6-hs16-merge/webrev.02/
>>>>>>>
>>>>>>> which reverts an erroneous change from SIZE_FORMAT to %d in
>>>>>>>
>>>>>>> changeset:   734:dbbe28fc66b5
>>>>>>> user:        twisti
>>>>>>> date:        Fri Feb 27 03:35:40 2009 -0800
>>>>>>> summary:     6778669: Patch from Red Hat -- fixes compilation 
>>>>>>> errors
>>>>>>>
>>>>>>> fixing the build on x86_64.  We'll need a bug ID for this 
>>>>>>> reversion.
>>>>>>>
>>>>>>>
>>>>>>>           
>>>>>> The webrev generally looks good, but I have a few questions and 
>>>>>> comments
>>>>>> before this goes back.
>>>>>>
>>>>>> Do you know why webrev shows so many files with zero changes?  I 
>>>>>> assume
>>>>>> this
>>>>>> is an artifact of the merge.
>>>>>>
>>>>>>
>>>>>>         
>>>>> No, you're right that does seem strange.  All I did was a pull from
>>>>> hs16 and then a merge.  Perhaps this is some webrev oddity?
>>>>>
>>>>> If you look at
>>>>> http://mail.openjdk.java.net/pipermail/jdk6-dev/2010-February/001230.html 
>>>>>
>>>>> I included a diff of the merged OpenJDK6 against the hs16 master
>>>>> (which includes Daniel's patch that is currently only in baseline).
>>>>> It actually makes the other questions below a bit clearer.
>>>>>
>>>>>       
>>>> Yes, that patch size is more manageable!
>>>>
>>>>   
>>>>>> In hotspot_version, what are the semantics of JDK_PREVIOUS_VERSION?
>>>>>>
>>>>>>
>>>>>>         
>>>>> I'm not sure.  It doesn't seem to do much.  Its only use is to set a
>>>>> default BOOTDIR AFAICS:
>>>>>
>>>>> # Find JDK used for javac compiles
>>>>>
>>>>> BOOTDIR=$(SLASH_JAVA)/re/j2se/$(PREVIOUS_JDK_VERSION)/latest/binaries/$(PLATFORM) 
>>>>>
>>>>>
>>>>> (from hotspot/make/defs.make)
>>>>>
>>>>> We actually differ from hs16 by still using 1.5.0 rather than 1.6.0.
>>>>> 1.5.0 implicitly seems to make more sense to me, because 1.6.0 is the
>>>>> version being built.  But in practice, it doesn't seem to matter
>>>>> unless you're relying on the contents of a /java tree.
>>>>>
>>>>>       
>>>> I have a vague recollection of there being a particular reason why 
>>>> this was
>>>> upgraded to 1.6.0 so I'd prefer to see this kept as 1.6.0.
>>>>
>>>>   
>>>>>> In terms of fixing the %d vs SIZE_FORMAT in vmError.cpp, I've filed
>>>>>> 6929005
>>>>>> "Fix format specifier in vmError.cpp."  However, I notice that 
>>>>>> the JDK 7
>>>>>> master still has %d in this location.  I'll approve going back 
>>>>>> from %d ->
>>>>>> SIZE_FORMAT in OpenJDK 6 conditional on the review and approval of a
>>>>>> HotSpot
>>>>>> engineer.
>>>>>>
>>>>>>
>>>>>>         
>>>>> Interesting; I didn't check OpenJDK7 and indeed you're right.  
>>>>> Looking
>>>>> again at the diff, it seems that we don't apply part of the same
>>>>> changeset:
>>>>>
>>>>> --- ../hs16/src/share/vm/utilities/vmError.hpp  2010-01-13
>>>>> 14:35:26.651211783 +0000
>>>>> +++ hotspot/src/share/vm/utilities/vmError.hpp  2010-02-17
>>>>> 11:07:40.899854566 +0000
>>>>> @@ -50,7 +50,7 @@
>>>>>
>>>>>   // additional info for VM internal errors
>>>>>   const char * _filename;
>>>>> -  int          _lineno;
>>>>> +  size_t       _lineno;
>>>>>
>>>>>   // used by fatal error handler
>>>>>   int          _current_step;
>>>>>
>>>>> keeping the size_t rather than using int.  So I'll change the fix to
>>>>> be making vmError.hpp match hs16 and OpenJDK7, rather than changing
>>>>> vmError.cpp.
>>>>>
>>>>> The remaining diffs are typos and license fixups from the previous
>>>>> merge which need to go back to the main HotSpot tree.
>>>>>
>>>>>
>>>>>       
>>>> If vmError.hpp matches what is in JDK 7 and HS 16, I approve your 
>>>> merge of
>>>> HS 16 + Dan's fixes into OpenJDK 6.
>>>>
>>>>     
>>>
>>> Pushed.  I reused 6929005 as Fix HS16 merge issues in OpenJDK 6 to
>>> make vmError.hpp and JDK_PREVIOUS_VERSION match hs16.  You should
>>> probably update the bug itself to match :-)
>>>
>>>   
>>
>> A fine use for that bug id; content of the bug adjusted accordingly :-)
>>
>> -Joe
>
> Hmm, after syncing my repos and doing a build, I see test 2 HotSpot 
> regression test failures
>
>    * compiler/6823453/Test.java : DeoptimizeALot causes fastdebug 
> server jvm to fail with assert(false,"unscheduable graph")
>    * compiler/6833129/Test.java : Object.clone() and Arrays.copyOf 
> ignore coping with -XX:+DeoptimizeALot
>
> both complaining of an unrecognized +DeoptimizeALot option.
>
> -Joe

PS Some more testing results, after the HS update all the langtools 
tests continue to pass and the jdk repo results are about the same.

-Joe