sc/inc/global.hxx | 1 -
sc/inc/table.hxx | 9 ++++++++-
sc/source/core/data/dociter.cxx | 6 ++++--
sc/source/core/data/global.cxx | 6 ++----
sc/source/core/data/table3.cxx | 28 +++++++++++++++++++++++-----
5 files changed, 37 insertions(+), 13 deletions(-)
New commits:
commit 1a32af4192291ea5cfe7c4c143144a5dab1f156e
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Mon Nov 22 13:16:43 2021 +0100
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Mon Nov 22 15:46:46 2021 +0100
cache error strings for ScTable::validQuery() (tdf#133835)
Avoid calling SharedStringPool::intern() on values that are
repeatedly the same.
Change-Id: I094f2e777a4ca24536e0c25e6a1c6358ccf49f61
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/125660
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index 40806e70f31a..332be67ee551 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -954,10 +954,17 @@ public:
void Reorder( const sc::ReorderParam& rParam );
+ // Internal cache that can be repeatedly used for a sequence of related
ValidQuery()
+ // calls (related meaning done in a loop for the same document and query
data).
+ struct ValidQueryCache
+ {
+ std::unordered_map<FormulaError, svl::SharedString>
mCachedSharedErrorStrings;
+ };
bool ValidQuery(
SCROW nRow, const ScQueryParam& rQueryParam, const ScRefCellValue*
pCell = nullptr,
bool* pbTestEqualCondition = nullptr, const ScInterpreterContext*
pContext = nullptr,
- sc::TableColumnBlockPositionSet* pBlockPos = nullptr );
+ sc::TableColumnBlockPositionSet* pBlockPos = nullptr,
+ ValidQueryCache* pQueryCache = nullptr );
void TopTenQuery( ScQueryParam& );
void PrepareQuery( ScQueryParam& rQueryParam );
SCSIZE Query(const ScQueryParam& rQueryParam, bool bKeepSub);
diff --git a/sc/source/core/data/dociter.cxx b/sc/source/core/data/dociter.cxx
index 378d867b4e55..91cc0a2c213c 100644
--- a/sc/source/core/data/dociter.cxx
+++ b/sc/source/core/data/dociter.cxx
@@ -1117,6 +1117,7 @@ bool ScQueryCellIterator::GetThis()
!maParam.bHasHeader && rItem.meType == ScQueryEntry::ByString &&
((maParam.bByRow && nRow == maParam.nRow1) ||
(!maParam.bByRow && nCol == maParam.nCol1));
+ ScTable::ValidQueryCache validQueryCache;
ScColumn* pCol = &(rDoc.maTabs[nTab])->aCol[nCol];
while (true)
@@ -1183,7 +1184,7 @@ bool ScQueryCellIterator::GetThis()
if ( rDoc.maTabs[nTab]->ValidQuery( nRow, maParam,
(nCol == static_cast<SCCOL>(nFirstQueryField) ? &aCell :
nullptr),
(nTestEqualCondition ? &bTestEqualCondition : nullptr),
- &mrContext) )
+ &mrContext, nullptr, &validQueryCache) )
{
if ( nTestEqualCondition && bTestEqualCondition )
nTestEqualCondition |= nTestEqualConditionMatched;
@@ -1506,6 +1507,7 @@ int ScCountIfCellIterator::GetCount()
const ScQueryEntry::Item& rItem = rEntry.GetQueryItem();
const bool bSingleQueryItem = rEntry.GetQueryItems().size() == 1;
int count = 0;
+ ScTable::ValidQueryCache validQueryCache;
ScColumn* pCol = &(rDoc.maTabs[nTab])->aCol[nCol];
while (true)
@@ -1561,7 +1563,7 @@ int ScCountIfCellIterator::GetCount()
if ( rDoc.maTabs[nTab]->ValidQuery( nRow, maParam,
(nCol == static_cast<SCCOL>(rEntry.nField) ? &aCell : nullptr),
nullptr,
- &mrContext) )
+ &mrContext, nullptr, &validQueryCache) )
{
if (aCell.isEmpty())
return count;
diff --git a/sc/source/core/data/table3.cxx b/sc/source/core/data/table3.cxx
index 0ef833511dba..6ff12c4a25b5 100644
--- a/sc/source/core/data/table3.cxx
+++ b/sc/source/core/data/table3.cxx
@@ -2330,6 +2330,7 @@ class QueryEvaluator
CollatorWrapper* mpCollator;
const bool mbMatchWholeCell;
const bool mbCaseSensitive;
+ ScTable::ValidQueryCache* mpValidQueryCache;
static bool isPartialTextMatchOp(const ScQueryEntry& rEntry)
{
@@ -2399,7 +2400,7 @@ class QueryEvaluator
public:
QueryEvaluator(ScDocument& rDoc, const ScTable& rTab, const ScQueryParam&
rParam,
- bool pTestEqualCondition) :
+ bool pTestEqualCondition, ScTable::ValidQueryCache*
pValidQueryCache) :
mrDoc(rDoc),
mrStrPool(rDoc.GetSharedStringPool()),
mrTab(rTab),
@@ -2408,7 +2409,8 @@ public:
mpTransliteration(nullptr),
mpCollator(nullptr),
mbMatchWholeCell(rDoc.GetDocOptions().IsMatchWholeCell()),
- mbCaseSensitive( rParam.bCaseSens )
+ mbCaseSensitive( rParam.bCaseSens ),
+ mpValidQueryCache( pValidQueryCache )
{
}
@@ -2595,6 +2597,20 @@ public:
if (rCell.meType == CELLTYPE_FORMULA &&
rCell.mpFormula->GetErrCode() != FormulaError::NONE)
{
// Error cell is evaluated as string (for now).
+ if(mpValidQueryCache)
+ {
+ const FormulaError error = rCell.mpFormula->GetErrCode();
+ auto it =
mpValidQueryCache->mCachedSharedErrorStrings.find(error);
+ if( it ==
mpValidQueryCache->mCachedSharedErrorStrings.end())
+ {
+ svl::SharedString str =
mrStrPool.intern(ScGlobal::GetErrorString(error));
+ auto pos =
mpValidQueryCache->mCachedSharedErrorStrings.insert({error, str});
+ assert(pos.second); // inserted
+ it = pos.first;
+ }
+ const svl::SharedString& sharedError = it->second;
+ return compareByStringComparator(rEntry, rItem,
&sharedError, nullptr);
+ }
const OUString aCellStr =
ScGlobal::GetErrorString(rCell.mpFormula->GetErrCode());
return compareByStringComparator(rEntry, rItem, nullptr,
&aCellStr);
}
@@ -2976,7 +2992,8 @@ public:
bool ScTable::ValidQuery(
SCROW nRow, const ScQueryParam& rParam, const ScRefCellValue* pCell, bool*
pbTestEqualCondition,
- const ScInterpreterContext* pContext, sc::TableColumnBlockPositionSet*
pBlockPos)
+ const ScInterpreterContext* pContext, sc::TableColumnBlockPositionSet*
pBlockPos,
+ ValidQueryCache* pValidQueryCache)
{
if (!rParam.GetEntry(0).bDoQuery)
return true;
@@ -2991,7 +3008,7 @@ bool ScTable::ValidQuery(
bool* pTest = ( nEntryCount <= nFixedBools ? &aTest[0] : new
bool[nEntryCount] );
tools::Long nPos = -1;
- QueryEvaluator aEval(rDocument, *this, rParam, pbTestEqualCondition !=
nullptr);
+ QueryEvaluator aEval(rDocument, *this, rParam, pbTestEqualCondition !=
nullptr, pValidQueryCache);
ScQueryParam::const_iterator it, itBeg = rParam.begin(), itEnd =
rParam.end();
for (it = itBeg; it != itEnd && (*it)->bDoQuery; ++it)
{
@@ -3389,12 +3406,13 @@ SCSIZE ScTable::Query(const ScQueryParam& rParamOrg,
bool bKeepSub)
}
sc::TableColumnBlockPositionSet blockPos( GetDoc(), nTab ); // cache mdds
access
+ ValidQueryCache validQueryCache;
SCROW nRealRow2 = aParam.nRow2;
for (SCROW j = aParam.nRow1 + nHeader; j <= nRealRow2; ++j)
{
bool bResult; // Filter result
- bool bValid = ValidQuery(j, aParam, nullptr, nullptr, nullptr,
&blockPos);
+ bool bValid = ValidQuery(j, aParam, nullptr, nullptr, nullptr,
&blockPos, &validQueryCache);
if (!bValid && bKeepSub) // Keep subtotals
{
for (SCCOL nCol=aParam.nCol1; nCol<=aParam.nCol2 && !bValid;
nCol++)
commit 2de99501b7d7b32867d23d4089197e132c6a0526
Author: Luboš Luňák <l.lu...@collabora.com>
AuthorDate: Mon Nov 22 10:35:48 2021 +0100
Commit: Luboš Luňák <l.lu...@collabora.com>
CommitDate: Mon Nov 22 15:46:32 2021 +0100
Revert "cache FormulaError::NoRef error string (tdf#144249)"
A more generic fix coming.
This reverts commit b4c1bb2f91e9ae47820c289e2c08f640a958cf05.
Change-Id: Ia7821f1c8585506556708f1bf8526e7f509aafd1
Reviewed-on: https://gerrit.libreoffice.org/c/core/+/125659
Tested-by: Jenkins
Reviewed-by: Luboš Luňák <l.lu...@collabora.com>
diff --git a/sc/inc/global.hxx b/sc/inc/global.hxx
index 3eab2e36d8e2..dad30b02e7a8 100644
--- a/sc/inc/global.hxx
+++ b/sc/inc/global.hxx
@@ -496,7 +496,6 @@ class ScGlobal
static std::atomic<ScUnoAddInCollection*> pAddInCollection;
static std::unique_ptr<ScUserList> xUserList;
static OUString aStrClipDocName;
- static OUString aStrErrorStringNoRef;
static std::unique_ptr<SvxBrushItem> xEmptyBrushItem;
static std::unique_ptr<SvxBrushItem> xButtonBrushItem;
diff --git a/sc/source/core/data/global.cxx b/sc/source/core/data/global.cxx
index 60c0afb1ecf2..e29fccf5c1b2 100644
--- a/sc/source/core/data/global.cxx
+++ b/sc/source/core/data/global.cxx
@@ -88,7 +88,6 @@ std::atomic<::utl::TransliterationWrapper*>
ScGlobal::pTransliteration(nullptr);
std::atomic<::utl::TransliterationWrapper*>
ScGlobal::pCaseTransliteration(nullptr);
css::uno::Reference< css::i18n::XOrdinalSuffix> ScGlobal::xOrdinalSuffix;
OUString ScGlobal::aStrClipDocName;
-OUString ScGlobal::aStrErrorStringNoRef;
std::unique_ptr<SvxBrushItem> ScGlobal::xEmptyBrushItem;
std::unique_ptr<SvxBrushItem> ScGlobal::xButtonBrushItem;
@@ -305,8 +304,8 @@ OUString ScGlobal::GetErrorString(FormulaError nErr)
switch (nErr)
{
case FormulaError::NoRef:
- // tdf#144249 This may get called very extensively, so cached.
- return aStrErrorStringNoRef;
+ pErrNumber = STR_NO_REF_TABLE;
+ break;
case FormulaError::NoAddin:
pErrNumber = STR_NO_ADDIN;
break;
@@ -452,7 +451,6 @@ void ScGlobal::Init()
InitAddIns();
aStrClipDocName = ScResId( SCSTR_NONAME ) + "1";
- aStrErrorStringNoRef = ScResId( STR_NO_REF_TABLE );
// ScDocumentPool::InitVersionMaps() has been called earlier already
}
