Need reviewer: JDK 8 CR for Support Integer overflow (original) (raw)

Eamonn McManus eamonn at mcmanus.net
Sun Feb 5 00:40:00 UTC 2012

Concerning the long multiplyExactly, I have a number of comments.

public static long multiplyExact(long x, long y) {
    long r = x * y;
    long ax = Math.abs(x);
    long ay = Math.abs(y);
    if (((ax | ay) >>> 31 == 0) || (x == 1) || (y == 1)) {
        return r;
    }
    if (((y != 0) && r / y != x) || (r == Long.MIN_VALUE )) {
        throw new ArithmeticException("Multiplication overflows a

long: " + x + " * " + y); } return r; }

I believe that the (ax | ay) condition is an optimization, but I wonder if it is worthwhile. Presumably the intent is to avoid the division if possible, but is division really more expensive than all these extra operations (abs, abs, or, shift, compare) that we are doing?

In addition to returning when x == 1 or y == 1, we could return when y == 0, since we're going to be making that check anyway to avoid divide-by-zero.

The final check (r == Long.MIN_VALUE) is incorrect. I presume the intent is to detect overflow when we multiply Long.MIN_VALUE by -1 (and obtain Long.MIN_VALUE), but Long.MIN_VALUE can be the result of a non-overflowing multiplication, for example ((Long.MIN_VALUE / 2) * 2). The division check will catch the case of x=-1,y=Long.MIN_VALUE, so we could just check for the other case x=Long.MIN_VALUE,y=-1 explicitly.

Éamonn

On 4 February 2012 10:51, Roger Riggs <Roger.Riggs at oracle.com> wrote:

The methods for increment, decrement, and also negation test for exactly one value at the extremities of the value range.

The verbosity of method calls in the source code and the overhead in the execution make these a poor choice for developers. If the code must really operate correctly at the extremities then the developer needs to carefully implement and check where appropriate. The checking for overflow in addition, subtraction, and multiplication are subtle or complex enough to warrant support in the runtime. Roger

On 02/03/2012 01:00 PM, Stephen Colebourne wrote: On 3 February 2012 17:52, Eamonn McManus<eamonn at mcmanus.net>  wrote:

I agree with Stephen Colebourne that brief implementation comments would be useful. But I disagree with his proposed further methods in Math (increment, decrement, int+long variants), which I don't think would pull their weight. FWIW, JSR-310 currently has 18 non-test usages of increment/decrement, vs 42 uses of add. Less, but certainly used. The real value in increment/decrement is that it becomes a simple mapping from operator to method a++ = increment(a) a-- = decrement(a) a + b = add(a, b) This makes it easier to see what the intent would have been were the real operators safe. Stephen

More information about the core-libs-dev mailing list