[Libreoffice-commits] core.git: sc/inc sc/source
Luboš Luňák
l.lunak at collabora.com
Fri Jun 15 13:12:42 UTC 2018
sc/inc/document.hxx | 1 +
sc/inc/interpretercontext.hxx | 9 +++++++++
sc/source/core/data/documen8.cxx | 5 +++++
sc/source/core/data/document.cxx | 12 ++++++++++++
sc/source/core/data/formulacell.cxx | 32 ++++++++++++++++++++++++--------
5 files changed, 51 insertions(+), 8 deletions(-)
New commits:
commit dc046b7367bf2bcb5b3b883731723da7c92f4cca
Author: Luboš Luňák <l.lunak at collabora.com>
Date: Mon Jun 11 10:28:28 2018 +0200
move SetNumberFormat() calls out of calc threads
As it modifies ScAttrArray, which is not thread-safe (has std::vector
per each column, so multiple threads may try resize it etc.). So if
threaded calculation is done, delay the calls to the main thread.
Change-Id: I3d87665c0dd0d40f0c2efbcf8958240ee5580233
Reviewed-on: https://gerrit.libreoffice.org/55602
Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
Tested-by: Jenkins
diff --git a/sc/inc/document.hxx b/sc/inc/document.hxx
index 813d388abb4d..fcd09a1214de 100644
--- a/sc/inc/document.hxx
+++ b/sc/inc/document.hxx
@@ -573,6 +573,7 @@ public:
maInterpreterContext.mpFormatter = GetFormatTable();
return maInterpreterContext;
}
+ void MergeBackIntoNonThreadedContext( ScInterpreterContext& threadedContext );
void SetThreadedGroupCalcInProgress( bool set ) { (void)this; ScGlobal::bThreadedGroupCalcInProgress = set; }
bool IsThreadedGroupCalcInProgress() const { (void)this; return ScGlobal::bThreadedGroupCalcInProgress; }
diff --git a/sc/inc/interpretercontext.hxx b/sc/inc/interpretercontext.hxx
index d2b6814fad6e..8b920472fb44 100644
--- a/sc/inc/interpretercontext.hxx
+++ b/sc/inc/interpretercontext.hxx
@@ -12,18 +12,27 @@
#include <vector>
#include <formula/token.hxx>
+#include "types.hxx"
#define TOKEN_CACHE_SIZE 8
class ScDocument;
class SvNumberFormatter;
+// SetNumberFormat() is not thread-safe, so calls to it need to be delayed to the main thread.
+struct DelayedSetNumberFormat
+{
+ SCROW mRow; // Used only with formula groups, so column and tab do not need to be stored.
+ sal_uInt32 mnNumberFormat;
+};
+
struct ScInterpreterContext
{
const ScDocument& mrDoc;
SvNumberFormatter* mpFormatter;
size_t mnTokenCachePos;
std::vector<formula::FormulaToken*> maTokens;
+ std::vector<DelayedSetNumberFormat> maDelayedSetNumberFormat;
ScInterpreterContext(const ScDocument& rDoc, SvNumberFormatter* pFormatter)
: mrDoc(rDoc)
diff --git a/sc/source/core/data/documen8.cxx b/sc/source/core/data/documen8.cxx
index 293eb556d676..7d5542173b96 100644
--- a/sc/source/core/data/documen8.cxx
+++ b/sc/source/core/data/documen8.cxx
@@ -445,6 +445,11 @@ const ScDocumentThreadSpecific& ScDocument::CalculateInColumnInThread( ScInterpr
void ScDocument::HandleStuffAfterParallelCalculation( const ScAddress& rTopPos, size_t nLen )
{
+ assert(!IsThreadedGroupCalcInProgress());
+ for( DelayedSetNumberFormat& data : GetNonThreadedContext().maDelayedSetNumberFormat)
+ SetNumberFormat( ScAddress( rTopPos.Col(), data.mRow, rTopPos.Tab()), data.mnNumberFormat );
+ GetNonThreadedContext().maDelayedSetNumberFormat.clear();
+
ScTable* pTab = FetchTable(rTopPos.Tab());
if (!pTab)
return;
diff --git a/sc/source/core/data/document.cxx b/sc/source/core/data/document.cxx
index 88d0617b038a..411e71127e38 100644
--- a/sc/source/core/data/document.cxx
+++ b/sc/source/core/data/document.cxx
@@ -3693,6 +3693,7 @@ sal_uInt32 ScDocument::GetNumberFormat( const ScInterpreterContext& rContext, co
void ScDocument::SetNumberFormat( const ScAddress& rPos, sal_uInt32 nNumberFormat )
{
+ assert(!IsThreadedGroupCalcInProgress());
SCTAB nTab = rPos.Tab();
if (!TableExists(nTab))
return;
@@ -6756,4 +6757,15 @@ void ScDocumentThreadSpecific::MergeBackIntoNonThreadedData(ScDocumentThreadSpec
// What about recursion helper and lookup cache?
}
+void ScDocument::MergeBackIntoNonThreadedContext(ScInterpreterContext& threadedContext)
+{
+ // Move data from a context used by a calculation thread to the main thread's context.
+ // Called from the main thread after the calculation thread has already finished.
+ assert(!IsThreadedGroupCalcInProgress());
+ maInterpreterContext.maDelayedSetNumberFormat.insert(
+ maInterpreterContext.maDelayedSetNumberFormat.end(),
+ std::make_move_iterator(threadedContext.maDelayedSetNumberFormat.begin()),
+ std::make_move_iterator(threadedContext.maDelayedSetNumberFormat.end()));
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index 4aca388c2f6e..6dda20b6a3b5 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -2009,7 +2009,16 @@ void ScFormulaCell::InterpretTail( ScInterpreterContext& rContext, ScInterpretTa
if (bSetFormat && (bForceNumberFormat || ((nFormatIndex % SV_COUNTRY_LANGUAGE_OFFSET) != 0)))
{
// set number format explicitly
- pDocument->SetNumberFormat( aPos, nFormatIndex );
+ if (!pDocument->IsThreadedGroupCalcInProgress())
+ pDocument->SetNumberFormat( aPos, nFormatIndex );
+ else
+ {
+ // SetNumberFormat() is not thread-safe (modifies ScAttrArray), delay the work
+ // to the main thread. Since thread calculations operate on formula groups,
+ // it's enough to store just the row.
+ DelayedSetNumberFormat data = { aPos.Row(), nFormatIndex };
+ rContext.maDelayedSetNumberFormat.push_back( data );
+ }
bChanged = true;
}
@@ -4435,7 +4444,7 @@ bool ScFormulaCell::InterpretFormulaGroup()
const unsigned mnThisThread;
const unsigned mnThreadsTotal;
ScDocument* mpDocument;
- SvNumberFormatter* mpFormatter;
+ ScInterpreterContext* mpContext;
const ScAddress& mrTopPos;
SCROW mnLength;
@@ -4444,14 +4453,14 @@ bool ScFormulaCell::InterpretFormulaGroup()
unsigned nThisThread,
unsigned nThreadsTotal,
ScDocument* pDocument2,
- SvNumberFormatter* pFormatter,
+ ScInterpreterContext* pContext,
const ScAddress& rTopPos,
SCROW nLength) :
comphelper::ThreadTask(rTag),
mnThisThread(nThisThread),
mnThreadsTotal(nThreadsTotal),
mpDocument(pDocument2),
- mpFormatter(pFormatter),
+ mpContext(pContext),
mrTopPos(rTopPos),
mnLength(nLength)
{
@@ -4459,9 +4468,7 @@ bool ScFormulaCell::InterpretFormulaGroup()
virtual void doWork() override
{
- ScInterpreterContext aContext(*mpDocument, mpFormatter);
-
- auto aNonThreadedData = mpDocument->CalculateInColumnInThread(aContext, mrTopPos, mnLength, mnThisThread, mnThreadsTotal);
+ auto aNonThreadedData = mpDocument->CalculateInColumnInThread(*mpContext, mrTopPos, mnLength, mnThisThread, mnThreadsTotal);
aNonThreadedData.MergeBackIntoNonThreadedData(mpDocument->maNonThreaded);
}
@@ -4485,9 +4492,11 @@ bool ScFormulaCell::InterpretFormulaGroup()
// Start nThreadCount new threads
std::shared_ptr<comphelper::ThreadTaskTag> aTag = comphelper::ThreadPool::createThreadTaskTag();
+ std::vector<ScInterpreterContext*> contexts(nThreadCount);
for (int i = 0; i < nThreadCount; ++i)
{
- rThreadPool.pushTask(new Executor(aTag, i, nThreadCount, pDocument, pNonThreadedFormatter, mxGroup->mpTopCell->aPos, mxGroup->mnLength));
+ contexts[i] = new ScInterpreterContext(*pDocument, pNonThreadedFormatter);
+ rThreadPool.pushTask(new Executor(aTag, i, nThreadCount, pDocument, contexts[i], mxGroup->mpTopCell->aPos, mxGroup->mnLength));
}
SAL_INFO("sc.threaded", "Joining threads");
@@ -4495,6 +4504,13 @@ bool ScFormulaCell::InterpretFormulaGroup()
pDocument->SetThreadedGroupCalcInProgress(false);
+ for (int i = 0; i < nThreadCount; ++i)
+ {
+ // This is intentionally done in this main thread in order to avoid locking.
+ pDocument->MergeBackIntoNonThreadedContext(*contexts[i]);
+ delete contexts[i];
+ }
+
SAL_INFO("sc.threaded", "Done");
}
More information about the Libreoffice-commits
mailing list