<i18n dev> [9] RFR 8038436: Re-examine the mechanism to determine available localedata and cldrdata

[Alan Bateman]

On 22/08/2014 19:46, Naoto Sato wrote:
> Hello,
>
> Please review the changes for the following issue:
>
> https://bugs.openjdk.java.net/browse/JDK-8038436
>
> The proposed changes are located at:
>
> http://cr.openjdk.java.net/~naoto/8038436/webrev.3/
>
> The idea is to introduce an SPI so that supported locales are 
> dynamically searched at runtime, not depending on the existence of 
> physical jar files.
>
> I'd appreciate it if build folks could review the makefile related 
> changes, i18n forks to review locale framework files, and Mandy from 
> modularization point of view.
I've skimmed over the changes (not a detailed review, I see Mandy and 
Masayoshi are doing that). It is good to see this code changed to use 
ServiceLoader and dropping the direct access to localeddata.jar and 
cldrdata.jar. It would be useful to see if you can run a few startup 
performance tests to see that the changes have any impact.

On JREENLocaleDataMetaInfo then I wonder if you've considering using 
camel case instead. Maybe "JRE" could be dropped (I realize the type is 
"JRE") and it can be renamed to EnLocaleDataMetaInfo to avoid all caps 
"JREEN".

A minor comment on JREENLocaleDataMetaInfo.getSupportedLocaleString is 
that it could be changed to return 
resourceNameToLocales.getOrDefault(resourceName, "");

For the initialization then this might be help with some of the long 
lines and might make the publication a bit clearer too:

private static final Map<String, String> resourceNameToLocales ;

static {
     Map<String, String> map = new HashMap<>();
     map.put(...);
     resourceNameToLocales = map;
}

On the long lines then JRENonENLocaleDataMetaInfo will need attention as 
the 872 character line will make future side-by-side reviews fun :-)

In CLDRLocaleProviderAdapter's constructor then it is catches/ignores 
exceptions, the subsequent UOE does not help diagnose any issues. 
Hopefully this can be re-examined before these changes are pushed as any 
issue here would be difficult to diagnose.

One thing that isn't clear to me is the getLangTags implementations in 
several classes where it has to cast to JRELocaleProviderAdapter. It's 
not clear to the reader why this is necessary and how it would behave 
with CLDRLocaleProviderAdapter.

-Alan.