RFR 4947890 : Minimize JNI upcalls in system property initialization
Magnus Ihse Bursie
magnus.ihse.bursie at oracle.com
Tue Nov 20 11:10:49 UTC 2018
On 2018-11-20 00:37, Roger Riggs wrote:
> Webrev updated in place:
Build changes still look fine. But please, in the future, avoid updating
webrevs in place, but create new ones instead. It's hopeless to figure
out what has changed otherwise. :-( Updating in place is OK for things
like typos, but not actual code changes...
> 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
> 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.
> 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
>> 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
>> 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
>> 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
>>> 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
>>> - 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)
>>> Thanks, Roger
More information about the build-dev