RFR [9] 8151384: Examine sun.misc.ASCIICaseInsensitiveComparator

[Chris Hegarty]

… and the links:

  http://cr.openjdk.java.net/~chegar/8151384/webrev.02/
  http://cr.openjdk.java.net/~chegar/8151384/bench.02/

-Chris.

On 8 Mar 2016, at 11:07, Chris Hegarty <chris.hegarty at oracle.com> wrote:

> On 7 Mar 2016, at 22:36, Xueming Shen <xueming.shen at oracle.com> wrote:
> 
>> 515                 hashCode = name.toLowerCase(Locale.ROOT).hashCode();
> 
> Fixed.
> 
> On 7 Mar 2016, at 22:20, Aleksey Shipilev <aleksey.shipilev at oracle.com> wrote:
> 
>> Hi Chris,
>> 
>> On 03/08/2016 12:55 AM, Chris Hegarty wrote:
>>> Updated links:
>>> http://cr.openjdk.java.net/~chegar/8151384/webrev.01/
>> 
>> *) Your previous patch had the explicit access to
>> CharacterDataLatin1.instance.(toLowerCase|toUpperCase). Any reason not
>> to use it in your new patch? Probably saves a shift and branch on a hot
>> path -- j.l.String is one of those magical places where it actually matters.
> 
> I added this back for Latin1 compareToCI, which seems the most common
> path.
> 
>> Otherwise looks good.
> 
> Thanks.
> 
>>> http://cr.openjdk.java.net/~chegar/8151384/bench.01/
>> 
>> *) This one worries me a little bit. ASCIICaseInsensitive has the
>> anomalous 360 us/op result in one of the forks, and that "best" is
>> better than average StringCaseInsensitive:
>> 
>> http://cr.openjdk.java.net/~chegar/8151384/bench.01/AvailableCharsetsCompare_afterChanges.txt
>> 
>> This might be the genuine run-to-run variance with compiling the hot
>> loop; I'd recommend using Blackhole.consume, not the accumulator
>> variable to try avoiding that.
> 
> Blackhole added.  The results show that the ASCII version is marginally better,
> even after the changes, but I suspect this can be further optimised in the future.
> 
>> Otherwise looks good.
> 
> -Chris.
>