RFR: JDK-8073152: Update Standard/ExtendedCharsets to work with module system

[Alan Bateman]

On 14/02/2015 20:30, Xueming Shen wrote:
> Hi,
>
> Please help review the changes for  JDK-8073152
>
> Issue: https://bugs.openjdk.java.net/browse/JDK-8073152
> Webrev: http://cr.openjdk.java.net/~sherman/8073152/webrev/
>
> This change is to re-organize how the "standard" and "extended" 
> charsets repository are
> built for the jdk/jre images. Before module system, the "standard" 
> charset provider is built
> into rt.jar and the "extended" charsets provider is in charsets.jar, 
> which is located in the
> "lib/ext" directory. both are available during the system boot time. 
> In jigsaw, the extended
> charsets provider is in jdk.charset (vs the "standard" is in 
> java.base) module and might not
> be "visible" during the early phase of system classes loading, which 
> might cause problem
> as some of the charsets now in "extended" charsets may needed during 
> the boot stage. 
Right, the overall goal is to ensure that we only execute in code 
java.base during startup (essentially until VM.isBooted returns true) so 
this means fixing a number of issues.

Another thing to say is that the compatibility impact will be that 
attempting to start with file.encoding set to a charset that remains in 
jdk.charset will not work but then again, startup up with this property 
set has never been supported anyway. The other impact for embedded 
environments is increased static footprint of java.base but I think that 
can be dealt with via filtering when creating the runtime image for the 
target platform.

So overall then I think these changes look good. At some point I think 
that the private Charset.lookupExtendedCharset should fail if 
VM.isBooted() is false. Also I think ExtendedProviderHolder should go 
away and the extended charsets loaded via ServiceLoader.

Another thing that comes to mind is the Charset.isSupported and forName 
methods, we might need to think about adding a note to their javadoc to 
say that some charsets might not be available during startup. Chris is 
penning similar text for URL protocol handlers.

A few small comments about the changes in the webrev:

1. Is list_old needed, I can't tell if this is checked in for use by 
future archaeologists.

2. Hasher.java is showing up in the webrev as a new file, was this 
build.tools.hasher.Hasher and so we know have two copies?

3. In Typo in SPI.java line 136 ("Unknow type"). As this is a built-time 
tool then it's not a big deal but it is possible to use the streams API 
in more succulent ways, for example List<String> aliasValues = 
cs.aliases().stream().map(a -> 
a.toLowerCase(Locale.ENGLISH)).collect(Collectors::toList);

4. Are there any tests that need to be updated?

Finally, I see the sun.awt.motif.** classes are changed to importing 
using wildcard, a subtle trick as the package name gets decided at build 
time. Hopefully someone in the client area can sort this dependency 
issue soon.

-Alan