RFR(M) JDK-8031819: Remove legacy jdk checks and code
harold seigel
harold.seigel at oracle.com
Mon Jun 9 15:03:04 UTC 2014
Hi,
Please review this updated webrev for JDK-8031819
<https://bugs.openjdk.java.net/browse/JDK-8031819>:
http://cr.openjdk.java.net/~hseigel/bug_8031819_2/
The following files were changed from the previous webrev:
javaClasses.cpp
vmSymbols.hpp
linkResolver.cpp
unsafe.cpp
reflection.cpp
reflectionUtils.cpp
thread.cpp
threadService.cpp
This fix does not contain changes that would prevent hotspot from
working with JDK-8. To test this, I put these changes into a version 9
hotspot and then put that version 9 hotspot into both a JDK-8 and
JDK-9. Then, I ran the JCK lang, vm, and hotspot JTREG tests and had no
issues with either JDK version.
Please see inline responses to David's excellent comments.
Thanks, Harold
On 6/6/2014 1:29 AM, David Holmes wrote:
> Typo:
>
> > You must a chunk of code regarding uncaught exception handling:
>
> You _missed_ a chunk of code regarding uncaught exception handling:
>
> David
> -----
>
> On 6/06/2014 3:26 PM, David Holmes wrote:
>> Hi Harold,
>>
>> Love to see this cleanup! :) But everyone needs to be very aware that
>> once this goes in you can forget about placing a JDK 9 VM anywhere but a
>> JDK 9 JDK. (Or did we already hit that?)
>>
>> A few comments and further cleanups:
>>
>> In classLoader.* is it worth renaming/absorbing the is_rt_jar13 and
>> related "13" items?
Good suggestion, but I think that this is a lot of change without a
significant benefit.
>>
>> ---
>>
>> javaClasses.cpp:
>>
>> This comment:
>>
>> 1732 // For Java 7+ we support the Throwable immutability protocol
>> defined for Java 7. This support
>> 1733 // was missing in 7u0 so in 7u0 there is a workaround in the
>> Throwable class. That workaround
>> 1734 // can be removed in a JDK using this JVM version
>>
>> can be reduced to simply:
>>
>> 1732 // We support the Throwable immutability protocol defined since
>> Java 7.
>>
>> Or even deleted completely.
>>
Done.
>> ---
>>
>> systemDictionary.hpp:
>>
>> The check_klass_Opt_Only_JDK* functions seem to be unused and only
>> generated assertion failures when they were used.
>>
I was unsure about this change so I did not remove the
check_klass_Opt_Only_JDK* functions.
>> ---
>>
>> linkResolver.cpp
>>
>> The comment above the change seems irrelevant now the change is made.
>>
I modified the comment.
>> ---
>>
>> reflection.cpp
>>
>> Comment:
>>
>> 423 // New (1.4) reflection implementation. Allow all accesses from
>> 424 // sun/reflect/MagicAccessorImpl subclasses to succeed
>> trivially.
>>
>> can drop the first sentence. Ditto #526
>>
Done.
>> ---
>>
>> reflectionUtils.cpp
>>
>>
>> This comment makes no sense without a specific version reference and can
>> be deleted:
>>
>> 79 // The following class fields do not exist in previous version of
>> jdk
>>
Done.
>> ---
>>
>> thread.cpp:
>>
>> You must a chunk of code regarding uncaught exception handling:
>>
>> 1744 // JSR-166: change call from from
>> ThreadGroup.uncaughtException to
>> 1745 // java.lang.Thread.dispatchUncaughtException
>> 1746 if (uncaught_exception.not_null()) {
>> 1747 Handle group(this,
>> java_lang_Thread::threadGroup(threadObj()));
>> 1748 {
>> 1749 EXCEPTION_MARK;
>> 1750 // Check if the method Thread.dispatchUncaughtException()
>> exists. If so
>> 1751 // call it. Otherwise we have an older library without the
>> JSR-166 changes,
>> 1752 // so call ThreadGroup.uncaughtException()
>>
>> We only need the code that calls dispatchUncaughtException.
>>
Done.
>> ---
>>
>> threadService.cpp
>>
>> You don't need the block that used to delineate the "if" statement. ie
>> lines:
>>
>> 668 {
>> 678 }
>>
>> can be deleted and the indentation fixed up.
>>
Done.
>> ---
>>
>> src/share/vm/prims/unsafe.cpp:
>>
>> There is unused code in here too: Unsafe_SetObject140,
>> Unsafe_GetObject140
Done.
>>
>> Cheers,
>> David
>>
>> On 5/06/2014 1:34 AM, harold seigel wrote:
>>> Hi,
>>>
>>> Please review this JDK 9 fix for bug JDK-8031819. The fix removes
>>> legacy code for old JDK versions. Although the webrev contains lots of
>>> files, the changes are not complicated.
>>>
>>> Open webrev: http://cr.openjdk.java.net/~hseigel/bug_8031819/
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8031819
>>>
>>> The fix was tested with the Hotspot JTREG tests, JCK Lang, VM, and
>>> API/java_lang tests, nsk Quick tests, and JPRT.
>>>
>>> Thanks! Harold
More information about the hotspot-runtime-dev
mailing list