RFR 4947890 : Minimize JNI upcalls in system property initialization

Roger Riggs Roger.Riggs at oracle.com
Mon Nov 19 23:37:44 UTC 2018


Hi,

Webrev updated in place:

     http://cr.openjdk.java.net/~rriggs/webrev-props-only-raw

On 11/16/2018 06:36 PM, Mandy Chung wrote:
> Hi Roger,
>
> Looking good.  I have a few small comments:
>
> I assume VM.saveAndRemoveProperties will be a separate cleanup.
> SystemProps::cmdProperties adds the system properties that can
> skip adding these internal properties to the Properties object.
> In fact, these internal properties are the interface between VM
> and libraries that can be replaced with a different mechanism
> other than system properties.
fine as a future change
>
> Suggest to move the call to VersionProps.init(props) in
> SystemProps.initProperties
That's harder that it might seems, the method can't be made public
and moving it to jdk.internal.util with SystemProps is more difficult
since there is Hotspot code that reads those fields directly. (thread.cpp)
That goes back quiet a number of years.

Another future cleanup/decoupling.
>
> The `VENDOR*` build time variables can always have a default
> like the other constants in VersionProps.  Then no need to
> handle if not set.
Moved defaults to configure,
easy to override with --with-vendor-url, --with-vendor-bug-url, and 
--with-vendor-name.
>
> SystemProps.staticInitOnly_* are a bit odd.  They are temporarily
> set with the values and then reset to null.  Perhaps leave
> StaticProperty as is for now and re-visit it in the future.
> Then I think SystemProps::putDefault can be removed.
ok, reverted the code so the static values are read via System.getProperty.
>
> Raw::xxx_NDX are initialized to 1 + previous_NDX.  It's a general
> good approach to increment the index but I find it error-prone and
> hard to catch mistake since the (adjacent) variable names look
> so alike. Perhaps some form of verification or assertion to ensure
> the indices are correctly initialized.
Added a test that uses reflection to verify the uniqueness and sequence.

Thanks, Roger
>
> Mandy
>
> On 11/16/18 10:49 AM, Roger Riggs wrote:
>> Updates to include the suggestions made by Mandy and Brent:
>>
>>  - Move the build-time properties from native code to the 
>> VersionProps.java.template
>>    including java.vendor.* and java.specification.* properties, plus 
>> the java.class.version (major.minor)
>>    This included two changes to the build that generates source of 
>> VersionProps.java.
>>
>>  - Updated the StaticProperty initialization to be explicitly invoked 
>> by initProperties.
>>
>>  - Makes separate calls to native to retrieve the platform properties 
>> and VM/command line properties
>>
>>  - (The hotspot function for JVM_GetProperties are unchanged)
>>
>> Webrev:
>> http://cr.openjdk.java.net/~rriggs/webrev-props-only-raw/
>>
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-4947890
>>
>> Thanks, Roger
>>
>>
>



More information about the core-libs-dev mailing list