RFR (S): 8204668: Cleanup management of the java.vm.info System property

[David Holmes]

Thanks Chris.

I added some more info on the initialization sequence to the bug report. 
And filed a related bug on the LogVMOutput output.

David

On 13/06/2018 8:29 AM, Chris Plummer wrote:
> Ok. Changes look good to me.
> 
> thanks,
> 
> Chris
> 
> On 6/12/18 2:37 PM, David Holmes wrote:
>> Hi Chris,
>>
>> Thanks for looking at this.
>>
>> On 13/06/2018 3:52 AM, Chris Plummer wrote:
>>> Hi David,
>>>
>>> Is there a reason you moved the PropertyList_add() call to a slightly 
>>> later point?
>>
>> Just following the existing pattern in that method: define the 
>> SystemProperty objects, then later add them.
>>
>>> You moved the Arguments::update_vm_info_property() call up a bit. How 
>>> did you decide exactly how early it can be called without risk of 
>>> other flags changing?
>>
>> Trial and error. I originally thought it was safe to do this once all 
>> the argument processing was complete: ergo changes, OS changes etc. 
>> But in fact the two flags we are concerned with do not get their final 
>> value until somewhere deep in init_globals(). If I put the update 
>> before init_globals() then it had the wrong value; if I put it after 
>> then it was correct. The main thing was to move it before the 
>> initialization of the system classes which would push the value 
>> through to the Java side.
>>
>> Thanks,
>> David
>>
>>> thanks,
>>>
>>> Chris
>>>
>>> On 6/12/18 2:56 AM, David Holmes wrote:
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8204668
>>>> webrev: http://cr.openjdk.java.net/~dholmes/8204668/webrev/
>>>>
>>>> JDK-8203329 fixed a problem where the native system property for the 
>>>> vm.info string was not updated after argument parsing, resulting in 
>>>> JVM TI reporting an incorrect value.
>>>>
>>>> Looking at the overall approach for this property it can be 
>>>> simplified quite a bit. The basic issue is that it is initialized 
>>>> early in VM startup (so it can be present for crash logs) before 
>>>> argument parsing, but some details can change due to argument 
>>>> parsing. If we update the native value immediately after argument 
>>>> parsing, and so before the properties are passed through to the Java 
>>>> side, then we don't need to execute the Java code in reset_vm_info() 
>>>> to perform that update. Additionally, if we expose the 
>>>> SystemProperty directly (as done for other properties) then we can 
>>>> do away with the new PropertyList_update_value() function that has 
>>>> to search for the property to be updated.
>>>>
>>>> Overall this cuts out a chunk of initialization code that may aid 
>>>> with startup costs; and simplifies the code.
>>>>
>>>> There's some additional history in the bug report.
>>>>
>>>> Testing:
>>>>   - tier 1, 2, 3
>>>>   - regression test from JDK-8203329:
>>>>      - 
>>>> serviceability/jvmti/GetSystemProperty/JvmtiGetSystemPropertyTest.java
>>>>
>>>> Thanks,
>>>> David
>>>
>>>
>>>
> 
>