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