Hi Mandy,

On 11/14/2018 05:18 PM, Mandy Chung wrote:
Hi Roger,

The system property initialization long for a clean up.
Thanks for looking at it.  This is a good start.  I think
it'd be good to make further improvement.
140 cmdProps = JVM_GetProperties(env); Instead of prepending the predefined set of properties with cmdProps, I suggest to separate getRawProperties into two separate methods 1. get the system properties defined by -D 2. the predefined properties
I considered that too, but decided a single native call was sufficient
even if the String array was dual purpose.
Perhaps with so little impact on performance two calls are insignificant.
176 PUTPROP(propArray, _java_specification_version_NDX, VERSION_SPECIFICATION); 177 PUTPROP(propArray, _java_specification_name_NDX, "Java Platform API Specification"); 178 PUTPROP(propArray, _java_specification_vendor_NDX, JAVA_SPECIFICATION_VENDOR);
179
180 PUTPROP(propArray, _java_vendor_NDX, VENDOR);
181 PUTPROP(propArray, _java_vendor_url_NDX, VENDOR_URL);
182 PUTPROP(propArray, _java_vendor_url_bug_NDX, VENDOR_URL_BUG);
183
184 jio_snprintf(buf, sizeof(buf), "%d.%d", JVM_CLASSFILE_MAJOR_VERSION,
185 JVM_CLASSFILE_MINOR_VERSION);
186 PUTPROP(propArray, _java_class_version_NDX, buf);
These are defined at build time.  Can they be moved to VersionProps.java.template?
yes, but that is a separate cdata structure change, and
there are other properties that should be moved to desktop or 2d modules.

130 // Get the platform specific values
  131     sprops = GetJavaProperties(env);
   :
  191     PUTPROP(propArray, _os_name_NDX, sprops->os_name);
  192     PUTPROP(propArray, _os_version_NDX, sprops->os_version);
  193     PUTPROP(propArray, _os_arch_NDX, sprops->os_arch);

This obtains the platform-specific configuration and then later
sets the value in the array of a predefined index.

Have you considered other alternatives?  SystemProps.Raw can
define the named prop value instead of index e.g.
    private static String _os_arch;
yes, I built a complete prototype using raw fields and having
the native code store to them directly.
I decided that it was more overhead and a more complex native
implementation so was less desirable.  It had no better performance.


these fields are set via JNI.  Or have the native method
to return Object[] with the property name/value pair as
in the VM.

SystemProps.java

   69         putIfAbsent(props, "user.name", 
raw.propDefault(Raw._user_name_NDX));
   70         putIfAbsent(props, "os.name", raw.propDefault(Raw._os_name_NDX));
   71         putIfAbsent(props, "os.arch", raw.propDefault(Raw._os_arch_NDX));
   72         putIfAbsent(props, "os.version", 
raw.propDefault(Raw._os_version_NDX));
   73         putIfAbsent(props, "line.separator", 
raw.propDefault(Raw._line_separator_NDX));
   74         putIfAbsent(props, "file.separator", 
raw.propDefault(Raw._file_separator_NDX));
   75         putIfAbsent(props, "path.separator", 
raw.propDefault(Raw._path_separator_NDX));

jdk.internal.util.StaticProperty calls System.getProperty.
Should we move the above (or more) static properties and
initialize their values directly in StaticProperty class?
This is out of the scope of this issue but if we initialize
the field values directly via JNI.  Something that might
worth exploring.
I left that cleanup for a second pass, if the basic idea was found to be sound.
It kept this change simple, modifying fewer files and kept the change set
easier to understand.

My goal besides performance, was to minimize the native surface area and
structure it in a way to reduce maintenance and native <-> java dependencies.

Thanks, Roger



thanks
Mandy



Reply via email to