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
