From 663b0ac320e673e7821f794bda58e9628fa7ae86 Mon Sep 17 00:00:00 2001 From: Charles Oliver Nutter Date: Thu, 12 Dec 2024 18:26:43 -0600 Subject: [PATCH] Revert multiply to use non-intrinsic exactness checks 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/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/jruby#8516 --- core/src/main/java/org/jruby/RubyFixnum.java | 26 ++++++++++++++++---- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/core/src/main/java/org/jruby/RubyFixnum.java b/core/src/main/java/org/jruby/RubyFixnum.java index b6434713016..1ee1d1650ec 100644 --- a/core/src/main/java/org/jruby/RubyFixnum.java +++ b/core/src/main/java/org/jruby/RubyFixnum.java @@ -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) {