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

Chris Hegarty chris.hegarty at oracle.com
Mon Mar 7 21:55:52 UTC 2016


Aleksey,

Very helpful, as always.

I pushed the methods down into  String[Latin1|UTF16], and followed existing
style. This is much cleaner.

Thanks for catching the silly mistakes in the benchmarks.

Updated links:
  http://cr.openjdk.java.net/~chegar/8151384/webrev.01/
  http://cr.openjdk.java.net/~chegar/8151384/bench.01/

-Chris.


On 7 Mar 2016, at 16:51, Aleksey Shipilev <aleksey.shipilev at oracle.com> wrote:

> Hi,
> 
> On 03/07/2016 07:29 PM, Chris Hegarty wrote:
>> What is in the webrev is specialized versions of compare when
>> the coder of the strings match. Alternatively, this could be pushed
>> down to String[Latin1|UTF16].
>> 
>> Webrev & bug:
>>  http://cr.openjdk.java.net/~chegar/8151384/webrev.00/
>>  https://bugs.openjdk.java.net/browse/JDK-8151384
> 
> Overall, good cleanup idea. I think the actual helpers deserve to be
> pushed to String[Latin1|UTF16], as String is already overloaded with
> lots of code. See how e.g. String.regionMatches(boolean ignoreCase, int
> toffset, String other, int ooffset, int len) does it.
> 
> Nits:
> 
> *) Change:  compareLatin1ToUTF16(v2, v1) * -1;
>        To: -compareLatin1ToUTF16(v2, v1);
> 
> *) Do we really need to cast up/down to "char" in compareLatin1*?
> 
> 
>> Benchmarks and results ( based, somewhat, on Aleksey's [1] ):
>>  http://cr.openjdk.java.net/~chegar/8151384/bench/
> 
> Comments on benchmarks (that might have impact on validity):
> 
> *) "# JMH 1.6 (released 388 days ago, please consider updating!)"
> 
> *) CaseInsensitiveCompare.cmp1_cmp1 is suspiciously unaffected by size.
> That's because benchmark goes through the identity comparison:
> 
>    @Benchmark
>    @CompilerControl(CompilerControl.Mode.DONT_INLINE)
>    public int cmp1_cmp1() {
>        return CASE_INSENSITIVE_ORDER.compare(cmp1_1, cmp1_1);
>    }
> 
> ...should really be:
> 
>    @Benchmark
>    @CompilerControl(CompilerControl.Mode.DONT_INLINE)
>    public int cmp1_cmp1() {
>        return CASE_INSENSITIVE_ORDER.compare(cmp1_1, cmp1_2);
>    }
> 
> *) Probable dead-code elimination here:
> 
>    @Benchmark
>    @CompilerControl(CompilerControl.Mode.DONT_INLINE)
>    public void StringCaseInsensitiveComparator() {
>        List<String> strings = AVAILABLE_CHARSETS;
>        for (String s1 : strings) {
>            for (String s2 : strings) {
>                 String.CASE_INSENSITIVE_ORDER.compare(s1, s2);
>            }
>        }
>    }
> 
> ...should be:
> 
>    @Benchmark
>    @CompilerControl(CompilerControl.Mode.DONT_INLINE)
>    public void StringCaseInsensitiveComparator(Blackhole bh) {
>        List<String> strings = AVAILABLE_CHARSETS;
>        for (String s1 : strings) {
>            for (String s2 : strings) {
>                 bh.consume(String.CASE_INSENSITIVE_ORDER.compare(s1, s2));
>            }
>        }
>    }
> 
> 
> Thanks,
> -Aleksey
> 




More information about the core-libs-dev mailing list