<i18n dev> RFR: 8330802: Desugar switch in Locale::createLocale

[Claes Redestad]

On Mon, 22 Apr 2024 13:05:47 GMT, Chen Liang <liach at openjdk.org> wrote:

>> This switch expression in `Locale::createLocale` is causing a somewhat large startup regression on my local system. Desugaring to if statements seem like the right thing to do while we investigate ways to further reduce overheads in `SwitchBootstraps`.
>> 
>> These numbers are against a baseline which include #18865 and #18845, which already improved the situation:
>> 
>> 
>> Name             Cnt          Base         Error           Test         Error         Unit  Change
>> Perfstartup-Noop  20        40,500 ±       1,942         31,000 ±       2,673        ms/op   1,31x (p = 0,000*)
>>   :.cycles           143254849,000 ± 3398321,355  102205427,650 ± 2192784,853       cycles   0,71x (p = 0,000*)
>>   :.instructions     307138448,850 ± 2095834,550  219415574,800 ±  376992,067 instructions   0,71x (p = 0,000*)
>>   :.taskclock               39,500 ±       1,942         22,500 ±       3,858           ms   0,57x (p = 0,000*)
>>   * = significant
>> 
>> 
>> Comparing to a baseline without those recent improvements the overhead was almost the double: 
>> 
>> Name             Cnt          Base         Error           Test         Error         Unit  Change
>> Perfstartup-Noop  20        50,000 ±       0,000         31,000 ±       2,673        ms/op   1,61x (p = 0,000*)
>>   :.cycles           187047932,000 ± 3330400,381  102205427,650 ± 2192784,853       cycles   0,55x (p = 0,000*)
>>   :.instructions     408219060,350 ± 4031173,140  219415574,800 ±  376992,067 instructions   0,54x (p = 0,000*)
>>   :.taskclock               53,500 ±       4,249         22,500 ±       3,858           ms   0,42x (p = 0,000*)
>>   * = significant
>
> src/java.base/share/classes/java/util/Locale.java line 1003:
> 
>> 1001:             return new Locale(lk.base, lk.exts);
>> 1002:         } else {
>> 1003:             throw new InternalError("should not happen");
> 
> The default branch was required in the switch. Can we simply drop this branch and have a ClassCastException to LocalKey instead?

You mean like so:


    private static Locale createLocale(Object key) {
        if (key instanceof BaseLocale base) {
            return new Locale(base, null);
        }
        LocaleKey lk = (LocaleKey)key;
        return new Locale(lk.base, lk.exts);
    }
 ```
 ?
 
 I think this should be alright. The type of the "impossibly" thrown exception would differ.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/18882#discussion_r1574776021