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

Coleen Phillimore coleen.phillimore at oracle.com
Tue Jun 10 20:05:07 UTC 2014


On 6/10/14, 3:44 PM, harold seigel wrote:
> Hi Coleen,
>
> I agree about not maintaining code to make JDK9 hotspot work with 
> jdk8.  I did not revert any change.  I was just pointing out that this 
> particular proposed change did not cause a JDK9 hotspot 
> incompatibility with JDK8.

Ok, that's good.  Thanks!
Coleen

>
> Thanks, Harold
>
> On 6/10/2014 2:38 PM, Coleen Phillimore wrote:
>>
>> 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