Need reviewer: JDK 8 CR for Support Integer overflow

[Roger Riggs]

Éamonn,  Thanks for the detailed review.

I did some simple performance tests with and without the optimization test.
With the optimization, performance was nearly doubled.  The individual
tests for  x or y ==1 did not improve performance.  Those cases are
likely optimized inside the divide operation.

Special casing the simple arguments and return values seems to slow down
the non-special cases and it isn't important to speed them up.

I will correct for the multiplyExact case for  Long.MIN_VALUE * -1.

Roger




On 02/04/2012 07:40 PM, Eamonn McManus wrote:
> 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