include/tools/fract.hxx | 22 ++---
tools/source/generic/fract.cxx | 173 ++++++++++++++++-------------------------
2 files changed, 80 insertions(+), 115 deletions(-)
New commits:
commit 31589bf0239679d73417902655045c48c4868016
Author: Noel Grandin <noel.gran...@collabora.co.uk>
AuthorDate: Mon Jun 24 15:02:55 2019 +0200
Commit: Noel Grandin <noel.gran...@collabora.co.uk>
CommitDate: Tue Jun 25 13:20:51 2019 +0200
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.gran...@collabora.co.uk>
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;
}
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
