<i18n dev> RFR: 8302877: Speed up latin1 case conversions [v2]

Eirik Bjorsnos duke at openjdk.org
Tue Feb 21 11:12:23 UTC 2023


On Tue, 21 Feb 2023 10:29:24 GMT, Claes Redestad <redestad at openjdk.org> wrote:

>> Eirik Bjorsnos has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Spell fix for 'exhaustive' in comments in sun/text/resources
>
> src/java.base/share/classes/java/lang/CharacterDataLatin1.java.template line 142:
> 
>> 140:         }
>> 141:         int l = ch | 0x20; // Lowercase using 'oldest ASCII trick in the book'
>> 142:         if ( l <= 'z' // In range a-z
> 
> Suggestion:
> 
>         if (l <= 'z' // In range a-z

Fixed! (My IDE does not highlight this code, making it a bit harder to spot mistakes like this)

> test/micro/org/openjdk/bench/java/lang/Characters.java line 92:
> 
>> 90:     @Measurement(iterations = 5, time = 1)
>> 91:     @Fork(3)
>> 92:     public static class Latin1CaseConversions {
> 
> Not sure if qualifying this as "Latin1" is necessary, even though that's what you've focused on for this PR. We could easily add some codePoints outside of the latin1 range (now or later) without changing the test. 
> 
> While having a switch with some readable names is a nice touch I think we should additionally allow integer codePoint as-is to keep it in line with the outer class, e.g. `default -> Integer.parseInt(codePoint);`

You are probably right that Latin1 is a bit narrow here, removing the prefix.

I added Integer.parseInt as the default, good idea!

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

PR: https://git.openjdk.org/jdk/pull/12623


More information about the i18n-dev mailing list