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

[harold seigel]

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