sc/inc/queryiter.hxx | 6 +++- sc/source/core/data/queryiter.cxx | 52 +++++++++++++++++++++++++++++--------- 2 files changed, 45 insertions(+), 13 deletions(-)
New commits: commit 248e49d3f409d414331945ba91b3083406d59f78 Author: Luboš Luňák <l.lu...@collabora.com> AuthorDate: Tue May 10 12:49:55 2022 +0200 Commit: Luboš Luňák <l.lu...@collabora.com> CommitDate: Wed May 11 11:51:38 2022 +0200 no PerformQuery() with ScSortedRangeCache if not needed (tdf#144777) It should not be necessary in that case, since the rows are restricted only to those that match, selected by BinarySearch(). Still at least assert that, just in case I'm missing something (or BinarySearch() doesn't quite match what validQuery() does). Change-Id: Ie91d3ba997692e5b3650b1549e8a38b6c5f44c01 Reviewed-on: https://gerrit.libreoffice.org/c/core/+/134130 Tested-by: Jenkins Reviewed-by: Luboš Luňák <l.lu...@collabora.com> diff --git a/sc/inc/queryiter.hxx b/sc/inc/queryiter.hxx index 7c454f784a2e..fa8f08083981 100644 --- a/sc/inc/queryiter.hxx +++ b/sc/inc/queryiter.hxx @@ -94,12 +94,15 @@ class ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache { public: void SetSortedRangeCache( const ScSortedRangeCache& cache ); + template<bool fast> + bool IncPosImpl(); protected: ScQueryCellIteratorAccessSpecific( ScDocument& rDocument, ScInterpreterContext& rContext, const ScQueryParam& rParam ); void InitPosStart(); void InitPosFinish( SCROW beforeRow, SCROW lastRow ); - void IncPos(); + void IncPos() { IncPosImpl<false>(); } + bool IncPosFast() { return IncPosImpl<true>(); } void IncBlock() { IncPos(); } // Cannot skip entire block, not linear. // These members needs to be available already in the base class. @@ -111,6 +114,7 @@ protected: SCTAB nTab; SCCOL nCol; SCROW nRow; + const ScColumn* pColumn; // matching nCol, set by InitPos() const ScSortedRangeCache* sortedCache; size_t sortedCachePos; diff --git a/sc/source/core/data/queryiter.cxx b/sc/source/core/data/queryiter.cxx index 8a7bf2ce6e5b..656fedc3c717 100644 --- a/sc/source/core/data/queryiter.cxx +++ b/sc/source/core/data/queryiter.cxx @@ -1083,7 +1083,7 @@ void ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache > void ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache >::InitPosFinish( SCROW beforeRow, SCROW lastRow ) { - const ScColumn& rCol = rDoc.maTabs[nTab]->CreateColumnIfNotExists(nCol); + pColumn = &rDoc.maTabs[nTab]->CreateColumnIfNotExists(nCol); if(lastRow >= 0) { sortedCachePos = beforeRow >= 0 ? sortedCache->indexForRow(beforeRow) + 1 : 0; @@ -1091,35 +1091,43 @@ void ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache > if(sortedCachePos <= sortedCachePosLast) { nRow = sortedCache->rowForIndex(sortedCachePos); - maCurPos = rCol.maCells.position(nRow); + maCurPos = pColumn->maCells.position(nRow); return; } } // No rows, set to end. sortedCachePos = sortedCachePosLast = 0; - maCurPos.first = rCol.maCells.end(); + maCurPos.first = pColumn->maCells.end(); maCurPos.second = 0; } -void ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache >::IncPos() +template<bool fast> +bool ScQueryCellIteratorAccessSpecific< ScQueryCellIteratorAccess::SortedCache >::IncPosImpl() { - const ScColumn& rCol = rDoc.maTabs[nTab]->aCol[nCol]; if(sortedCachePos < sortedCachePosLast) { ++sortedCachePos; nRow = sortedCache->rowForIndex(sortedCachePos); - // Avoid mdds position() call if row is in the same block. - if(maCurPos.first != rCol.maCells.end() && o3tl::make_unsigned(nRow) >= maCurPos.first->position - && o3tl::make_unsigned(nRow) < maCurPos.first->position + maCurPos.first->size) - maCurPos.second = nRow - maCurPos.first->position; - else - maCurPos = rCol.maCells.position(nRow); +#ifndef DBG_UTIL + if constexpr (!fast) +#endif + { + // Avoid mdds position() call if row is in the same block. + if(maCurPos.first != pColumn->maCells.end() && o3tl::make_unsigned(nRow) >= maCurPos.first->position + && o3tl::make_unsigned(nRow) < maCurPos.first->position + maCurPos.first->size) + maCurPos.second = nRow - maCurPos.first->position; + else + maCurPos = pColumn->maCells.position(nRow); + } + return true; } else { // This will make PerformQuery() go to next column. - maCurPos.first = rCol.maCells.end(); + // Necessary even in fast mode, as GetNext() will call GetThis() in this case. + maCurPos.first = pColumn->maCells.end(); maCurPos.second = 0; + return false; } } @@ -1281,6 +1289,26 @@ bool ScQueryCellIterator< accessType >::GetNext() return GetThis(); } +template<> +bool ScQueryCellIterator< ScQueryCellIteratorAccess::SortedCache >::GetNext() +{ + assert( !nStopOnMismatch ); + assert( !nTestEqualCondition ); + // When searching using sorted cache, we should always find cells that match, + // because InitPos()/IncPos() select only such rows, so skip GetThis() (and thus + // the somewhat expensive PerformQuery) as long as we're not at the end + // of a column. As an optimization IncPosFast() returns true if not at the end, + // in which case in non-DBG_UTIL mode it doesn't even bother to set maCurPos. + if( IncPosFast()) + { +#ifdef DBG_UTIL + assert(GetThis()); +#endif + return true; + } + return GetThis(); +} + bool ScQueryCellIteratorSortedCache::CanBeUsed(const ScDocument& rDoc, const ScQueryParam& rParam, const ScFormulaCell* cell, const ScComplexRefData* refData) {