É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.ri...@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<eam...@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
