RFR: 8161203: ResourceBundle.getBundle performance regression

[Claes Redestad]

On 2016-07-21 15:13, Peter Levart wrote:
> Hi Masayoshi,
>
> Previously the CacheKey::clone() method cleared a reference to 
> 'providers' in the clone making the provides unreachable from the 
> clone and making the clone unable to obtain providers. Now you also 
> reset the 'providersChecked' flag which makes the clone be able to 
> re-obtain the providers. This is dangerous as the clone is used as a 
> key in the cache and is strongly reachable from the cache. A slight 
> future modification of code could unintentionally produce a class 
> loader leak. To prevent that, I would somehow mark the clone so that 
> any attempt to invoke getProviders() on the clone would throw 
> IllegalStateException.

Seems to me that simply setting providersChecked = true; in clone() 
should keep the old behavior?

Also, while not trying to make ResourceBundle thread-safe, perhaps 
getProviders() would be more idiomatically implemented as:

     ServiceLoader<ResourceBundleProvider> getProviders() {
         ServiceLoader<ResourceBundleProvider> providers = this.providers;
         if (!providersChecked) {
this.providers = providers = getServiceLoader(getModule(), name);
providersChecked = true;
}
         return providers;
     }

... to guard against the method ever returning null on weakly-ordered CPUs.

Thanks!

/Claes

>
> Regards, Peter
>
> On 07/21/2016 06:14 AM, Masayoshi Okutsu wrote:
>> Hi,
>>
>> Please review the fix for JDK-8161203. The fix is to lazily load 
>> ResourceBundleProviders. It's not necessary to load providers before 
>> cache look-up.
>>
>> Issue:
>> https://bugs.openjdk.java.net/browse/JDK-8161203
>>
>> Webrev:
>> http://cr.openjdk.java.net/~okutsu/9/8161203/webrev.01
>>
>> Thanks,
>> Masayoshi
>>
>