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

harold seigel harold.seigel at oracle.com
Tue Jun 10 19:44:27 UTC 2014


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.

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