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

[Ivan Gerasimov]

On 2/2/18 6:36 PM, Ivan Gerasimov wrote:
>
>
> 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;
>
Not sure why I thought that for (value == -1) the two other checks will 
be skipped.
But still, could you please check that variant with the checks reordered?

> Thanks!

-- 
With kind regards,
Ivan Gerasimov