On 5/28/15 12:46 AM, Alan Bateman wrote:
On 27/05/2015 20:09, Xueming Shen wrote:
Hi,
Please help review the change of changing the hard-wired extended
charsets loading mechanism
to ServiceLoader.loadInstalled(), for the module system. With the fix
for JDK-8069588 in JDK 9 all
charsets needed to startup in supported environments are in the base
module. This change is to
use ServiceLoader to load the extended charsets to remove the
java.base dependency on jdk.charsets
moduel.
Issues:
JDK-8038310: Re-examine integration of extended Charsets
JDK-8069588: Need to avoid attempting to load extended charsets (from
jdk.charsets module) before module system initialization runs
webrev:
http://cr.openjdk.java.net/~sherman/8069588_8038310/webrev
It's good to get this moved to ServiceLoader as that will make it easy
for jdk.charsets to be a service provider module.
ExtendedProviderHolder.extendedProviders() returning an array is a
surprise but okay given that there will mostly be just the one
provider. Builds for small devices might have zero and would be a bit
cleaner to return an empty array rather than null. Also a bit cleaner
to just use the for-each construct, I don't see any reason to use
hasNext/next here.
The VM.isBooted check looks right as we should never get here during
startup if running on a supported configuration.
Alan,
Thanks for the comments. Webrev has been updated accordingly.
http://cr.openjdk.java.net/~sherman/8069588_8038310/webrev
-Sherman
