sc/inc/queryiter.hxx | 7 +++++--
sc/source/core/data/queryiter.cxx | 33 ++++++++++++++++++++++++++++-----
sc/source/core/inc/interpre.hxx | 4 +++-
sc/source/core/tool/interpr1.cxx | 29 ++++++++++++++++++++---------
sc/source/core/tool/interpr4.cxx | 23 +++++++++++++++++++++++
5 files changed, 79 insertions(+), 17 deletions(-)
New commits:
commit 4c8f4390c846e0184955916b0555359a6ad98981
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Tue May 10 10:54:26 2022 +0200
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Wed May 11 11:51:16 2022 +0200
do not use ScSortedRangeCache inefficiently
If the amount of data or size of the formula group is small, the caching
is probably not worth it. And if rows are not absolute, then each
formula cell would use a different range instead of reusing the cache.
Change-Id: I42a82aaa5f4ffccc63e527793f46fc52fd539af6
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/134129
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 545955d77963..7c454f784a2e 100644
--- a/sc/inc/queryiter.hxx
+++ b/sc/inc/queryiter.hxx
@@ -26,6 +26,7 @@
#include "mtvelements.hxx"
#include "types.hxx"
+struct ScComplexRefData;
class ScSortedRangeCache;
/*
@@ -318,7 +319,8 @@ public:
SCTAB nTable, const ScQueryParam& aParam, bool bMod)
: Base( rDocument, rContext, nTable, aParam, bMod ) {}
// Returns true if this iterator can be used for the given query.
- static bool CanBeUsed(const ScDocument& rDoc, const ScQueryParam& aParam);
+ static bool CanBeUsed(const ScDocument& rDoc, const ScQueryParam& aParam,
+ const ScFormulaCell* cell, const ScComplexRefData* refData);
};
@@ -366,7 +368,8 @@ public:
SCTAB nTable, const ScQueryParam& aParam, bool bMod)
: Base( rDocument, rContext, nTable, aParam, bMod ) {}
// Returns true if this iterator can be used for the given query.
- static bool CanBeUsed(const ScDocument& rDoc, const ScQueryParam& aParam);
+ static bool CanBeUsed(const ScDocument& rDoc, const ScQueryParam& aParam,
+ const ScFormulaCell* cell, const ScComplexRefData* refData);
};
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/data/queryiter.cxx
b/sc/source/core/data/queryiter.cxx
index ffb37c0e272f..8a7bf2ce6e5b 100644
--- a/sc/source/core/data/queryiter.cxx
+++ b/sc/source/core/data/queryiter.cxx
@@ -1198,7 +1198,8 @@ ScQueryCellIteratorAccessSpecific<
ScQueryCellIteratorAccess::SortedCache >::Mak
return SortedCacheIndexer(rCells, nStartRow, nEndRow, sortedCache);
}
-static bool CanBeUsedForSorterCache(const ScDocument& rDoc, const
ScQueryParam& rParam)
+static bool CanBeUsedForSorterCache(const ScDocument& rDoc, const
ScQueryParam& rParam,
+ const ScFormulaCell* cell, const ScComplexRefData* refData)
{
if(!rParam.GetEntry(0).bDoQuery || rParam.GetEntry(1).bDoQuery
|| rParam.GetEntry(0).GetQueryItems().size() != 1 )
@@ -1221,6 +1222,26 @@ static bool CanBeUsedForSorterCache(const ScDocument&
rDoc, const ScQueryParam&
&& rParam.GetEntry(0).eOp != SC_GREATER && rParam.GetEntry(0).eOp !=
SC_GREATER_EQUAL
&& rParam.GetEntry(0).eOp != SC_EQUAL)
return false;
+ // For unittests allow inefficient caching, in order for the code to be
checked.
+ static bool inUnitTest = getenv("LO_TESTNAME") != nullptr;
+ if(refData == nullptr || refData->Ref1.IsRowRel() ||
refData->Ref2.IsRowRel())
+ {
+ // If this is not a range, then a cache is not worth it. If rows are
relative, then each
+ // computation will use a different area, so the cache wouldn't be
reused. Tab/cols are
+ // not a problem, because formula group computations are done for the
same tab/col.
+ if(!inUnitTest)
+ return false;
+ }
+ if(rParam.nRow2 - rParam.nRow1 < 10)
+ {
+ if(!inUnitTest)
+ return false;
+ }
+ if( !cell || !cell->GetCellGroup() || cell->GetCellGroup()->mnLength < 10 )
+ {
+ if(!inUnitTest)
+ return false;
+ }
return true;
}
@@ -1260,9 +1281,10 @@ bool ScQueryCellIterator< accessType >::GetNext()
return GetThis();
}
-bool ScQueryCellIteratorSortedCache::CanBeUsed(const ScDocument& rDoc, const
ScQueryParam& rParam)
+bool ScQueryCellIteratorSortedCache::CanBeUsed(const ScDocument& rDoc, const
ScQueryParam& rParam,
+ const ScFormulaCell* cell, const ScComplexRefData* refData)
{
- return CanBeUsedForSorterCache(rDoc, rParam);
+ return CanBeUsedForSorterCache(rDoc, rParam, cell, refData);
}
// Countifs implementation.
@@ -1289,9 +1311,10 @@ sal_uInt64 ScCountIfCellIterator< accessType
>::GetCount()
}
-bool ScCountIfCellIteratorSortedCache::CanBeUsed(const ScDocument& rDoc, const
ScQueryParam& rParam)
+bool ScCountIfCellIteratorSortedCache::CanBeUsed(const ScDocument& rDoc, const
ScQueryParam& rParam,
+ const ScFormulaCell* cell, const ScComplexRefData* refData)
{
- return CanBeUsedForSorterCache(rDoc, rParam);
+ return CanBeUsedForSorterCache(rDoc, rParam, cell, refData);
}
template<>
diff --git a/sc/source/core/inc/interpre.hxx b/sc/source/core/inc/interpre.hxx
index 47763ed8fea4..30e79d9272bb 100644
--- a/sc/source/core/inc/interpre.hxx
+++ b/sc/source/core/inc/interpre.hxx
@@ -345,6 +345,8 @@ private:
ScDBRangeBase* PopDBDoubleRef();
void PopDoubleRef(SCCOL& rCol1, SCROW &rRow1, SCTAB& rTab1,
SCCOL& rCol2, SCROW &rRow2, SCTAB& rTab2 );
+ // peek double ref data
+ const ScComplexRefData* GetStackDoubleRef(size_t rRefInList = 0);
void PopExternalSingleRef(sal_uInt16& rFileId, OUString& rTabName,
ScSingleRefData& rRef);
@@ -481,7 +483,7 @@ private:
inline void TreatDoubleError( double& rVal );
// Lookup using ScLookupCache, @returns true if found and result address
bool LookupQueryWithCache( ScAddress & o_rResultPos,
- const ScQueryParam & rParam ) const;
+ const ScQueryParam & rParam, const ScComplexRefData* refData )
const;
void ScIfJump();
void ScIfError( bool bNAonly );
diff --git a/sc/source/core/tool/interpr1.cxx b/sc/source/core/tool/interpr1.cxx
index e80b299f4771..afd9079de498 100644
--- a/sc/source/core/tool/interpr1.cxx
+++ b/sc/source/core/tool/interpr1.cxx
@@ -4839,6 +4839,7 @@ void ScInterpreter::ScMatch()
rParam.nRow1 = nRow1;
rParam.nCol2 = nCol2;
rParam.nTab = nTab1;
+ const ScComplexRefData* refData = nullptr;
ScQueryEntry& rEntry = rParam.GetEntry(0);
ScQueryEntry::Item& rItem = rEntry.GetQueryItem();
@@ -4863,6 +4864,8 @@ void ScInterpreter::ScMatch()
}
break;
case svDoubleRef :
+ refData = GetStackDoubleRef();
+ [[fallthrough]];
case svSingleRef :
{
ScAddress aAdr;
@@ -5037,7 +5040,7 @@ void ScInterpreter::ScMatch()
rParam.nRow2 = nRow2;
rEntry.nField = nCol1;
ScAddress aResultPos( nCol1, nRow1, nTab1);
- if (!LookupQueryWithCache( aResultPos, rParam))
+ if (!LookupQueryWithCache( aResultPos, rParam, refData))
{
PushNA();
return;
@@ -5689,6 +5692,7 @@ void ScInterpreter::ScCountIf()
SCROW nRow2 = 0;
SCTAB nTab2 = 0;
ScMatrixRef pQueryMatrix;
+ const ScComplexRefData* refData = nullptr;
switch ( GetStackType() )
{
case svRefList :
@@ -5696,6 +5700,7 @@ void ScInterpreter::ScCountIf()
[[fallthrough]];
case svDoubleRef :
{
+ refData = GetStackDoubleRef(nRefInList);
ScRange aRange;
PopDoubleRef( aRange, nParam, nRefInList);
aRange.GetVars( nCol1, nRow1, nTab1, nCol2, nRow2, nTab2);
@@ -5788,7 +5793,7 @@ void ScInterpreter::ScCountIf()
}
else
{
- if(ScCountIfCellIteratorSortedCache::CanBeUsed(mrDoc, rParam))
+ if(ScCountIfCellIteratorSortedCache::CanBeUsed(mrDoc, rParam,
pMyFormulaCell, refData))
{
ScCountIfCellIteratorSortedCache aCellIter(mrDoc,
mrContext, nTab1, rParam, false);
fCount += aCellIter.GetCount();
@@ -5979,6 +5984,7 @@ void ScInterpreter::IterateParametersIfs(
double(*ResultFunc)( const sc::ParamIf
ScMatrixRef pQueryMatrix;
while (nParam-- == nParamCount)
{
+ const ScComplexRefData* refData = nullptr;
switch ( GetStackType() )
{
case svRefList :
@@ -6015,12 +6021,14 @@ void ScInterpreter::IterateParametersIfs(
double(*ResultFunc)( const sc::ParamIf
}
nRefArrayPos = nRefInList;
}
+ refData = GetStackDoubleRef(nRefInList);
ScRange aRange;
PopDoubleRef( aRange, nParam, nRefInList);
aRange.GetVars( nCol1, nRow1, nTab1, nCol2, nRow2,
nTab2);
}
break;
case svDoubleRef :
+ refData = GetStackDoubleRef();
PopDoubleRef( nCol1, nRow1, nTab1, nCol2, nRow2, nTab2 );
break;
case svSingleRef :
@@ -6150,7 +6158,7 @@ void ScInterpreter::IterateParametersIfs(
double(*ResultFunc)( const sc::ParamIf
}
else
{
- if( ScQueryCellIteratorSortedCache::CanBeUsed( mrDoc, rParam ))
+ if( ScQueryCellIteratorSortedCache::CanBeUsed( mrDoc, rParam,
pMyFormulaCell, refData ))
{
ScQueryCellIteratorSortedCache aCellIter(mrDoc, mrContext,
nTab1, rParam, false);
// Increment Entry.nField in iterator when switching to
next column.
@@ -7250,9 +7258,11 @@ void ScInterpreter::CalculateLookup(bool bHLookup)
SCTAB nTab1 = 0;
SCCOL nCol2 = 0;
SCROW nRow2 = 0;
+ const ScComplexRefData* refData = nullptr;
StackVar eType = GetStackType();
if (eType == svDoubleRef)
{
+ refData = GetStackDoubleRef(0);
SCTAB nTab2;
PopDoubleRef(nCol1, nRow1, nTab1, nCol2, nRow2, nTab2);
if (nTab1 != nTab2)
@@ -7467,7 +7477,7 @@ void ScInterpreter::CalculateLookup(bool bHLookup)
else
{
ScAddress aResultPos( nCol1, nRow1, nTab1);
- bFound = LookupQueryWithCache( aResultPos, aParam);
+ bFound = LookupQueryWithCache( aResultPos, aParam, refData);
nRow = aResultPos.Row();
nCol = nSpIndex;
}
@@ -9967,7 +9977,8 @@ utl::SearchParam::SearchType
ScInterpreter::DetectSearchType( std::u16string_vie
}
static bool lcl_LookupQuery( ScAddress & o_rResultPos, ScDocument& rDoc,
ScInterpreterContext& rContext,
- const ScQueryParam & rParam, const ScQueryEntry & rEntry )
+ const ScQueryParam & rParam, const ScQueryEntry & rEntry, const
ScFormulaCell* cell,
+ const ScComplexRefData* refData )
{
if (rEntry.eOp != SC_EQUAL)
{
@@ -9984,7 +9995,7 @@ static bool lcl_LookupQuery( ScAddress & o_rResultPos,
ScDocument& rDoc, ScInter
}
else // EQUAL
{
- if( ScQueryCellIteratorSortedCache::CanBeUsed( rDoc, rParam ))
+ if( ScQueryCellIteratorSortedCache::CanBeUsed( rDoc, rParam, cell,
refData ))
{
ScQueryCellIteratorSortedCache aCellIter( rDoc, rContext,
rParam.nTab, rParam, false);
if (aCellIter.GetFirst())
@@ -10047,7 +10058,7 @@ static SCROW lcl_getPrevRowWithEmptyValueLookup( const
ScLookupCache& rCache,
}
bool ScInterpreter::LookupQueryWithCache( ScAddress & o_rResultPos,
- const ScQueryParam & rParam ) const
+ const ScQueryParam & rParam, const ScComplexRefData* refData ) const
{
bool bFound = false;
const ScQueryEntry& rEntry = rParam.GetEntry(0);
@@ -10060,7 +10071,7 @@ bool ScInterpreter::LookupQueryWithCache( ScAddress &
o_rResultPos,
* direct lookups here. We could even further attribute volatility per
* parameter so it would affect only the lookup range parameter. */
if (!bColumnsMatch || GetVolatileType() != NOT_VOLATILE)
- bFound = lcl_LookupQuery( o_rResultPos, mrDoc, mrContext, rParam,
rEntry);
+ bFound = lcl_LookupQuery( o_rResultPos, mrDoc, mrContext, rParam,
rEntry, pMyFormulaCell, refData);
else
{
ScRange aLookupRange( rParam.nCol1, rParam.nRow1, rParam.nTab,
@@ -10091,7 +10102,7 @@ bool ScInterpreter::LookupQueryWithCache( ScAddress &
o_rResultPos,
{
case ScLookupCache::NOT_CACHED :
case ScLookupCache::CRITERIA_DIFFERENT :
- bFound = lcl_LookupQuery( o_rResultPos, mrDoc, mrContext,
rParam, rEntry);
+ bFound = lcl_LookupQuery( o_rResultPos, mrDoc, mrContext,
rParam, rEntry, pMyFormulaCell, refData);
if (eCacheResult == ScLookupCache::NOT_CACHED)
rCache.insert( o_rResultPos, aCriteria, aPos, bFound);
break;
diff --git a/sc/source/core/tool/interpr4.cxx b/sc/source/core/tool/interpr4.cxx
index 658cc2b5f9fd..540928f0bee6 100644
--- a/sc/source/core/tool/interpr4.cxx
+++ b/sc/source/core/tool/interpr4.cxx
@@ -1114,6 +1114,29 @@ void ScInterpreter::PopDoubleRef( ScRange& rRange, bool
bDontCheckForTableOp )
SetError( FormulaError::UnknownStackVariable);
}
+const ScComplexRefData* ScInterpreter::GetStackDoubleRef(size_t rRefInList)
+{
+ if( sp )
+ {
+ const FormulaToken* p = pStack[ sp - 1 ];
+ switch (p->GetType())
+ {
+ case svDoubleRef:
+ return p->GetDoubleRef();
+ case svRefList:
+ {
+ const ScRefList* pList = p->GetRefList();
+ if (rRefInList < pList->size())
+ return &(*pList)[rRefInList];
+ break;
+ }
+ default:
+ break;
+ }
+ }
+ return nullptr;
+}
+
void ScInterpreter::PopExternalSingleRef(sal_uInt16& rFileId, OUString&
rTabName, ScSingleRefData& rRef)
{
if (!sp)
