summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--sc/inc/column.hxx2
-rw-r--r--sc/inc/document.hxx2
-rw-r--r--sc/inc/formulacell.hxx10
-rw-r--r--sc/inc/recursionhelper.hxx13
-rw-r--r--sc/inc/table.hxx2
-rw-r--r--sc/source/core/data/column2.cxx9
-rw-r--r--sc/source/core/data/document.cxx4
-rw-r--r--sc/source/core/data/formulacell.cxx150
-rw-r--r--sc/source/core/data/table1.cxx4
-rw-r--r--sc/source/core/tool/recursionhelper.cxx16
10 files changed, 141 insertions, 71 deletions
diff --git a/sc/inc/column.hxx b/sc/inc/column.hxx
index bff5e621160e..5da349006538 100644
--- a/sc/inc/column.hxx
+++ b/sc/inc/column.hxx
@@ -576,7 +576,7 @@ public:
bool ResolveStaticReference( ScMatrix& rMat, SCCOL nMatCol, SCROW nRow1, SCROW nRow2 );
void FillMatrix( ScMatrix& rMat, size_t nMatCol, SCROW nRow1, SCROW nRow2, svl::SharedStringPool* pPool ) const;
formula::VectorRefArray FetchVectorRefArray( SCROW nRow1, SCROW nRow2 );
- bool HandleRefArrayForParallelism( SCROW nRow1, SCROW nRow2 );
+ bool HandleRefArrayForParallelism( SCROW nRow1, SCROW nRow2, const ScFormulaCellGroupRef& mxGroup );
void SetFormulaResults( SCROW nRow, const double* pResults, size_t nLen );
void SetFormulaResults( SCROW nRow, const formula::FormulaConstTokenRef* pResults, size_t nLen );
diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx
index ad371bd73cd8..86079dd4c7b1 100644
--- a/sc/inc/document.hxx
+++ b/sc/inc/document.hxx
@@ -2384,7 +2384,7 @@ public:
formula::FormulaTokenRef ResolveStaticReference( const ScRange& rRange );
formula::VectorRefArray FetchVectorRefArray( const ScAddress& rPos, SCROW nLength );
- bool HandleRefArrayForParallelism( const ScAddress& rPos, SCROW nLength );
+ bool HandleRefArrayForParallelism( const ScAddress& rPos, SCROW nLength, const ScFormulaCellGroupRef& mxGroup );
/**
* Call this before any operations that might trigger one or more formula
diff --git a/sc/inc/formulacell.hxx b/sc/inc/formulacell.hxx
index 78eb1f6265b8..a9df1a3be625 100644
--- a/sc/inc/formulacell.hxx
+++ b/sc/inc/formulacell.hxx
@@ -68,6 +68,7 @@ public:
bool mbInvariant:1;
bool mbSubTotal:1;
bool mbSeenInPath:1; // For detecting cycle of formula groups
+ bool mbPartOfCycle:1; // To flag FG's part of a cycle
sal_uInt8 meCalcState;
@@ -142,8 +143,13 @@ private:
ScFormulaCell( const ScFormulaCell& ) = delete;
- bool InterpretFormulaGroupThreading(sc::FormulaLogger::GroupScope& aScope);
- bool InterpretFormulaGroupOpenCL(sc::FormulaLogger::GroupScope& aScope);
+ bool CheckComputeDependencies(sc::FormulaLogger::GroupScope& rScope);
+ bool InterpretFormulaGroupThreading(sc::FormulaLogger::GroupScope& aScope,
+ bool& bDependencyComputed,
+ bool& bDependencyCheckFailed);
+ bool InterpretFormulaGroupOpenCL(sc::FormulaLogger::GroupScope& aScope,
+ bool& bDependencyComputed,
+ bool& bDependencyCheckFailed);
bool InterpretInvariantFormulaGroup();
public:
diff --git a/sc/inc/recursionhelper.hxx b/sc/inc/recursionhelper.hxx
index b8ca1d087509..030e52c04b7c 100644
--- a/sc/inc/recursionhelper.hxx
+++ b/sc/inc/recursionhelper.hxx
@@ -104,6 +104,19 @@ public:
void PopFormulaGroup();
};
+/** A class to wrap ScRecursionHelper::PushFormulaGroup(),
+ ScRecursionHelper::PopFormulaGroup() and make these calls
+ exception safe. */
+class ScFormulaGroupCycleCheckGuard
+{
+ ScRecursionHelper& mrRecHelper;
+ bool mbShouldPop;
+public:
+ ScFormulaGroupCycleCheckGuard() = delete;
+ ScFormulaGroupCycleCheckGuard(ScRecursionHelper& rRecursionHelper, ScFormulaCellGroup* pGrp);
+ ~ScFormulaGroupCycleCheckGuard();
+};
+
#endif // INCLUDED_SC_INC_RECURSIONHELPER_HXX
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/inc/table.hxx b/sc/inc/table.hxx
index e7886b15c6ee..a0978122ca88 100644
--- a/sc/inc/table.hxx
+++ b/sc/inc/table.hxx
@@ -987,7 +987,7 @@ public:
formula::FormulaTokenRef ResolveStaticReference( SCCOL nCol, SCROW nRow );
formula::FormulaTokenRef ResolveStaticReference( SCCOL nCol1, SCROW nRow1, SCCOL nCol2, SCROW nRow2 );
formula::VectorRefArray FetchVectorRefArray( SCCOL nCol, SCROW nRow1, SCROW nRow2 );
- bool HandleRefArrayForParallelism( SCCOL nCol, SCROW nRow1, SCROW nRow2 );
+ bool HandleRefArrayForParallelism( SCCOL nCol, SCROW nRow1, SCROW nRow2, const ScFormulaCellGroupRef& mxGroup );
void SplitFormulaGroups( SCCOL nCol, std::vector<SCROW>& rRows );
void UnshareFormulaCells( SCCOL nCol, std::vector<SCROW>& rRows );
diff --git a/sc/source/core/data/column2.cxx b/sc/source/core/data/column2.cxx
index 912428f688e1..5ef16b9bf2c6 100644
--- a/sc/source/core/data/column2.cxx
+++ b/sc/source/core/data/column2.cxx
@@ -48,6 +48,7 @@
#include <scmatrix.hxx>
#include <rowheightcontext.hxx>
#include <tokenstringcontext.hxx>
+#include <recursionhelper.hxx>
#include <editeng/eeitem.hxx>
@@ -2831,7 +2832,7 @@ formula::VectorRefArray ScColumn::FetchVectorRefArray( SCROW nRow1, SCROW nRow2
return formula::VectorRefArray(formula::VectorRefArray::Invalid);
}
-bool ScColumn::HandleRefArrayForParallelism( SCROW nRow1, SCROW nRow2 )
+bool ScColumn::HandleRefArrayForParallelism( SCROW nRow1, SCROW nRow2, const ScFormulaCellGroupRef& mxGroup )
{
if (nRow1 > nRow2)
return false;
@@ -2858,6 +2859,12 @@ bool ScColumn::HandleRefArrayForParallelism( SCROW nRow1, SCROW nRow2 )
{
// Loop inside the formula block.
(*itCell)->MaybeInterpret();
+
+ // child cell's Interpret could result in calling dependency calc
+ // and that could detect a cycle involving mxGroup
+ // and do early exit in that case.
+ if (mxGroup->mbPartOfCycle)
+ return false;
}
nRow += nEnd - nOffset;
break;
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index f970fd44bba7..58777dc19e43 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -1803,13 +1803,13 @@ void ScDocument::UnlockAdjustHeight()
--nAdjustHeightLock;
}
-bool ScDocument::HandleRefArrayForParallelism( const ScAddress& rPos, SCROW nLength )
+bool ScDocument::HandleRefArrayForParallelism( const ScAddress& rPos, SCROW nLength, const ScFormulaCellGroupRef& mxGroup )
{
SCTAB nTab = rPos.Tab();
if (!TableExists(nTab))
return false;
- return maTabs[nTab]->HandleRefArrayForParallelism(rPos.Col(), rPos.Row(), rPos.Row()+nLength-1);
+ return maTabs[nTab]->HandleRefArrayForParallelism(rPos.Col(), rPos.Row(), rPos.Row()+nLength-1, mxGroup);
}
bool ScDocument::CanFitBlock( const ScRange& rOld, const ScRange& rNew )
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index 2de28fc8d651..dc13f05a68d9 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -529,6 +529,7 @@ ScFormulaCellGroup::ScFormulaCellGroup() :
mbInvariant(false),
mbSubTotal(false),
mbSeenInPath(false),
+ mbPartOfCycle(false),
meCalcState(sc::GroupCalcEnabled)
{
}
@@ -1474,6 +1475,15 @@ void ScFormulaCell::Interpret()
{
ScRecursionHelper& rRecursionHelper = pDocument->GetRecursionHelper();
+ if (mxGroup && mxGroup->mbSeenInPath)
+ {
+ // This call arose from a dependency calculation and
+ // we just found a cycle.
+ // This will mark all elements in the cycle as parts-of-cycle
+ ScFormulaGroupCycleCheckGuard aCycleCheckGuard(rRecursionHelper, mxGroup.get());
+ return;
+ }
+
#if DEBUG_CALCULATION
static bool bDebugCalculationInit = true;
if (bDebugCalculationInit)
@@ -1529,13 +1539,6 @@ void ScFormulaCell::Interpret()
{
pDocument->IncInterpretLevel();
- bool bCheckForFGCycle = mxGroup && !pDocument->IsThreadedGroupCalcInProgress() &&
- mxGroup->meCalcState == sc::GroupCalcEnabled &&
- ScCalcConfig::isThreadingEnabled();
- bool bPopFormulaGroup = false;
- if (bCheckForFGCycle)
- bPopFormulaGroup = rRecursionHelper.PushFormulaGroup(mxGroup.get());
-
#if DEBUG_CALCULATION
aDC.enterGroup();
bool bGroupInterpreted = InterpretFormulaGroup();
@@ -1547,9 +1550,6 @@ void ScFormulaCell::Interpret()
InterpretTail( pDocument->GetNonThreadedContext(), SCITP_NORMAL);
#endif
- if (bPopFormulaGroup)
- rRecursionHelper.PopFormulaGroup();
-
pDocument->DecInterpretLevel();
}
@@ -2452,7 +2452,10 @@ void ScFormulaCell::SetDirtyVar()
bDirty = true;
mbPostponedDirty = false;
if (mxGroup && mxGroup->meCalcState == sc::GroupCalcRunning)
+ {
mxGroup->meCalcState = sc::GroupCalcEnabled;
+ mxGroup->mbPartOfCycle = false;
+ }
// mark the sheet of this cell to be calculated
//#FIXME do we need to revert this remnant of old fake vba events? pDocument->AddCalculateTable( aPos.Tab() );
@@ -4164,13 +4167,15 @@ struct ScDependantsCalculator
{
ScDocument& mrDoc;
const ScTokenArray& mrCode;
+ const ScFormulaCellGroupRef& mxGroup;
const SCROW mnLen;
const ScAddress& mrPos;
ScDependantsCalculator(ScDocument& rDoc, const ScTokenArray& rCode, const ScFormulaCell& rCell, const ScAddress& rPos) :
mrDoc(rDoc),
mrCode(rCode),
- mnLen(rCell.GetCellGroup()->mnLength),
+ mxGroup(rCell.GetCellGroup()),
+ mnLen(mxGroup->mnLength),
mrPos(rPos)
{
}
@@ -4303,16 +4308,6 @@ struct ScDependantsCalculator
// Trim data array length to actual data range.
SCROW nTrimLen = trimLength(aRefPos.Tab(), aRefPos.Col(), aRefPos.Col(), aRefPos.Row(), mnLen);
- // Fetch double array guarantees that the length of the
- // returned array equals or greater than the requested
- // length.
-
- formula::VectorRefArray aArray;
- if (nTrimLen)
- aArray = mrDoc.FetchVectorRefArray(aRefPos, nTrimLen);
-
- if (!aArray.isValid())
- return false;
aRangeList.Join(ScRange(aRefPos.Col(), aRefPos.Row(), aRefPos.Tab(),
aRefPos.Col(), aRefPos.Row() + nTrimLen - 1, aRefPos.Tab()));
@@ -4391,7 +4386,7 @@ struct ScDependantsCalculator
for (auto nCol = rRange.aStart.Col(); nCol <= rRange.aEnd.Col(); nCol++)
{
if (!mrDoc.HandleRefArrayForParallelism(ScAddress(nCol, rRange.aStart.Row(), rRange.aStart.Tab()),
- rRange.aEnd.Row() - rRange.aStart.Row() + 1))
+ rRange.aEnd.Row() - rRange.aStart.Row() + 1, mxGroup))
return false;
}
}
@@ -4407,12 +4402,9 @@ bool ScFormulaCell::InterpretFormulaGroup()
auto aScope = sc::FormulaLogger::get().enterGroup(*pDocument, *this);
ScRecursionHelper& rRecursionHelper = pDocument->GetRecursionHelper();
- if (rRecursionHelper.GetRecursionCount())
+ if (mxGroup->mbPartOfCycle)
{
- // Do not attempt to interpret a group when calculations are already
- // running, otherwise we may run into a circular reference hell. See
- // tdf#95748
- aScope.addMessage("group calc disabled during recursive calculation.");
+ aScope.addMessage("This formula-group is part of a cycle");
return false;
}
@@ -4444,58 +4436,86 @@ bool ScFormulaCell::InterpretFormulaGroup()
// ScFormulaCell::InterpretTail()
RecursionCounter aRecursionCounter( rRecursionHelper, this);
+ bool bDependencyComputed = false;
+ bool bDependencyCheckFailed = false;
+
// Preference order:
// First try OpenCL, but only if actual OpenCL is available (i.e. no SwInterpreter).
// Then try threading and as the last one try SwInterpreter.
if( ScCalcConfig::isOpenCLEnabled())
- if( InterpretFormulaGroupOpenCL(aScope))
+ if( InterpretFormulaGroupOpenCL(aScope, bDependencyComputed, bDependencyCheckFailed))
return true;
- if( InterpretFormulaGroupThreading(aScope))
+ if( InterpretFormulaGroupThreading(aScope, bDependencyComputed, bDependencyCheckFailed))
return true;
- return InterpretFormulaGroupOpenCL(aScope);
+ return InterpretFormulaGroupOpenCL(aScope, bDependencyComputed, bDependencyCheckFailed);
}
-
-// To be called only from InterpretFormulaGroup().
-bool ScFormulaCell::InterpretFormulaGroupThreading(sc::FormulaLogger::GroupScope& aScope)
+bool ScFormulaCell::CheckComputeDependencies(sc::FormulaLogger::GroupScope& rScope)
{
- static const bool bThreadingProhibited = std::getenv("SC_NO_THREADED_CALCULATION");
- if (!bThreadingProhibited &&
- pCode->IsEnabledForThreading() &&
- ScCalcConfig::isThreadingEnabled())
- {
- // iterate over code in the formula ...
- // ensure all input is pre-calculated -
- // to avoid writing during the calculation
- ScDependantsCalculator aCalculator(*pDocument, *pCode, *this, mxGroup->mpTopCell->aPos);
-
- // Disable or hugely enlarge subset for S/W group
- // threading interpreter
-
- bool bOKToThread = aCalculator.DoIt();
+ ScRecursionHelper& rRecursionHelper = pDocument->GetRecursionHelper();
+ // iterate over code in the formula ...
+ // ensure all input is pre-calculated -
+ // to avoid writing during the calculation
- if (pDocument->GetRecursionHelper().IsInRecursionReturn())
+ bool bOKToParallelize = false;
+ {
+ ScFormulaGroupCycleCheckGuard aCycleCheckGuard(rRecursionHelper, mxGroup.get());
+ if (mxGroup->mbPartOfCycle)
{
mxGroup->meCalcState = sc::GroupCalcDisabled;
- aScope.addMessage("Recursion limit reached, cannot thread this formula group now");
+ rScope.addMessage("found circular formula-group dependencies");
return false;
}
- if (!bOKToThread)
- {
- mxGroup->meCalcState = sc::GroupCalcDisabled;
- aScope.addMessage("could not do new dependencies calculation thing");
- return false;
- }
+ ScDependantsCalculator aCalculator(*pDocument, *pCode, *this, mxGroup->mpTopCell->aPos);
+ bOKToParallelize = aCalculator.DoIt();
+ }
- if (mxGroup->meCalcState == sc::GroupCalcDisabled)
+ if (rRecursionHelper.IsInRecursionReturn())
+ {
+ mxGroup->meCalcState = sc::GroupCalcDisabled;
+ rScope.addMessage("Recursion limit reached, cannot thread this formula group now");
+ return false;
+ }
+
+ if (mxGroup->mbPartOfCycle)
+ {
+ mxGroup->meCalcState = sc::GroupCalcDisabled;
+ rScope.addMessage("found circular formula-group dependencies");
+ return false;
+ }
+
+ if (!bOKToParallelize)
+ {
+ mxGroup->meCalcState = sc::GroupCalcDisabled;
+ rScope.addMessage("could not do new dependencies calculation thing");
+ return false;
+ }
+
+ return true;
+}
+
+// To be called only from InterpretFormulaGroup().
+bool ScFormulaCell::InterpretFormulaGroupThreading(sc::FormulaLogger::GroupScope& aScope,
+ bool& bDependencyComputed,
+ bool& bDependencyCheckFailed)
+{
+ static const bool bThreadingProhibited = std::getenv("SC_NO_THREADED_CALCULATION");
+ if (!bDependencyCheckFailed && !bThreadingProhibited &&
+ pCode->IsEnabledForThreading() &&
+ ScCalcConfig::isThreadingEnabled())
+ {
+ if(!bDependencyComputed && !CheckComputeDependencies(aScope))
{
- aScope.addMessage("found circular formula-group dependencies");
+ bDependencyComputed = true;
+ bDependencyCheckFailed = true;
return false;
}
+ bDependencyComputed = true;
+
const static bool bHyperThreadingActive = tools::cpuid::hasHyperThreading();
// Then do the threaded calculation
@@ -4584,7 +4604,9 @@ bool ScFormulaCell::InterpretFormulaGroupThreading(sc::FormulaLogger::GroupScope
}
// To be called only from InterpretFormulaGroup().
-bool ScFormulaCell::InterpretFormulaGroupOpenCL(sc::FormulaLogger::GroupScope& aScope)
+bool ScFormulaCell::InterpretFormulaGroupOpenCL(sc::FormulaLogger::GroupScope& aScope,
+ bool& bDependencyComputed,
+ bool& bDependencyCheckFailed)
{
bool bCanVectorize = pCode->IsEnabledForOpenCL();
switch (pCode->GetVectorState())
@@ -4622,6 +4644,18 @@ bool ScFormulaCell::InterpretFormulaGroupOpenCL(sc::FormulaLogger::GroupScope& a
return false;
}
+ if (bDependencyCheckFailed)
+ return false;
+
+ if(!bDependencyComputed && !CheckComputeDependencies(aScope))
+ {
+ bDependencyComputed = true;
+ bDependencyCheckFailed = true;
+ return false;
+ }
+
+ bDependencyComputed = true;
+
// TODO : Disable invariant formula group interpretation for now in order
// to get implicit intersection to work.
if (mxGroup->mbInvariant && false)
diff --git a/sc/source/core/data/table1.cxx b/sc/source/core/data/table1.cxx
index 3129eff669a0..76cd876e1d49 100644
--- a/sc/source/core/data/table1.cxx
+++ b/sc/source/core/data/table1.cxx
@@ -2309,7 +2309,7 @@ formula::VectorRefArray ScTable::FetchVectorRefArray( SCCOL nCol, SCROW nRow1, S
return aCol[nCol].FetchVectorRefArray(nRow1, nRow2);
}
-bool ScTable::HandleRefArrayForParallelism( SCCOL nCol, SCROW nRow1, SCROW nRow2 )
+bool ScTable::HandleRefArrayForParallelism( SCCOL nCol, SCROW nRow1, SCROW nRow2, const ScFormulaCellGroupRef& mxGroup )
{
if (nRow2 < nRow1)
return false;
@@ -2317,7 +2317,7 @@ bool ScTable::HandleRefArrayForParallelism( SCCOL nCol, SCROW nRow1, SCROW nRow2
if ( !IsColValid( nCol ) || !ValidRow( nRow1 ) || !ValidRow( nRow2 ) )
return false;
- return aCol[nCol].HandleRefArrayForParallelism(nRow1, nRow2);
+ return aCol[nCol].HandleRefArrayForParallelism(nRow1, nRow2, mxGroup);
}
ScRefCellValue ScTable::GetRefCellValue( SCCOL nCol, SCROW nRow )
diff --git a/sc/source/core/tool/recursionhelper.cxx b/sc/source/core/tool/recursionhelper.cxx
index 665ee2ae7aeb..0c185dba3c6b 100644
--- a/sc/source/core/tool/recursionhelper.cxx
+++ b/sc/source/core/tool/recursionhelper.cxx
@@ -109,9 +109,7 @@ bool ScRecursionHelper::PushFormulaGroup(ScFormulaCellGroup* pGrp)
{
--nIdx;
assert(nIdx >= 0);
- auto& eCalcState = aFGList[nIdx]->meCalcState;
- if (eCalcState == sc::GroupCalcEnabled)
- eCalcState = sc::GroupCalcDisabled;
+ aFGList[nIdx]->mbPartOfCycle = true;
} while (aFGList[nIdx] != pGrp);
return false;
@@ -131,4 +129,16 @@ void ScRecursionHelper::PopFormulaGroup()
aFGList.pop_back();
}
+ScFormulaGroupCycleCheckGuard::ScFormulaGroupCycleCheckGuard(ScRecursionHelper& rRecursionHelper, ScFormulaCellGroup* pGrp) :
+ mrRecHelper(rRecursionHelper)
+{
+ mbShouldPop = mrRecHelper.PushFormulaGroup(pGrp);
+}
+
+ScFormulaGroupCycleCheckGuard::~ScFormulaGroupCycleCheckGuard()
+{
+ if (mbShouldPop)
+ mrRecHelper.PopFormulaGroup();
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */