RFR(M) JDK-8031819: Remove legacy jdk checks and code

Coleen Phillimore coleen.phillimore at oracle.com
Tue Jun 10 18:38:40 UTC 2014


I can't tell how many changes you reverted.  I don't think we should 
maintain code to make jdk9 hotspot work with jdk8 jdk.  Very soon, it 
should not work and then someone would have to remove this code again.  
I think it would be better to have an explicit version check even.

Coleen


On 6/9/14, 11:03 AM, harold seigel wrote:
> 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