RFR 8189208: Cleanup ancient argument processing code
gerard ziemski
gerard.ziemski at oracle.com
Fri May 3 15:25:09 UTC 2019
Thank you Harold for catching the post_vm_init_hook callback issue.
Thank you Harold and David for more feedback.
Here is the latest with the fix:
http://cr.openjdk.java.net/~gziemski/8189208_rev3
cheers
On 5/2/19 5:55 PM, David Holmes wrote:
> On 3/05/2019 5:09 am, Harold Seigel wrote:
>> Hi Gerard,
>>
>> I don't think you can remove the post_vm_init_hook support in
>> thread.cpp. It is still being used.
>
> Right. When I said remove all the "post_vm_init_hook_enabled code" I
> meant only in relation to the flag itself - which is always "true".
> The hook itself must be called.
Thank you Harold
>
> Otherwise additional changes seem fine.
>
> Thanks,
> David
>
>> Harold
>>
>> On 5/2/2019 2:34 PM, gerard ziemski wrote:
>>> Thank you Claes, Harold, David for your reviews!
>>>
>>>
>>> On 5/1/19 5:27 PM, David Holmes wrote:
>>>> Hi Gerard,
>>>>
>>>> Mostly looks good - a couple of comments below.
>>>>
>>>> On 2/05/2019 1:45 am, gerard ziemski wrote:
>>>>> Hi all,
>>>>>
>>>>> Please review this fix, which:
>>>>>
>>>>> #1 removes overriding of “UseLinuxPosixThreadCPUClocks” to
>>>>> different default values in older JDKs
>>>>> #2 removes checks for “supports_thread_park_blocker” and all of
>>>>> the supporting code
>>>>
>>>> src/hotspot/share/include/jvm.h
>>>>
>>>> - unsigned int reserved2;
>>>> + unsigned int reserved2 : 2;
>>>>
>>>> This change seems wrong to me. reserved2 is a full size int (I
>>>> don't know why this struct has been laid out this way but it has).
>>>> To handle the deleted _thread_park_blocker field you can either:
>>>>
>>>> a) don't delete it but rename it to unused_slot; or
>>>> b) change the following "unsigned int : 29;" to "unsigned int :
>>>> 30;" to take up the extra bit.
>>>
>>> Fixed.
>>>
>>>
>>>>
>>>> Also note that the entire comment block from line 1284 can be deleted.
>>>>
>>>> As a follow up we should also remove all the
>>>> post_vm_init_hook_enabled code and
>>>> pending_list_uses_discovered_field code.
>>>
>>> Since I had to put in the above fix, I decided to also remove
>>> "post_vm_init_hook_enabled" and "pending_list_uses_discovered_field"
>>> code as you pointed out.
>>>
>>>
>>>> Or more generally cleanup all the legacy code in JDK_Version that
>>>> was there to allow different JDKs to run on different versions of
>>>> hotspot (eg dll_lookup of JDK_GetVersionInfo0).
>>>
>>> I will handle that in a followup
>>> https://bugs.openjdk.java.net/browse/JDK-8223261
>>>
>>>
>>>>
>>>> Minor nit in threadService.cpp
>>>>
>>>> // Support for JSR-166 locks
>>>> ! if (_thread_status == java_lang_Thread::PARKED ||
>>>> ! _thread_status == java_lang_Thread::PARKED_TIMED) {
>>>>
>>>> Please fix the indent so _thread_status line up. (The original code
>>>> wasn't correctly indented.)
>>>
>>> Fixed, following the existing style.
>>>
>>> New webrev: http://cr.openjdk.java.net/~gziemski/8189208_rev2
>>>
>>>
>>> Cheers
>>>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8189208
>>>>> Web rev: http://cr.openjdk.java.net/~gziemski/8189208_rev1
>>>>> Tested with Mach5 tier1,2,3, another Mach5 tier1,2,3,4,5 in progress…
>
More information about the hotspot-runtime-dev
mailing list