On 7 Mar 2016, at 22:36, Xueming Shen <xueming.s...@oracle.com> wrote:
> 515 hashCode = name.toLowerCase(Locale.ROOT).hashCode(); Fixed. On 7 Mar 2016, at 22:20, Aleksey Shipilev <aleksey.shipi...@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.