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)
 {

Reply via email to