RFR 8196740 : Character.digit(int,int) returns wrong value for out of range radix

[Ivan Gerasimov]

On 2/2/18 5:47 PM, Claes Redestad wrote:
> Hi Ivan,
>
> On 2018-02-03 02:14, Ivan Gerasimov wrote:
>> Would you please help review the fix?
>>
>> BUGURL: https://bugs.openjdk.java.net/browse/JDK-8196740
>> WEBREV: http://cr.openjdk.java.net/~igerasim/8196740/00/webrev/
>
> yes, an obvious error in hindsight!
>
>>
>>
>> I suspect that this version may even work a tiny bit faster, as we 
>> perform less comparisons in a common case. 
>
> Counter-intuitively it actually seems the compiler generates faster 
> code in the end when keeping the
> value >= 0 test around.
>
> Your version:
>
> Benchmark Mode  Cnt     Score   Error  Units
> UUIDBenchmark.benchmarkUUIDFromString avgt    4     0.326 ± 0.017 us/op
> UUIDBenchmark.benchmarkUUIDFromString:branches avgt 289.750           
> #/op
> UUIDBenchmark.benchmarkUUIDFromString:cycles avgt 1044.087           #/op
>
> With:
>         int value = DIGITS[ch];
>         return (value >= 0 && value < radix && radix >= 
> Character.MIN_RADIX
>                 && radix <= Character.MAX_RADIX) ? value : -1;
>
> Benchmark Mode  Cnt     Score   Error  Units
> UUIDBenchmark.benchmarkUUIDFromString avgt    4     0.310 ± 0.018 us/op
> UUIDBenchmark.benchmarkUUIDFromString:branches avgt 256.489           
> #/op
> UUIDBenchmark.benchmarkUUIDFromString:cycles avgt 988.998           #/op
>
> /Claes
>
Hm.  Interesting.
Maybe it helps that you placed the check  (value < radix) before testing 
the radix?
So that when value == -1 we skip the two other checks.

Could you please try your benchmark with this variant:
        return (value < radix && radix >= Character.MIN_RADIX
                 && radix <= Character.MAX_RADIX) ? value : -1;

Thanks!
-- 

With kind regards,
Ivan Gerasimov