RFR JDK-8038310: Re-examine integration of extended Charsets

Xueming Shen xueming.shen at oracle.com
Thu May 28 15:53:29 UTC 2015


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



More information about the core-libs-dev mailing list