RFR: JDK-8075577: java.time does not support HOST provider
Roger.Riggs at Oracle.com
Fri Dec 2 15:35:26 UTC 2016
Ok, looks fine.
On 12/1/2016 8:20 AM, Rachna Goel wrote:
> Hi Roger,
> Thanks for the review. Updated patch is :
> Please find some of my comments inlined.
> On 30/11/16 10:17 PM, Roger Riggs wrote:
>> Hi Rachna,
>> - line 172-177, might be a good place to use the new
> I could have replaced those lines with :
> 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
>> Thanks, Roger
>> On 11/30/2016 4:39 AM, Masayoshi Okutsu wrote:
>>> Looks good to me.
>>> On 11/22/2016 6:30 PM, Rachna Goel wrote:
>>>> 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" .
More information about the core-libs-dev