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

[harold seigel]

Thank you!

Harold

On 6/11/2014 9:50 AM, Coleen Phillimore wrote:
>
> Looks great!!
> Coleen
>
> On 6/11/14, 8:18 AM, harold seigel wrote:
>> Hi David,
>>
>> Thanks for all the reviews!
>>
>> I'll make those changes before I check it in.
>>
>> Harold
>>
>> On 6/10/2014 10:19 PM, David Holmes wrote:
>>> Hi Harold,
>>>
>>> New changes look good - thanks!
>>>
>>> One minor nit in thread.cpp:
>>>
>>> 1742     // Invoke java.lang.Thread.dispatchUncaughtException
>>> 1743     if (uncaught_exception.not_null()) {
>>> 1744       EXCEPTION_MARK;
>>> 1745       // Call method Thread.dispatchUncaughtException().
>>>
>>> You don't need both comments. :)
>>>
>>> Also this seems unused:
>>>
>>> 1746       KlassHandle recvrKlass(THREAD, threadObj->klass());
>>>
>>> Thanks,
>>> David
>>>
>>> On 11/06/2014 10:20 AM, harold seigel wrote:
>>>> 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
>>>>>>>
>>>>>>
>>>>>
>>>>
>>
>