RFR: JDK-8075577: java.time does not support HOST provider

[Roger Riggs]

Hi Rachna,

Ok,  looks fine.

Thanks, Roger


On 12/1/2016 8:20 AM, Rachna Goel wrote:
>
> Hi Roger,
>
> Thanks for the review.   Updated patch is : 
> http://cr.openjdk.java.net/~rgoel/JDK-8075577/webrev.02/
>
> Please find some of my comments inlined.
>
>
> On 30/11/16 10:17 PM, Roger Riggs wrote:
>> Hi Rachna,
>>
>> macosx//HostLocaleProviderAdapterImpl.java:
>>
>>  - line 172-177,  might be a good place to use the new 
>> ConcurrentMap.computeIfAbsent
> I could have replaced those lines with :
> dateFormatPatternsMap.computeIfAbsent(locale,
>                         k -> new SoftReference<>(new 
> AtomicReferenceArray<>(5 * 5)));
>
> But, there are two check made on line no 173. (ref == null ) which 
> will be checked by computeIfAbsent, But about second 
> (dateFormatPatterns = ref.get()) == null)
> will not be checked,  if those lines replaced.
>>
>>  - line 197: you could pre-size the StringBuilder since the target 
>> length can be estimated
>>    (unless they are all less than the default 16)
> pre-sized it with initial " jrepattern" string.
>>
>> macosx && windows//HostLocaleProviderAdapterImpl.java
>>  -  toJavaTimeDateTimePattern()  - is there a way to avoid having two 
>> copies of this function?
>>     Perhaps as a static method in JavaTimeDateTimePatternImpl.java.
> There seems to be no way to avoid having two copies.
> Implementations of "HostLocaleProviderAdapterImpl" for different HOST 
> Environments are loaded at run time by HOSTLocaleProviderAdapter.
> JavaTimeDateTimePatternImpl is an implementation of 
> "JavaTimeDateTimePatternProvider" for JRE LocaleProviderAdapter.
>
>>
>> The noreg-hard label on issue indicates testing is difficult and 
>> specific to OS and host
>> configuration but it also seems unusual to have this much code and 
>> not have a regression test.
>>
> I am in touch with I18n SQE on writing tests for HOST Providers. But 
> testing scope, Golden data are yet to be discussed.
>> If Masayoshi is satisfied and you have tested it in the target 
>> configuration then
>> perhaps it is not worthwhile to invest in a special case regression 
>> test.
>>
>> Thanks, Roger
>>
>> On 11/30/2016 4:39 AM, Masayoshi Okutsu wrote:
>>> Looks good to me.
>>>
>>> Masayoshi
>>>
>>>
>>> On 11/22/2016 6:30 PM, Rachna Goel wrote:
>>>> Hi,
>>>>
>>>> Please review fix for JDK-8075577.
>>>>
>>>> Bug : https://bugs.openjdk.java.net/browse/JDK-8075577
>>>>
>>>> webrev : http://cr.openjdk.java.net/~rgoel/JDK-8075577/webrev.01/
>>>>
>>>> Fix is to introduce new private spi 
>>>> "sun.text.spi.JavaTimeDateTimePatternProvider.java"  to retrieve 
>>>> LocaleProvider specific Date/Time Patterns for "java.time" .
>>>>
>>>> Thanks,
>>>> Rachna
>>>>
>>>>
>>>>
>>>
>>
>