RFR 8199756 : Simplify language, country, script, and variant property initialization

[mandy chung]

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
>>>>>
>>>
>>
>