RFR 8189208: Cleanup ancient argument processing code

David Holmes david.holmes at oracle.com
Sat May 4 05:26:27 UTC 2019


Looks good!

Thanks,
David

On 4/05/2019 1:25 am, gerard ziemski wrote:
> 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