Skip to content

Commit

Permalink
Revert multiply to use non-intrinsic exactness checks
Browse files Browse the repository at this point in the history
The call to Math.multiplyExact works very well when the result
does not overflow, skipping range checks and using the host CPU's
support for overflow checks. However when we are dancing on the
range boundary and multiplyExact fails repeatedly, the cost of
raising all those exceptions massively outweighs the gains.

This is the primary cause of performance issues reported in
jruby#8516.

This patch reverts to a version of the old range-checking logic.
Performance on the bug report's benchmark is an order of magnitude
faster for the two cases that seemed to overflow a lot, and the
other cases do not seem to degrade.

Fixes jruby#8516
  • Loading branch information
headius committed Jan 10, 2025
1 parent 73e2526 commit b6f3b05
Showing 1 changed file with 14 additions and 5 deletions.
19 changes: 14 additions & 5 deletions core/src/main/java/org/jruby/RubyFixnum.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import java.math.BigInteger;

import org.jcodings.specific.USASCIIEncoding;
import org.jruby.RubyEnumerator.SizeFn;
import org.jruby.compiler.Constantizable;
import org.jruby.runtime.Block;
import org.jruby.runtime.CallSite;
Expand Down Expand Up @@ -603,11 +602,21 @@ private IRubyObject multiplyOther(ThreadContext context, IRubyObject other) {
@Override
public IRubyObject op_mul(ThreadContext context, long other) {
Ruby runtime = context.runtime;
try {
return newFixnum(runtime, Math.multiplyExact(value, other));
} catch (ArithmeticException ae) {
return RubyBignum.newBignum(runtime, value).op_mul(context, other);

long value = this.value;
long result = value * other;

if (other == 0 // result is zero
|| ((Math.abs(value) | Math.abs(other)) >>> 31 == 0) // factors are both int32 range
|| (result / other == value && (value != Long.MIN_VALUE || other != -1)) // division produces value
) {
return newFixnum(runtime, result);
}

// overflow, use Bignum
BigInteger valueBig = BigInteger.valueOf(value);
BigInteger otherBig = BigInteger.valueOf(other);
return RubyBignum.newBignum(runtime, valueBig.multiply(otherBig));
}

public IRubyObject op_mul(ThreadContext context, double other) {
Expand Down

0 comments on commit b6f3b05

Please sign in to comment.