Re: Bug in Long.parseUnsignedLong

2013-12-24 Thread Brian Burkhalter
Thanks for the clarification. It looks good to me. Does anyone have any comments on the various competing proposed fixes? As for me, I like this one with int operations and no cache. Thanks, Brian On Dec 23, 2013, at 6:02 PM, Dmitry Nadezhin wrote: We have first/0x1p57 - 1 JAVA{first57}

Re: Bug in Long.parseUnsignedLong

2013-12-23 Thread Brian Burkhalter
It looks like this could be rearranged to long result = first * radix + second; int guard = radix * (int) (first 57); if (guard = 128 || (result = 0 guard = 128 - Character.MAX_RADIX)) { … provided reasonable comments were added. I understand the first part of this conditional, guard = 128,

Re: Bug in Long.parseUnsignedLong

2013-12-23 Thread Dmitry Nadezhin
We have first/0x1p57 - 1 JAVA{first57} JAVA{first57} = first/0x1p57. Here JAVA{.} stands for Java expression with all its truncations and wrappings. Expressions without JAVA{.}are exact mathematical expressions. JAVA{first57} = first/0x1p57 1p64/1p57 = 1p7, radix = Character.MAX_RADIX = 36,

Re: Bug in Long.parseUnsignedLong

2013-12-21 Thread Dmitry Nadezhin
I can weaken the question: Is there a reason to prefer extra int multiplication to the cache ? long result = first * radix + second; final int GUARD_BIT = 7; int guard = radix * (int) (first (Long.SIZE - GUARD_BIT)); if (guard = (1 GUARD_BIT) - Character.MAX_RADIX (guard = (1 GUARD_BIT)

Re: Bug in Long.parseUnsignedLong

2013-12-20 Thread Paul Sandoz
Hi Brian, It would be nice to avoid the caches, on a hunch i am wondering if the following will work: long result = first * radix + second; if ((first 1) (Long.MAX_VALUE / radix) || // possible overflow of first * radix Long.compareUnsigned(result, first) 0) { // overflow of

Re: Bug in Long.parseUnsignedLong

2013-12-20 Thread Dmitry Nadezhin
What is performance of Long.compareUnsigned(x, y) ? Does HotSpot implement it as a call of Long.compare(x + MIN_VALUE, y + MIN_VALUE) or as a single machine instruction ? On Fri, Dec 20, 2013 at 7:53 PM, Paul Sandoz paul.san...@oracle.com wrote: Hi Brian, It would be nice to avoid the

Re: Bug in Long.parseUnsignedLong

2013-12-19 Thread roger riggs
Created JDK-8030814 https://bugs.openjdk.java.net/browse/JDK-8030814 to track this issue. Roger On 12/18/2013 6:52 PM, Louis Wasserman wrote: Derp. Here is the test case: import java.math.BigInteger; public class UnsignedLongBug { public static void main(String[] args) { try {

Re: Bug in Long.parseUnsignedLong

2013-12-19 Thread Brian Burkhalter
Thanks, you saved me the trouble. I already verified it yesterday myself. Brian On Dec 19, 2013, at 8:26 AM, roger riggs wrote: Created JDK-8030814 https://bugs.openjdk.java.net/browse/JDK-8030814 to track this issue.

Re: Bug in Long.parseUnsignedLong

2013-12-19 Thread Paul Sandoz
Hi, I think the logic for overflow when using the compareUnsigned is incorrect in Long: long first = parseLong(s.substring(0, len - 1), radix); int second = Character.digit(s.charAt(len - 1), radix); if (second 0) { throw new

Re: Bug in Long.parseUnsignedLong

2013-12-19 Thread Brian Burkhalter
On Dec 19, 2013, at 9:36 AM, Paul Sandoz wrote: I think the logic for overflow when using the compareUnsigned is incorrect in Long: […] long result = first * radix + second; if (compareUnsigned(result, first) 0) { I concur and verified that yesterday by

Re: Bug in Long.parseUnsignedLong

2013-12-19 Thread Brian Burkhalter
Upon inspection only that indeed looks correct. Thanks … On Dec 19, 2013, at 10:28 AM, Louis Wasserman wrote: Here's one approach that works: there is overflow iff compareUnsigned(first, divideUnsigned(MAX_UNSIGNED, radix)) 0 || (first == divideUnsigned(MAX_UNSIGNED, radix) second

Re: Bug in Long.parseUnsignedLong

2013-12-19 Thread Brian Burkhalter
Here's a formalization of the suggested fix: http://cr.openjdk.java.net/~bpb/8030814/webrev/ It works against the test case. Brian On Dec 19, 2013, at 11:26 AM, Brian Burkhalter wrote: Upon inspection only that indeed looks correct. Thanks … On Dec 19, 2013, at 10:28 AM, Louis