From d0455fcc405f8cb2f22c8b34736e1fcfe56c4ef1 Mon Sep 17 00:00:00 2001
From: Tim Morgan <tim@timmorgan.org>
Date: Sun, 1 Dec 2024 11:52:31 -0600
Subject: [PATCH] Optimize integer comparisons between fixnum and bignum

Bignums should never be in the NAT_MIN_FIXNUM..NAT_MAX_FIXNUM range, so
we can optimize some comparisons between the different backing types.
---
 include/natalie/bigint.hpp   |  2 +-
 src/integer.cpp              | 39 +++++++++++++++++++++++++++++-------
 test/natalie/integer_test.rb | 37 ++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 8 deletions(-)

diff --git a/include/natalie/bigint.hpp b/include/natalie/bigint.hpp
index 1940728f67..450f54e9fe 100644
--- a/include/natalie/bigint.hpp
+++ b/include/natalie/bigint.hpp
@@ -206,7 +206,7 @@ struct BigInt {
         return strtol(buf, nullptr, 10);
     }
 
-    long to_long_long() const {
+    long long to_long_long() const {
         char buf[32];
         bigint_write(buf, 32, data);
         return strtoll(buf, nullptr, 10);
diff --git a/src/integer.cpp b/src/integer.cpp
index 411914ff32..ffe81c7cf2 100644
--- a/src/integer.cpp
+++ b/src/integer.cpp
@@ -24,9 +24,15 @@ Integer::Integer(double other) {
     }
 }
 
-Integer::Integer(const TM::String &other)
-    : m_bignum(new BigInt(other))
-    , m_is_fixnum(false) {
+Integer::Integer(const TM::String &other) {
+    auto bigint = BigInt(other);
+    if (bigint >= NAT_MIN_FIXNUM && bigint <= NAT_MAX_FIXNUM) {
+        m_is_fixnum = true;
+        m_fixnum = bigint.to_long_long();
+    } else {
+        m_is_fixnum = false;
+        m_bignum = new BigInt(bigint);
+    }
 }
 
 Integer::Integer(const BigInt &other) {
@@ -251,9 +257,20 @@ Integer &Integer::operator--() {
 }
 
 bool Integer::operator<(const Integer &other) const {
-    if (is_bignum() || other.is_bignum()) {
-        return to_bigint() < other.to_bigint();
+    if (is_bignum()) {
+        if (other.is_bignum())
+            return *m_bignum < *other.m_bignum;
+        else if (m_bignum->is_negative())
+            return true;
+        else
+            return false;
+    } else if (other.is_bignum()) {
+        if (other.is_negative())
+            return false;
+        else
+            return true;
     }
+
     return m_fixnum < other.m_fixnum;
 }
 
@@ -270,8 +287,14 @@ bool Integer::operator>=(const Integer &other) const {
 }
 
 bool Integer::operator==(const Integer &other) const {
-    if (is_bignum() || other.is_bignum())
-        return to_bigint() == other.to_bigint();
+    if (is_bignum()) {
+        if (other.is_bignum())
+            return *m_bignum == *other.m_bignum;
+        else
+            return false;
+    } else if (other.is_bignum()) {
+        return false;
+    }
 
     return m_fixnum == other.m_fixnum;
 }
@@ -307,12 +330,14 @@ bool Integer::operator>=(const double &other) const {
 bool Integer::operator==(const double &other) const {
     if (is_bignum())
         return to_bigint() == other;
+
     return to_double() == other;
 }
 
 bool Integer::operator!=(const double &other) const {
     if (is_bignum())
         return to_bigint() != other;
+
     return to_double() != other;
 }
 
diff --git a/test/natalie/integer_test.rb b/test/natalie/integer_test.rb
index e754dd89e3..57d8593a0e 100644
--- a/test/natalie/integer_test.rb
+++ b/test/natalie/integer_test.rb
@@ -165,6 +165,20 @@
       (1 == Rational(1, 2)).should == false
       (1 == Rational(1, 1)).should == true
     end
+
+    it 'handles bignums correctly' do
+      (1 == bignum_value).should == false
+      (0 == bignum_value).should == false
+      (-1 == bignum_value).should == false
+
+      (bignum_value == 1).should == false
+      (bignum_value == 0).should == false
+      (bignum_value == -1).should == false
+
+      (bignum_value == bignum_value).should == true
+      (-bignum_value == -bignum_value).should == true
+      (-bignum_value == bignum_value - 1).should == false
+    end
   end
 
   describe 'eql?' do
@@ -196,6 +210,29 @@
       (1 < Rational(1, 2)).should == false
       (1 < Rational(1, 1)).should == false
     end
+
+    it 'handles bignums correctly' do
+      (1 < bignum_value).should == true
+      (0 < bignum_value).should == true
+      (-1 < bignum_value).should == true
+
+      (1 < -bignum_value).should == false
+      (0 < -bignum_value).should == false
+      (-1 < -bignum_value).should == false
+
+      (bignum_value < 1).should == false
+      (bignum_value < 0).should == false
+      (bignum_value < -1).should == false
+
+      (-bignum_value < 1).should == true
+      (-bignum_value < 0).should == true
+      (-bignum_value < -1).should == true
+
+      (-bignum_value < bignum_value).should == true
+      (bignum_value < -bignum_value).should == false
+      (bignum_value < bignum_value - 1).should == false
+      (bignum_value - 1 < bignum_value).should == true
+    end
   end
 
   describe '<=' do