From 4f29a7766edd19757a8ab2e38bf7fdff36c704b7 Mon Sep 17 00:00:00 2001 From: Markus Mohrhard Date: Mon, 22 Dec 2014 00:14:35 +0100 Subject: improve performance of some matrix operations, related fdo#83187 I0e6816a7f0d2dc051dff6a462724cb4a3c155289 fix error in last commit Icafbe6e5daab64e7431d80c8956143341eb2ef0b fix a few problems with my matrix commit I6e3fdf4bd26c952a59ad130dc6e5c9d1f3ff5f07 coverity#1260446 Uninitialized scalar field and coverity#1260447 Uninitialized scalar field I3aa5a1caf776fddc8b6029e96c24aa86b21de880 iterator::operator*() should return a reference Change-Id: Id09f555c5ece9e5cb60a2ae7bc2456d4343744f5 Reviewed-on: https://gerrit.libreoffice.org/13676 Reviewed-by: Eike Rathke Tested-by: Eike Rathke --- sc/inc/scmatrix.hxx | 2 + sc/source/core/tool/interpr5.cxx | 21 +-- sc/source/core/tool/scmatrix.cxx | 310 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 317 insertions(+), 16 deletions(-) diff --git a/sc/inc/scmatrix.hxx b/sc/inc/scmatrix.hxx index 851caa474373..6bcd8857f1e0 100644 --- a/sc/inc/scmatrix.hxx +++ b/sc/inc/scmatrix.hxx @@ -385,6 +385,8 @@ public: void GetDoubleArray( std::vector& rArray, bool bEmptyAsZero = true ) const; void MergeDoubleArray( std::vector& rArray, Op eOp ) const; + void SubAddOp(bool bSub, double fVal, svl::SharedString aString, ScMatrix& rMat); + ScMatrix& operator+= ( const ScMatrix& r ); #if DEBUG_MATRIX diff --git a/sc/source/core/tool/interpr5.cxx b/sc/source/core/tool/interpr5.cxx index e33484658837..0d5f907cddb8 100644 --- a/sc/source/core/tool/interpr5.cxx +++ b/sc/source/core/tool/interpr5.cxx @@ -1188,6 +1188,7 @@ void ScInterpreter::ScAdd() { CalculateAddSub(false); } + void ScInterpreter::CalculateAddSub(bool _bSub) { ScMatrixRef pMat1 = NULL; @@ -1275,29 +1276,17 @@ void ScInterpreter::CalculateAddSub(bool _bSub) } SCSIZE nC, nR; pMat->GetDimensions(nC, nR); - ScMatrixRef pResMat = GetNewMat(nC, nR); + ScMatrixRef pResMat = GetNewMat(nC, nR, true); if (pResMat) { - SCSIZE nCount = nC * nR; + svl::SharedString aString = mrStrPool.intern(ScGlobal::GetRscString(STR_NO_VALUE)); if (bFlag || !_bSub ) { - for ( SCSIZE i = 0; i < nCount; i++ ) - { - if (pMat->IsValue(i)) - pResMat->PutDouble( _bSub ? ::rtl::math::approxSub( fVal, pMat->GetDouble(i)) : ::rtl::math::approxAdd( pMat->GetDouble(i), fVal), i); - else - pResMat->PutString(mrStrPool.intern(ScGlobal::GetRscString(STR_NO_VALUE)), i); - } + pMat->SubAddOp(_bSub, fVal, aString, *pResMat); } else { - for ( SCSIZE i = 0; i < nCount; i++ ) - { - if (pMat->IsValue(i)) - pResMat->PutDouble( ::rtl::math::approxSub( pMat->GetDouble(i), fVal), i); - else - pResMat->PutString(mrStrPool.intern(ScGlobal::GetRscString(STR_NO_VALUE)), i); - } + pMat->SubAddOp(true, fVal, aString, *pResMat); } PushMatrix(pResMat); } diff --git a/sc/source/core/tool/scmatrix.cxx b/sc/source/core/tool/scmatrix.cxx index 429fd5faee04..b111ddf2d652 100644 --- a/sc/source/core/tool/scmatrix.cxx +++ b/sc/source/core/tool/scmatrix.cxx @@ -295,6 +295,9 @@ public: void MergeDoubleArray( std::vector& rArray, ScMatrix::Op eOp ) const; void AddValues( const ScMatrixImpl& rMat ); + template + void ApplyOperation(T aOp, ScMatrixImpl& rMat); + #if DEBUG_MATRIX void Dump() const; #endif @@ -1858,6 +1861,215 @@ void ScMatrixImpl::AddValues( const ScMatrixImpl& rMat ) } } +namespace Op { + +template +struct return_type +{ + typedef T type; +}; + +template<> +struct return_type +{ + typedef double type; +}; + +template<> +struct return_type +{ + typedef svl::SharedString type; +}; + +} + +template +struct wrapped_iterator +{ + typedef ::std::bidirectional_iterator_tag iterator_category; + typedef typename T::const_iterator::value_type old_value_type; + typedef typename Op::return_type::type value_type; + typedef value_type* pointer; + typedef value_type& reference; + typedef typename T::const_iterator::difference_type difference_type; + + typename T::const_iterator it; + mutable value_type val; + U maOp; + +private: + + value_type calcVal() const + { + return maOp(*it); + } + +public: + + wrapped_iterator(typename T::const_iterator it_, U aOp): + it(it_), + val(value_type()), + maOp(aOp) + { + } + + wrapped_iterator(const wrapped_iterator& r): + it(r.it), + val(r.val), + maOp(r.maOp) + { + } + + wrapped_iterator& operator=(const wrapped_iterator& r) + { + it = r.it; + return *this; + } + + bool operator==(const wrapped_iterator& r) const + { + return it == r.it; + } + + bool operator!=(const wrapped_iterator& r) const + { + return !operator==(r); + } + + wrapped_iterator& operator++() + { + ++it; + + return *this; + } + + wrapped_iterator& operator--() + { + --it; + + return *this; + } + + value_type& operator*() const + { + val = calcVal(); + return val; + } + + pointer operator->() const + { + val = calcVal(); + return &val; + } +}; + +template +struct MatrixIteratorWrapper +{ +private: + typename T::const_iterator m_itBegin; + typename T::const_iterator m_itEnd; + U maOp; +public: + MatrixIteratorWrapper(typename T::const_iterator itBegin, typename T::const_iterator itEnd, U aOp): + m_itBegin(itBegin), + m_itEnd(itEnd), + maOp(aOp) + { + } + + wrapped_iterator begin() + { + return wrapped_iterator(m_itBegin, maOp); + } + + wrapped_iterator end() + { + return wrapped_iterator(m_itEnd, maOp); + } +}; + +template +struct MatrixOpWrapper +{ +private: + MatrixImplType& mrMat; + MatrixImplType::position_type pos; + T maOp; + +public: + MatrixOpWrapper(MatrixImplType& rMat, T aOp): + mrMat(rMat), + pos(rMat.position(0,0)), + maOp(aOp) + { + } + + void operator()(const MatrixImplType::element_block_node_type& node) + { + switch (node.type) + { + case mdds::mtm::element_numeric: + { + typedef MatrixImplType::numeric_block_type block_type; + + block_type::const_iterator it = block_type::begin(*node.data); + block_type::const_iterator itEnd = block_type::end(*node.data); + MatrixIteratorWrapper aFunc(it, itEnd, maOp); + pos = mrMat.set(pos,aFunc.begin(), aFunc.end()); + ++pos.first; + } + break; + case mdds::mtm::element_boolean: + { + typedef MatrixImplType::boolean_block_type block_type; + + block_type::const_iterator it = block_type::begin(*node.data); + block_type::const_iterator itEnd = block_type::end(*node.data); + + MatrixIteratorWrapper aFunc(it, itEnd, maOp); + pos = mrMat.set(pos, aFunc.begin(), aFunc.end()); + ++pos.first; + } + break; + case mdds::mtm::element_string: + { + typedef MatrixImplType::string_block_type block_type; + + block_type::const_iterator it = block_type::begin(*node.data); + block_type::const_iterator itEnd = block_type::end(*node.data); + + MatrixIteratorWrapper aFunc(it, itEnd, maOp); + pos = mrMat.set(pos, aFunc.begin(), aFunc.end()); + ++pos.first; + } + break; + case mdds::mtm::element_empty: + { + if (maOp.useFunctionForEmpty()) + { + std::vector aVec(node.size); + MatrixIteratorWrapper, T> aFunc(aVec.begin(), aVec.end(), maOp); + pos = mrMat.set(pos, aFunc.begin(), aFunc.end()); + ++pos.first; + } + else + pos.second += node.size; + } + break; + default: + ; + } + } +}; + +template +void ScMatrixImpl::ApplyOperation(T aOp, ScMatrixImpl& rMat) +{ + MatrixOpWrapper aFunc(rMat.maMat, aOp); + maMat.walk(aFunc); +} + #if DEBUG_MATRIX void ScMatrixImpl::Dump() const { @@ -2283,6 +2495,104 @@ void ScMatrix::MergeDoubleArray( std::vector& rArray, Op eOp ) const pImpl->MergeDoubleArray(rArray, eOp); } +namespace { + +struct AddOp +{ +private: + double mnVal; + svl::SharedString maString; + +public: + + AddOp(double nVal, svl::SharedString aString): + mnVal(nVal), + maString(aString) + { + } + + double operator()(double nVal) const + { + return nVal + mnVal; + } + + double operator()(bool bVal) const + { + return mnVal + (double)bVal; + } + + svl::SharedString operator()(const svl::SharedString&) const + { + return maString; + } + + svl::SharedString operator()(char) const + { + return maString; + } + + bool useFunctionForEmpty() const + { + return true; + } +}; + +struct SubOp +{ +private: + double mnVal; + svl::SharedString maString; + +public: + + SubOp(double nVal, svl::SharedString aString): + mnVal(nVal), + maString(aString) + { + } + + double operator()(double nVal) const + { + return nVal - mnVal; + } + + double operator()(bool bVal) const + { + return (double)bVal - mnVal; + } + + svl::SharedString operator()(const svl::SharedString&) const + { + return maString; + } + + svl::SharedString operator()(char) const + { + return maString; + } + + bool useFunctionForEmpty() const + { + return true; + } +}; + +} + +void ScMatrix::SubAddOp(bool bSub, double fVal, svl::SharedString aString, ScMatrix& rMat) +{ + if(bSub) + { + SubOp aOp(fVal, aString); + pImpl->ApplyOperation(aOp, *rMat.pImpl); + } + else + { + AddOp aOp(fVal, aString); + pImpl->ApplyOperation(aOp, *rMat.pImpl); + } +} + ScMatrix& ScMatrix::operator+= ( const ScMatrix& r ) { pImpl->AddValues(*r.pImpl); -- cgit v1.2.3