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 Dec 13, 2024
1 parent 73e2526 commit 663b0ac
Showing 1 changed file with 21 additions and 5 deletions.
26 changes: 21 additions & 5 deletions core/src/main/java/org/jruby/RubyFixnum.java
Original file line number Diff line number Diff line change
Expand Up @@ -602,12 +602,28 @@ 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);
final Ruby runtime = context.runtime;
final long value = this.value;

if (value == 0 || other == 0) {
return RubyFixnum.zero(runtime);
}

// fast check for known ranges that won't overflow
if (
(value <= 3037000499L && other <= 3037000499L && value >= -3037000499L && other >= -3037000499L) ||
(value == -1 && other != Long.MIN_VALUE)
) {
return newFixnum(runtime, value * other);
} else {
long result = value * other;
if (result / value == other) {
return newFixnum(runtime, result);
}
}

// if here (value * otherValue) overflows long, so must return Bignum
return RubyBignum.newBignum(runtime, value).op_mul(context, other);
}

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

0 comments on commit 663b0ac

Please sign in to comment.