RFR 4947890 : Minimize JNI upcalls in system-properties initialization

[Brent Christian]

Hi, Roger

* I like Mandy's idea of not combining the cli/VM props and the 
well-known props into a single array.  Maybe we could then avoid copying 
between arrays (System.c L166).

* The name "Raw" doesn't really speak to me.  It's OK as an inner class, 
though I wonder if everything could be done in SystemProps directly.

A few other comments:

java.base/share/native/libjava/System.c:

  112  * The first FIXED_LENGTH entries are the platform defined 
property values, no names.
  113  * The remaining array indices are alternating key/value pairs
  114  * supplied by the VM including those defined on the command line
  115  * using -Dkey=value that may override the platform defined value.

Some of this would also be useful information SystemProps.java, maybe in 
the comment for Raw.initProps.  Or refer the reader to 
Java_jdk_internal_util_SystemProps_00024Raw_getRawProperties for a 
description of the layout of the array.  Or may become moot if we use 
two separate arrays.

--

file.encoding used to be set from sprops->encoding on Mac, and 
sprops->sun_jnu_encoding otherwise:

  382 #ifdef MACOSX
  383         /*
  384          * Since sun_jnu_encoding is now hard-coded to UTF-8 on 
Mac, we don't
  385          * want to use it to overwrite file.encoding
  386          */
  387         PUTPROP(props, "file.encoding", sprops->encoding);
  388 #else
  389         PUTPROP(props, "file.encoding", sprops->sun_jnu_encoding);
  390 #endif

There is no longer an ifdef for this.  But I guess this is OK, as
SystemProps.initProperties() will only put a value for "file.encoding" 
if there isn't one, and only uses sun.jnu.encoding if there is no 
file.encoding:

   59         putIfAbsent(props, "file.encoding",
   60                 ((raw.propDefault(Raw._file_encoding_NDX) == null)
   61                         ? raw.propDefault(Raw._sun_jnu_encoding_NDX)
   62                         : raw.propDefault(Raw._file_encoding_NDX)));

--

  313     * Command line overrides with -D are copied above from 
JVM_getProperties.

s/JVM_get/JVM_Get/


java.base/share/classes/jdk/internal/util/SystemProps.java:

  126     /**
  127      * Puts the property if it is non-null
  128      * @param props the Properties
  129      * @param key the key
  130      * @param value the value
  131      */
  132     private static void put(Properties props, String key, String 
value) {

I notice that this method is only called once.

--

  175             cmdProps.put(disp, dispValue);
...
  186             cmdProps.put(fmt, fmtValue);

Aren't L175 & L186 just putting the property that is already there ?

Thanks,
-Brent

On 11/13/18 7:59 AM, Roger Riggs wrote:
> Please review a re-implementation of the initialization of System 
> properties
> moving some functions from native to Java.
> 
> The upcalls from native to java for each property are replaced by a 
> mechanism
> to gather the platform, VM and command line properties and return them
> from a single JNI call.  The creation of the Properties instance and 
> applying
> command line overrides is handled in Java instead of native.
> 
> The JVM_initProperties interface in Hotspot is replaced by 
> JVM_getProperties.
> 
> Webrev:
>    http://cr.openjdk.java.net/~rriggs/webrev-props-only-raw/
> 
> Issue:
>    https://bugs.openjdk.java.net/browse/JDK-4947890
> 
> Thanks, Roger
>