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

[Coleen Phillimore]

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
>>>>>>
>>>>>
>>>>
>>>
>