HotSpot 16 and OpenJDK6
Joseph D. Darcy
Joe.Darcy at Sun.COM
Wed Feb 24 08:16:34 PST 2010
Andrew John Hughes wrote:
> On 24 February 2010 03:14, Joseph D. Darcy <Joe.Darcy at sun.com> 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.
>>
>
> Hmmm, both are new tests:
>
> changeset: 1023:9987d9d5eb0e
> user: cfang
> date: Fri Jul 31 17:12:33 2009 -0700
> summary: 6833129: specjvm98 fails with NullPointerException in the
> compiler with -XX:DeoptimizeALot
>
> changeset: 858:9c6be3edf0dc
> user: cfang
> date: Thu Apr 23 14:04:24 2009 -0700
> summary: 6589834: deoptimization problem with -XX:+DeoptimizeALot
>
> I guess we need the same fix as in this previous changeset:
>
> $ hg export 624
> # HG changeset patch
> # User andrew
> # Date 1254394160 -3600
> # Node ID 012339cadcbab0c1568e19dac6727b4b7a4cc4de
> # Parent 459552353368cf0cebcf5738d95ad6ebdc58f635
> 6886353: For DeoptimizeALot, JTreg tests should
> "IgnoreUnrecognizedVMOptions on a product build
> Summary: Add IgnoreUnrecognizedVMOptions so JTReg tests which use
> DeoptimizeALot pass for product builds
> Reviewed-by: darcy
>
> diff -r 459552353368 -r 012339cadcba test/compiler/6775880/Test.java
> --- a/test/compiler/6775880/Test.java Mon Sep 28 23:54:25 2009 +0100
> +++ b/test/compiler/6775880/Test.java Thu Oct 01 11:49:20 2009 +0100
> @@ -27,7 +27,7 @@
> * @bug 6775880
> * @summary EA +DeoptimizeALot:
> assert(mon_info->owner()->is_locked(),"object must be locked now")
> * @compile -source 1.4 -target 1.4 Test.java
> - * @run main/othervm -server -Xbatch -XX:+DoEscapeAnalysis
> -XX:+DeoptimizeALot
> -XX:CompileCommand=exclude,java.lang.AbstractStringBuilder::append
> Test
> + * @run main/othervm -server -Xbatch -XX:+IgnoreUnrecognizedVMOptions
> -XX:+DoEscapeAnalysis -XX:+DeoptimizeALot
> -XX:CompileCommand=exclude,java.lang.AbstractStringBuilder::append
> Test
> */
>
> And indeed it's fixed in JDK7. We need to backport the JDK7 6886353 fix:
>
> changeset: 996:d6b9fd78f389
> user: cfang
> date: Mon Sep 28 17:14:25 2009 -0700
> summary: 6886353: For DeoptimizeALot, JTreg tests should
> "IgnoreUnrecognizedVMOptions on a product build
>
> which, despite claiming to be by cfang, was actually submitted by
> myself: http://bugs.sun.com/view_bug.do?bug_id=6886353
>
> Ok to backport?
>
Certainly.
-Joe
More information about the jdk6-dev
mailing list