HotSpot 16 and OpenJDK6
Andrew John Hughes
gnu_andrew at member.fsf.org
Wed Feb 24 02:38:13 PST 2010
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?
>
> -Joe
>
--
Andrew :-)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and the OpenJDK
http://www.gnu.org/software/classpath
http://openjdk.java.net
PGP Key: 94EFD9D8 (http://subkeys.pgp.net)
Fingerprint: F8EF F1EA 401E 2E60 15FA 7927 142C 2591 94EF D9D8
More information about the jdk6-dev
mailing list