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

harold seigel harold.seigel at oracle.com
Wed Jun 11 00:20:30 UTC 2014


Hi,

Please review my latest version of the fix for JDK-8031819 
<https://bugs.openjdk.java.net/browse/JDK-8031819>. It contains the 
fixes suggested below by David and Coleen.

The changed files from the previous webrev are:

    classLoader.cpp
    classLoader.hpp
    systemDictionary.hpp
    thread.cpp

The new webrev is at: http://cr.openjdk.java.net/~hseigel/bug_8031819_3/

The latest changes have been tested with the JCK Lang, VM, and hotspot 
jtreg tests.  Additional tests are in progress.

Thanks! Harold

On 6/10/2014 3:45 PM, harold seigel wrote:
> Hi Coleen,
>
> Thanks for your comments.  I'll make the change that you (and Dave) 
> suggest.
>
> Harold
>
> On 6/10/2014 2:33 PM, Coleen Phillimore wrote:
>>
>> On 6/10/14, 1:24 PM, harold seigel wrote:
>>> Hi David,
>>>
>>> Thanks for the comments.  Please see my comments inline.
>>>
>>> Thanks, Harold
>>>
>>> On 6/10/2014 2:10 AM, David Holmes wrote:
>>>> Hi Harold,
>>>>
>>>> thread.cpp:
>>>>
>>>> You don't need the comment regarding the change in method to be 
>>>> called as again there is no context that tells you there was a 
>>>> different method in an earlier release. So this:
>>>>
>>>> 1742     // JSR-166: change call from ThreadGroup.uncaughtException to
>>>> 1743     // java.lang.Thread.dispatchUncaughtException
>>>>
>>>> can simply be:
>>>>
>>>> 1742     // Invoke java.lang.Thread.dispatchUncaughtException
>>> Done
>>>>
>>>> You are still checking if dispatchUncaughtException exists but that 
>>>> is not necessary so this block can go:
>>>>
>>>> 1748  CallInfo callinfo;
>>>> 1749  KlassHandle 
>>>> thread_klass(THREAD,SystemDictionary::Thread_klass());
>>>> 1750  LinkResolver::resolve_virtual_call(callinfo, threadObj, 
>>>> recvrKlass, thread_klass,
>>>> 1751 vmSymbols::dispatchUncaughtException_name(),
>>>> 1752             vmSymbols::throwable_void_signature(),
>>>> 1753             KlassHandle(), false, false, THREAD);
>>>> 1754  CLEAR_PENDING_EXCEPTION;
>>>> 1755  methodHandle method = callinfo.selected_method();
>>>> 1756  assert(method.not_null(), "Thread.dispatchUncaughtException() 
>>>> not found");
>>>>
>>> I'll remove all of the above except for line 1749. 'thread_klass' is 
>>> needed by the subsequent call to call_virtual().
>>>> ---
>>>>
>>>> A few follow ups on your other responses ...
>>>>
>>>> On 10/06/2014 1:03 AM, harold seigel wrote:
>>>>>>> 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.
>>>>
>>>> The benefit is code clarity, as noone will understand what the 13 
>>>> indicates. It seems to me that the real_jzentry12 and real_jzfile12 
>>>> definitions are now unused. So the "13" variants can just drop the 
>>>> 13 and is_rt_jar13 becomes is_rt_jar.
>>> I'll rename them and post a new webrev.
>>>>
>>>>>>> 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.
>>>>
>>>> Unsure in what sense - that they are dead code? They seem as dead 
>>>> as other things you are deleting :)
>>> The check_klass_Opt_Only_JDK* functions cannot be removed because 
>>> they are still being used.  The WK_KLASSES_DO and WK_KLASS_DECLARE 
>>> macros define functions such as reflect_ConstantPool_klass() and 
>>> reflect_MethodAccessorImpl_klass() that call the 
>>> check_klass_Opt_Only_JDK* functions:
>>>
>>>    #define WK_KLASSES_DO(do_klass)  \
>>>                          ...
>>>       do_klass(reflect_MethodAccessorImpl_klass,
>>>    sun_reflect_MethodAccessorImpl, Opt_Only_JDK14NewRef) \
>>>       do_klass(reflect_ConstructorAccessorImpl_klass,
>>>    sun_reflect_ConstructorAccessorImpl, Opt_Only_JDK14NewRef) \
>>>       do_klass(reflect_ConstantPool_klass,
>>>    sun_reflect_ConstantPool,                  Opt_Only_JDK15 ) \
>>>       do_klass(reflect_UnsafeStaticFieldAccessorImpl_klass,
>>>    sun_reflect_UnsafeStaticFieldAccessorImpl, Opt_Only_JDK15 ) \
>>
>> I think you can change OPT_Only_JDK14NewRef to "Pre" and 
>> Opt_Only_JDK15 to "Opt".  More code to remove!
>>
>> Coleen
>>
>>> ...
>>>
>>>       #define WK_KLASS_DECLARE(name, symbol, option) \
>>>         static Klass* name() { return
>>> check_klass_##option(_well_known_klasses[WK_KLASS_ENUM_NAME(name)]); 
>>> } \
>>>         static Klass** name##_addr() { \
>>>           return
>>> &SystemDictionary::_well_known_klasses[SystemDictionary::WK_KLASS_ENUM_NAME(name)]; 
>>>
>>>    \
>>>         }
>>>       WK_KLASSES_DO(WK_KLASS_DECLARE);
>>>
>>>   declares:
>>>
>>>       static Klass* reflect_MethodAccessorImpl_klass { return
>>>    check_klass_Opt_Only_JDK14NewRef(...);  }
>>>       static Klass* reflect_ConstructorAccessorImpl_klass { return
>>>    check_klass_Opt_Only_JDK14NewRef(...);  }
>>>       static Klass* reflect_ConstantPool_klass() { return
>>>    check_klass_Opt_Only_JDK15(...); }
>>>       static Klass* reflect_UnsafeStaticFieldAccessorImpl_klass() {
>>>    return check_klass_Opt_Only_JDK15(...);  }
>>>
>>>>
>>>> Thanks,
>>>> David
>>>
>>
>



More information about the hotspot-runtime-dev mailing list