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

[David Holmes]

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
> 
> 
>