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

[Rachna Goel]

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