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

Coleen Phillimore coleen.phillimore at oracle.com
Tue Jun 10 18:33:23 UTC 2014


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