tdf#94677 Calc is slow opening large CSV, improve tools::Fraction

Flatten the tools::Fraction class.

Shaves 1s off a load time of 49s

Change-Id: I7cbee9892831a1e47704a283351fa97eda262343
Reviewed-on: https://gerrit.libreoffice.org/74639
Tested-by: Jenkins
Reviewed-by: Noel Grandin <noel.grandin@collabora.co.uk>

2 files changed, 80 insertions, 115 deletions

diff --git a/include/tools/fract.hxx b/include/tools/fract.hxx
index bba143b0530d..ed1f5f0be649 100644
--- a/include/tools/fract.hxx
+++ b/include/tools/fract.hxx
@@ -29,14 +29,15 @@ class SvStream;
 
 class SAL_WARN_UNUSED TOOLS_DLLPUBLIC Fraction final
 {
-    struct Impl;
-
-    std::unique_ptr<Impl> mpImpl;
+    /// these two fields form a boost::rational, but I didn't want to put more boost headers into the global space
+    sal_Int32       mnNumerator = 0;
+    sal_Int32       mnDenominator = 1;
+    bool            mbValid = true;
 
 public:
-                    Fraction();
-                    Fraction( const Fraction & rFrac );
-                    Fraction( Fraction && rFrac );
+                    Fraction() = default;
+                    Fraction( const Fraction & rFrac ) = default;
+                    Fraction( Fraction && rFrac ) = default;
     explicit        Fraction( double dVal );
                     Fraction( double nNum, double nDen );
                     Fraction( sal_Int64 nNum, sal_Int64 nDen );
@@ -45,9 +46,8 @@ public:
                         T1 nNum, T2 nDen,
                         typename std::enable_if<std::is_integral<T1>::value && std::is_integral<T2>::value, int>::type = 0)
                         : Fraction( sal_Int64(nNum), sal_Int64(nDen) ) {}
-                    ~Fraction();
 
-    bool            IsValid() const;
+    bool            IsValid() const { return mbValid; }
 
     sal_Int32       GetNumerator() const;
     sal_Int32       GetDenominator() const;
@@ -58,8 +58,8 @@ public:
 #endif
     explicit operator double() const;
 
-    Fraction&       operator=( const Fraction& rfrFrac );
-    Fraction&       operator=( Fraction&& rfrFrac );
+    Fraction&       operator=( const Fraction& rfrFrac ) = default;
+    Fraction&       operator=( Fraction&& rfrFrac ) = default;
     Fraction&       operator=( double v ) { return operator=(Fraction(v)); }
 
     Fraction&       operator+=( const Fraction& rfrFrac );
@@ -85,7 +85,7 @@ public:
     TOOLS_DLLPUBLIC friend bool operator<=( const Fraction& rVal1, const Fraction& rVal2 );
     TOOLS_DLLPUBLIC friend bool operator>=( const Fraction& rVal1, const Fraction& rVal2 );
 
-    TOOLS_DLLPUBLIC friend SvStream& ReadFraction( SvStream& rIStream, Fraction const & rFract );
+    TOOLS_DLLPUBLIC friend SvStream& ReadFraction( SvStream& rIStream, Fraction & rFract );
     TOOLS_DLLPUBLIC friend SvStream& WriteFraction( SvStream& rOStream, const Fraction& rFract );
 };
 
diff --git a/tools/source/generic/fract.cxx b/tools/source/generic/fract.cxx
index e6e05dfd5dc6..3a8eb09822c8 100644
--- a/tools/source/generic/fract.cxx
+++ b/tools/source/generic/fract.cxx
@@ -39,40 +39,16 @@ static boost::rational<sal_Int32> rational_FromDouble(double dVal);
 
 static void rational_ReduceInaccurate(boost::rational<sal_Int32>& rRational, unsigned nSignificantBits);
 
-struct Fraction::Impl
-{
-    bool                        valid;
-    boost::rational<sal_Int32>  value;
-
-    Impl()
-        : valid(false)
-    {
-    }
-    Impl(const Impl&) = delete;
-    Impl& operator=(const Impl&) = delete;
-};
-
-Fraction::Fraction() : mpImpl(new Impl)
-{
-    mpImpl->valid = true;
-}
-
-Fraction::Fraction( const Fraction& rFrac ) : mpImpl(new Impl)
-{
-    mpImpl->valid = rFrac.mpImpl->valid;
-    if (mpImpl->valid)
-        mpImpl->value.assign( rFrac.mpImpl->value.numerator(), rFrac.mpImpl->value.denominator() );
-}
-
-Fraction::Fraction( Fraction&& rFrac ) : mpImpl(std::move(rFrac.mpImpl))
+static boost::rational<sal_Int32> toRational(sal_Int32 n, sal_Int32 d)
 {
+    return boost::rational<sal_Int32>(n, d);
 }
 
 // Initialized by setting nNum as nominator and nDen as denominator
 // Negative values in the denominator are invalid and cause the
 // inversion of both nominator and denominator signs
 // in order to return the correct value.
-Fraction::Fraction( sal_Int64 nNum, sal_Int64 nDen ) : mpImpl(new Impl)
+Fraction::Fraction( sal_Int64 nNum, sal_Int64 nDen ) : mnNumerator(nNum), mnDenominator(nDen)
 {
     assert( nNum >= std::numeric_limits<sal_Int32>::min() );
     assert( nNum <= std::numeric_limits<sal_Int32>::max( ));
@@ -80,18 +56,16 @@ Fraction::Fraction( sal_Int64 nNum, sal_Int64 nDen ) : mpImpl(new Impl)
     assert( nDen <= std::numeric_limits<sal_Int32>::max( ));
     if ( nDen == 0 )
     {
-        mpImpl->valid = false;
+        mbValid = false;
         SAL_WARN( "tools.fraction", "'Fraction(" << nNum << ",0)' invalid fraction created" );
         return;
     }
-    mpImpl->value.assign( nNum, nDen);
-    mpImpl->valid = true;
 }
 
 /**
  * only here to prevent passing of NaN
  */
-Fraction::Fraction( double nNum, double nDen ) : mpImpl(new Impl)
+Fraction::Fraction( double nNum, double nDen ) : mnNumerator(sal_Int64(nNum)), mnDenominator(sal_Int64(nDen))
 {
     assert( !std::isnan(nNum) );
     assert( !std::isnan(nDen) );
@@ -101,41 +75,36 @@ Fraction::Fraction( double nNum, double nDen ) : mpImpl(new Impl)
     assert( nDen <= std::numeric_limits<sal_Int32>::max( ));
     if ( nDen == 0 )
     {
-        mpImpl->valid = false;
+        mbValid = false;
         SAL_WARN( "tools.fraction", "'Fraction(" << nNum << ",0)' invalid fraction created" );
         return;
     }
-    mpImpl->value.assign( sal_Int64(nNum), sal_Int64(nDen));
-    mpImpl->valid = true;
 }
 
-Fraction::Fraction( double dVal ) : mpImpl(new Impl)
+Fraction::Fraction( double dVal )
 {
     try
     {
-        mpImpl->value = rational_FromDouble( dVal );
-        mpImpl->valid = true;
+        boost::rational<sal_Int32> v = rational_FromDouble( dVal );
+        mnNumerator = v.numerator();
+        mnDenominator = v.denominator();
     }
     catch (const boost::bad_rational&)
     {
-        mpImpl->valid = false;
+        mbValid = false;
         SAL_WARN( "tools.fraction", "'Fraction(" << dVal << ")' invalid fraction created" );
     }
 }
 
-Fraction::~Fraction()
-{
-}
-
 Fraction::operator double() const
 {
-    if (!mpImpl->valid)
+    if (!mbValid)
     {
         SAL_WARN( "tools.fraction", "'double()' on invalid fraction" );
         return 0.0;
     }
 
-    return boost::rational_cast<double>(mpImpl->value);
+    return boost::rational_cast<double>(toRational(mnNumerator, mnDenominator));
 }
 
 // This methods first validates both values.
@@ -144,32 +113,38 @@ Fraction::operator double() const
 // which cause the operation to be marked as invalid
 Fraction& Fraction::operator += ( const Fraction& rVal )
 {
-    if ( !rVal.mpImpl->valid )
-        mpImpl->valid = false;
+    if ( !rVal.mbValid )
+        mbValid = false;
 
-    if ( !mpImpl->valid )
+    if ( !mbValid )
     {
         SAL_WARN( "tools.fraction", "'operator +=' with invalid fraction" );
         return *this;
     }
 
-    mpImpl->value += rVal.mpImpl->value;
+    boost::rational<sal_Int32> a = toRational(mnNumerator, mnDenominator);
+    a += toRational(rVal.mnNumerator, rVal.mnDenominator);
+    mnNumerator = a.numerator();
+    mnDenominator = a.denominator();
 
     return *this;
 }
 
 Fraction& Fraction::operator -= ( const Fraction& rVal )
 {
-    if ( !rVal.mpImpl->valid )
-        mpImpl->valid = false;
+    if ( !rVal.mbValid )
+        mbValid = false;
 
-    if ( !mpImpl->valid )
+    if ( !mbValid )
     {
         SAL_WARN( "tools.fraction", "'operator -=' with invalid fraction" );
         return *this;
     }
 
-    mpImpl->value -= rVal.mpImpl->value;
+    boost::rational<sal_Int32> a = toRational(mnNumerator, mnDenominator);
+    a -= toRational(rVal.mnNumerator, rVal.mnDenominator);
+    mnNumerator = a.numerator();
+    mnDenominator = a.denominator();
 
     return *this;
 }
@@ -204,20 +179,24 @@ namespace
 
 Fraction& Fraction::operator *= ( const Fraction& rVal )
 {
-    if ( !rVal.mpImpl->valid )
-        mpImpl->valid = false;
+    if ( !rVal.mbValid )
+        mbValid = false;
 
-    if ( !mpImpl->valid )
+    if ( !mbValid )
     {
         SAL_WARN( "tools.fraction", "'operator *=' with invalid fraction" );
         return *this;
     }
 
-    bool bFail = checked_multiply_by(mpImpl->value, rVal.mpImpl->value);
+    boost::rational<sal_Int32> a = toRational(mnNumerator, mnDenominator);
+    boost::rational<sal_Int32> b = toRational(rVal.mnNumerator, rVal.mnDenominator);
+    bool bFail = checked_multiply_by(a, b);
+    mnNumerator = a.numerator();
+    mnDenominator = a.denominator();
 
     if (bFail)
     {
-        mpImpl->valid = false;
+        mbValid = false;
     }
 
     return *this;
@@ -225,16 +204,19 @@ Fraction& Fraction::operator *= ( const Fraction& rVal )
 
 Fraction& Fraction::operator /= ( const Fraction& rVal )
 {
-    if ( !rVal.mpImpl->valid )
-        mpImpl->valid = false;
+    if ( !rVal.mbValid )
+        mbValid = false;
 
-    if ( !mpImpl->valid )
+    if ( !mbValid )
     {
         SAL_WARN( "tools.fraction", "'operator /=' with invalid fraction" );
         return *this;
     }
 
-    mpImpl->value /= rVal.mpImpl->value;
+    boost::rational<sal_Int32> a = toRational(mnNumerator, mnDenominator);
+    a /= toRational(rVal.mnNumerator, rVal.mnDenominator);
+    mnNumerator = a.numerator();
+    mnDenominator = a.denominator();
 
     return *this;
 }
@@ -259,67 +241,49 @@ Fraction& Fraction::operator /= ( const Fraction& rVal )
 */
 void Fraction::ReduceInaccurate( unsigned nSignificantBits )
 {
-    if ( !mpImpl->valid )
+    if ( !mbValid )
     {
         SAL_WARN( "tools.fraction", "'ReduceInaccurate' on invalid fraction" );
         return;
     }
 
-    if ( !mpImpl->value.numerator() )
+    if ( !mnNumerator )
         return;
 
-    rational_ReduceInaccurate(mpImpl->value, nSignificantBits);
+    auto a = toRational(mnNumerator, mnDenominator);
+    rational_ReduceInaccurate(a, nSignificantBits);
+    mnNumerator = a.numerator();
+    mnDenominator = a.denominator();
 }
 
 sal_Int32 Fraction::GetNumerator() const
 {
-    if ( !mpImpl->valid )
+    if ( !mbValid )
     {
         SAL_WARN( "tools.fraction", "'GetNumerator()' on invalid fraction" );
         return 0;
     }
-    return mpImpl->value.numerator();
+    return mnNumerator;
 }
 
 sal_Int32 Fraction::GetDenominator() const
 {
-    if ( !mpImpl->valid )
+    if ( !mbValid )
     {
         SAL_WARN( "tools.fraction", "'GetDenominator()' on invalid fraction" );
         return -1;
     }
-    return mpImpl->value.denominator();
-}
-
-Fraction& Fraction::operator=( const Fraction& rFrac )
-{
-    if (this == &rFrac)
-        return *this;
-
-    Fraction tmp(rFrac);
-    std::swap(mpImpl, tmp.mpImpl);
-    return *this;
-}
-
-Fraction& Fraction::operator=( Fraction&& rFrac )
-{
-    mpImpl = std::move(rFrac.mpImpl);
-    return *this;
-}
-
-bool Fraction::IsValid() const
-{
-    return mpImpl->valid;
+    return mnDenominator;
 }
 
 Fraction::operator sal_Int32() const
 {
-    if ( !mpImpl->valid )
+    if ( !mbValid )
     {
         SAL_WARN( "tools.fraction", "'operator sal_Int32()' on invalid fraction" );
         return 0;
     }
-    return boost::rational_cast<sal_Int32>(mpImpl->value);
+    return boost::rational_cast<sal_Int32>(toRational(mnNumerator, mnDenominator));
 }
 
 Fraction operator+( const Fraction& rVal1, const Fraction& rVal2 )
@@ -367,38 +331,38 @@ bool operator >=( const Fraction& rVal1, const Fraction& rVal2 )
 
 bool operator == ( const Fraction& rVal1, const Fraction& rVal2 )
 {
-    if ( !rVal1.mpImpl->valid || !rVal2.mpImpl->valid )
+    if ( !rVal1.mbValid || !rVal2.mbValid )
     {
         SAL_WARN( "tools.fraction", "'operator ==' with an invalid fraction" );
         return false;
     }
 
-    return rVal1.mpImpl->value == rVal2.mpImpl->value;
+    return toRational(rVal1.mnNumerator, rVal1.mnDenominator) == toRational(rVal2.mnNumerator, rVal2.mnDenominator);
 }
 
 bool operator < ( const Fraction& rVal1, const Fraction& rVal2 )
 {
-    if ( !rVal1.mpImpl->valid || !rVal2.mpImpl->valid )
+    if ( !rVal1.mbValid || !rVal2.mbValid )
     {
         SAL_WARN( "tools.fraction", "'operator <' with an invalid fraction" );
         return false;
     }
 
-    return rVal1.mpImpl->value < rVal2.mpImpl->value;
+    return toRational(rVal1.mnNumerator, rVal1.mnDenominator) < toRational(rVal2.mnNumerator, rVal2.mnDenominator);
 }
 
 bool operator > ( const Fraction& rVal1, const Fraction& rVal2 )
 {
-    if ( !rVal1.mpImpl->valid || !rVal2.mpImpl->valid )
+    if ( !rVal1.mbValid || !rVal2.mbValid )
     {
         SAL_WARN( "tools.fraction", "'operator >' with an invalid fraction" );
         return false;
     }
 
-    return rVal1.mpImpl->value > rVal2.mpImpl->value;
+    return toRational(rVal1.mnNumerator, rVal1.mnDenominator) > toRational(rVal2.mnNumerator, rVal2.mnDenominator);
 }
 
-SvStream& ReadFraction( SvStream& rIStream, Fraction const & rFract )
+SvStream& ReadFraction( SvStream& rIStream, Fraction & rFract )
 {
     sal_Int32 num(0), den(0);
     rIStream.ReadInt32( num );
@@ -406,26 +370,27 @@ SvStream& ReadFraction( SvStream& rIStream, Fraction const & rFract )
     if ( den <= 0 )
     {
         SAL_WARN( "tools.fraction", "'ReadFraction()' read an invalid fraction" );
-        rFract.mpImpl->valid = false;
+        rFract.mbValid = false;
     }
     else
     {
-        rFract.mpImpl->value.assign( num, den );
-        rFract.mpImpl->valid = true;
+        rFract.mnNumerator = num;
+        rFract.mnDenominator = den;
+        rFract.mbValid = true;
     }
     return rIStream;
 }
 
 SvStream& WriteFraction( SvStream& rOStream, const Fraction& rFract )
 {
-    if ( !rFract.mpImpl->valid )
+    if ( !rFract.mbValid )
     {
         SAL_WARN( "tools.fraction", "'WriteFraction()' write an invalid fraction" );
         rOStream.WriteInt32( 0 );
         rOStream.WriteInt32( -1 );
     } else {
-        rOStream.WriteInt32( rFract.mpImpl->value.numerator() );
-        rOStream.WriteInt32( rFract.mpImpl->value.denominator() );
+        rOStream.WriteInt32( rFract.mnNumerator );
+        rOStream.WriteInt32( rFract.mnDenominator );
     }
     return rOStream;
 }