The performance concern is a fair point and it'll need the benchmark and
proper perf measurement. It's okay with me to follow up as a separate
clean up and you already have the work started any way.
Mandy
On 3/22/18 12:01 PM, Roger Riggs wrote:
Hi Mandy,
This is just one step in untangling and improving property
initialization.
I'm also working on a change similar to what you suggest to remove
some/more of the upcalls
and replace them with setting fields. In that case, the adjustments to
the values would be handled in java.
I have performance concern (as yet unmeasured) that moving from using
a mostly native encoding function
(JNU_NewStringPlatform) to a Java based encoding will hurt startup
performance. (its too early to be compiled).
So for now, I'd like to commit this as is and work on the next step
separately.
Thanks, Roger
On 3/22/18 2:19 PM, mandy chung wrote:
Hi Roger,
Thanks for adding the test that should have created when JDK-4700857
was resolved.
Since this fix changes the setting of those i18n properties to later
after JVM_InitProperties is called, it would be a good opportunity to
set these properties in Java and remove the native fillI18nProps
method. What it needs from Java is to get the value of
sprops->display_XXX, one option is to define a Java class holding
these displayXXX fields that are set by initProperties directly.
What do you think?
Mandy
On 3/22/18 8:24 AM, Roger Riggs wrote:
Hi,
The webrev[1] has been updated with a new test added to verify that
if a user.* property
is defined on the command line, then corresponding property has the
same value and
correctly overrides any value from the platform.
After verification, the comment (line 304) has been corrected to
refer to the platform native
encoding of bytes -> strings instead of the user.* I18N properties.
The tests for properties and locales have been run and passed in a
variety of locales.
Thanks, Roger
[1] http://cr.openjdk.java.net/~rriggs/webrev-prop-simplify-8199756/
On 3/19/2018 6:38 PM, David Holmes wrote:
Hi Roger,
On 19/03/2018 11:55 PM, Roger Riggs wrote:
Hi Alan,
The original changeset [1] is most easily viewed in the jdk-8u repo.
The comment and placement of the removes of the properties and the
code to re-set them
around the call to the JVM_InitProperties (which sets properties
from the
-D arguments among others), seem to indicate that the previous
property
values are irrelevant.
Not irrelevant: defaults.
The remaining question is whether there are any side effects or
usage of those
four property values between the time they are initially set and
when they are removed.
The code in System.initProperties is straightforward and the bulk
of the code
is simply setting property values, with one interspersed call to
initialize the sun.jnu_encoding.
From all appearances there are no side effects or uses of the
initially set values of those four properties.
I would have expected these values to possibly have an impact on
InitalizeEncoding and subsequent use of PUTPROP_ForPlatformNString.
Else why set them early then unset them again? Or maybe they impact
any error messages that might arise in this stage of initialization?
I expect any issues here to be extremely subtle - as Alan
indicated. If there is any path to Java code that may try to read
and use these properties then they need to be set.
And if these are truly unnecessary then doesn't that imply that
placing them in sprops via GetJavaProperties in the first place is
also unnecessary?
Has this been extensively tested on a non-English Windows system?
And do we even have tests for various initialization failures?
David
-----
Thanks, Roger
[1] https://bugs.openjdk.java.net/browse/JDK-4700857
RFE: separating user locale and user interface locale
On 3/17/18 5:46 AM, Alan Bateman wrote:
On 16/03/2018 20:32, Roger Riggs wrote:
Please review a small simplification of the initialization of
System properties for
language, country, script, and variant. Some steps for
initializing them are unnecessary.
The tests pass; a careful review would be appreciated so as to
avoid breakage.
Webrev:
http://cr.openjdk.java.net/~rriggs/webrev-prop-simplify-8199756/
Issue:
https://bugs.openjdk.java.net/browse/JDK-8199756
There are subtle interactions with how system properties are set
in the VM and on the command line which will take a bit effort to
research in order to review this and be satisfied that the change
is safe. Have you dug into the ancient history and bugs to see
why it was done this way?
-Alan
