[Libreoffice-commits] core.git: sc/inc sc/qa sc/source

Dennis Francis dennis.francis at collabora.co.uk
Tue Dec 12 15:52:17 UTC 2017


 sc/inc/calcconfig.hxx                |    1 
 sc/inc/tokenarray.hxx                |    9 +++++--
 sc/inc/types.hxx                     |    1 
 sc/qa/unit/parallelism.cxx           |   34 ++++++++++++++++++++++++++
 sc/source/core/data/formulacell.cxx  |   10 ++-----
 sc/source/core/tool/calcconfig.cxx   |   16 ++++++++++++
 sc/source/core/tool/formulagroup.cxx |    2 -
 sc/source/core/tool/token.cxx        |   45 +++++++++++++++++++++++++++++------
 8 files changed, 100 insertions(+), 18 deletions(-)

New commits:
commit 330b1b5bcec06e3a8d35413ff921fa219bc0a8dc
Author: Dennis Francis <dennis.francis at collabora.co.uk>
Date:   Thu Nov 30 14:36:24 2017 +0530

    Do not use threading for formula group if...
    
    ...there is at least one opcode in the token array that is
    in the blacklist set of opcodes for threading.
    
    Cleaned up the logic by removing FormulaVectorEnabledForThreading
    from ScFormulaVectorState enum and created two separate boolean
    fields in ScTokenArray to keep track of whether OpenCL or threading
    could be used.
    
    Change-Id: Ieca0004b33a3cfea6ca5c0ff90bc8cea8746d102
    Reviewed-on: https://gerrit.libreoffice.org/45564
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
    Tested-by: Michael Meeks <michael.meeks at collabora.com>
    Tested-by: Jenkins <ci at libreoffice.org>

diff --git a/sc/inc/calcconfig.hxx b/sc/inc/calcconfig.hxx
index eaf4f36f143b..77b16ab92162 100644
--- a/sc/inc/calcconfig.hxx
+++ b/sc/inc/calcconfig.hxx
@@ -49,6 +49,7 @@ struct SC_DLLPUBLIC ScCalcConfig
     bool mbHasStringRefSyntax:1;
 
     static bool isOpenCLEnabled();
+    static bool isThreadingEnabled();
     static bool isSwInterpreterEnabled();
 
     bool mbOpenCLSubsetOnly:1;
diff --git a/sc/inc/tokenarray.hxx b/sc/inc/tokenarray.hxx
index 071497e75323..308b4b23fa18 100644
--- a/sc/inc/tokenarray.hxx
+++ b/sc/inc/tokenarray.hxx
@@ -51,7 +51,9 @@ class SC_DLLPUBLIC ScTokenArray : public formula::FormulaTokenArray
     bool ImplGetReference( ScRange& rRange, const ScAddress& rPos, bool bValidOnly ) const;
 
     size_t mnHashValue;
-    ScFormulaVectorState meVectorState;
+    ScFormulaVectorState meVectorState : 4; // Only 4 bits
+    bool mbOpenCLEnabled : 1;
+    bool mbThreadingEnabled : 1;
 
 public:
     ScTokenArray();
@@ -69,7 +71,7 @@ public:
     size_t GetHash() const { return mnHashValue;}
 
     ScFormulaVectorState GetVectorState() const { return meVectorState;}
-    void ResetVectorState() { meVectorState = FormulaVectorEnabled; }
+    void ResetVectorState();
     bool IsFormulaVectorDisabled() const;
 
     /**
@@ -262,6 +264,9 @@ public:
 
     sal_Int32 GetWeight() const;
 
+    bool IsEnabledForOpenCL() const { return mbOpenCLEnabled; }
+    bool IsEnabledForThreading() const { return !mbOpenCLEnabled && mbThreadingEnabled; }
+
 #if DEBUG_FORMULA_COMPILER
     void Dump() const;
 #endif
diff --git a/sc/inc/types.hxx b/sc/inc/types.hxx
index cc82e363765d..51898c291fde 100644
--- a/sc/inc/types.hxx
+++ b/sc/inc/types.hxx
@@ -59,7 +59,6 @@ enum ScFormulaVectorState
 
     FormulaVectorEnabled,
     FormulaVectorCheckReference,
-    FormulaVectorEnabledForThreading,
     FormulaVectorUnknown
 };
 
diff --git a/sc/qa/unit/parallelism.cxx b/sc/qa/unit/parallelism.cxx
index e3bb00502ea8..428d65a20bec 100644
--- a/sc/qa/unit/parallelism.cxx
+++ b/sc/qa/unit/parallelism.cxx
@@ -45,11 +45,13 @@ public:
     void testSUMIFS();
     void testDivision();
     void testVLOOKUP();
+    void testVLOOKUPSUM();
 
     CPPUNIT_TEST_SUITE(ScParallelismTest);
     CPPUNIT_TEST(testSUMIFS);
     CPPUNIT_TEST(testDivision);
     CPPUNIT_TEST(testVLOOKUP);
+    CPPUNIT_TEST(testVLOOKUPSUM);
     CPPUNIT_TEST_SUITE_END();
 
 private:
@@ -262,6 +264,38 @@ void ScParallelismTest::testVLOOKUP()
     m_pDoc->DeleteTab(0);
 }
 
+void ScParallelismTest::testVLOOKUPSUM()
+{
+    m_pDoc->InsertTab(0, "1");
+
+    const size_t nNumRows = 4096*4;
+    OUString aTableRef = "$A$1:$B$" + OUString::number(nNumRows);
+    for (size_t i = 0; i < nNumRows; ++i)
+    {
+        m_pDoc->SetValue(0, i, 0, static_cast<double>(i));
+        m_pDoc->SetValue(1, i, 0, static_cast<double>(5*i + 100));
+        m_pDoc->SetValue(2, i, 0, static_cast<double>(nNumRows - i - 1));
+    }
+    for (size_t i = 0; i < nNumRows; ++i)
+    {
+        OUString aArgNum = "C" + OUString::number(i+1);
+        m_pDoc->SetFormula(ScAddress(3, i, 0),
+                           "=SUM(" + aArgNum + ";VLOOKUP(" + aArgNum + ";" + aTableRef + "; 2; 0)) + SUM($A1:$A2)",
+                           formula::FormulaGrammar::GRAM_NATIVE_UI);
+    }
+
+    m_xDocShell->DoHardRecalc();
+
+    size_t nArg;
+    for (size_t i = 0; i < nNumRows; ++i)
+    {
+        OString aMsg = "At row " + OString::number(i);
+        nArg = nNumRows - i - 1;
+        CPPUNIT_ASSERT_EQUAL_MESSAGE(aMsg.getStr(), 6*nArg + 101, static_cast<size_t>(m_pDoc->GetValue(3, i, 0)));
+    }
+    m_pDoc->DeleteTab(0);
+}
+
 CPPUNIT_TEST_SUITE_REGISTRATION(ScParallelismTest);
 
 CPPUNIT_PLUGIN_IMPLEMENT();
diff --git a/sc/source/core/data/formulacell.cxx b/sc/source/core/data/formulacell.cxx
index 2db3e9cda669..0d89476ec0a2 100644
--- a/sc/source/core/data/formulacell.cxx
+++ b/sc/source/core/data/formulacell.cxx
@@ -4349,8 +4349,8 @@ bool ScFormulaCell::InterpretFormulaGroup()
     RecursionCounter aRecursionCounter( pDocument->GetRecursionHelper(), this);
 
     if (!bThreadingProhibited && !ScCalcConfig::isOpenCLEnabled() &&
-        pCode->GetVectorState() == FormulaVectorEnabledForThreading &&
-        officecfg::Office::Calc::Formula::Calculation::UseThreadedCalculationForFormulaGroups::get())
+        pCode->IsEnabledForThreading() &&
+        ScCalcConfig::isThreadingEnabled())
     {
         // iterate over code in the formula ...
         // ensure all input is pre-calculated -
@@ -4444,12 +4444,11 @@ bool ScFormulaCell::InterpretFormulaGroup()
         return true;
     }
 
-    bool bCanVectorize = false;
+    bool bCanVectorize = pCode->IsEnabledForOpenCL();
     switch (pCode->GetVectorState())
     {
         case FormulaVectorEnabled:
         case FormulaVectorCheckReference:
-            bCanVectorize = true; // Good.
         break;
 
         // Not good.
@@ -4465,9 +4464,6 @@ bool ScFormulaCell::InterpretFormulaGroup()
         case FormulaVectorDisabledNotInSubSet:
             aScope.addMessage("group calc disabled due to vector state (opcode not in subset)");
             break;
-        case FormulaVectorEnabledForThreading:
-            aScope.addMessage("group calc disabled due to vector state (wanted to try threading but couldn't)");
-            break;
         case FormulaVectorDisabled:
         case FormulaVectorUnknown:
         default:
diff --git a/sc/source/core/tool/calcconfig.cxx b/sc/source/core/tool/calcconfig.cxx
index 7d5c0dbe4719..c58d07d7fb35 100644
--- a/sc/source/core/tool/calcconfig.cxx
+++ b/sc/source/core/tool/calcconfig.cxx
@@ -33,6 +33,14 @@ static rtl::Reference<ConfigurationListener> const & getMiscListener()
     return xListener;
 }
 
+static rtl::Reference<ConfigurationListener> const & getFormulaCalculationListener()
+{
+    static rtl::Reference<ConfigurationListener> xListener;
+    if (!xListener.is())
+        xListener.set(new ConfigurationListener("/org.openoffice.Office.Calc/Formula/Calculation"));
+    return xListener;
+}
+
 bool ScCalcConfig::isOpenCLEnabled()
 {
     if (utl::ConfigManager::IsFuzzing())
@@ -41,6 +49,14 @@ bool ScCalcConfig::isOpenCLEnabled()
     return gOpenCLEnabled.get();
 }
 
+bool ScCalcConfig::isThreadingEnabled()
+{
+    if (utl::ConfigManager::IsFuzzing())
+        return false;
+    static comphelper::ConfigurationListenerProperty<bool> gThreadingEnabled(getFormulaCalculationListener(), "UseThreadedCalculationForFormulaGroups");
+    return gThreadingEnabled.get();
+}
+
 bool ScCalcConfig::isSwInterpreterEnabled()
 {
     if (utl::ConfigManager::IsFuzzing())
diff --git a/sc/source/core/tool/formulagroup.cxx b/sc/source/core/tool/formulagroup.cxx
index 1c2d3d7ce727..57ad65d2b114 100644
--- a/sc/source/core/tool/formulagroup.cxx
+++ b/sc/source/core/tool/formulagroup.cxx
@@ -358,7 +358,7 @@ bool FormulaGroupInterpreterSoftware::interpret(ScDocument& rDoc, const ScAddres
 
     static const bool bThreadingProhibited = std::getenv("SC_NO_THREADED_CALCULATION");
 
-    bool bUseThreading = !bThreadingProhibited && officecfg::Office::Calc::Formula::Calculation::UseThreadedCalculationForFormulaGroups::get();
+    bool bUseThreading = !bThreadingProhibited && ScCalcConfig::isThreadingEnabled();
 
     SvNumberFormatter* pFormatter = rDoc.GetNonThreadedContext().GetFormatTable();
 
diff --git a/sc/source/core/tool/token.cxx b/sc/source/core/tool/token.cxx
index c1736c3bbbe9..1d698639d09a 100644
--- a/sc/source/core/tool/token.cxx
+++ b/sc/source/core/tool/token.cxx
@@ -1350,27 +1350,36 @@ void ScTokenArray::CheckForThreading( OpCode eOp  )
     // We only call this if it was already disabled
     assert(IsFormulaVectorDisabled());
 
+    // Don't enable threading once we decided to disable it.
+    if (!mbThreadingEnabled)
+        return;
+
     static const bool bThreadingProhibited = std::getenv("SC_NO_THREADED_CALCULATION");
 
-    if (!bThreadingProhibited && !ScCalcConfig::isOpenCLEnabled() && officecfg::Office::Calc::Formula::Calculation::UseThreadedCalculationForFormulaGroups::get())
+    if (!bThreadingProhibited)
     {
         if (aThreadedCalcBlackList.count(eOp))
         {
             SAL_INFO("sc.core.formulagroup", "opcode " << formula::FormulaCompiler().GetOpCodeMap(sheet::FormulaLanguage::ENGLISH)->getSymbol(eOp) << " disables threaded calculation of formula group");
+            mbThreadingEnabled = false;
         }
         else
-        {
             SAL_INFO("sc.core.formulagroup", "but enabling for threading instead");
-            meVectorState = FormulaVectorEnabledForThreading;
-        }
+
     }
+    else
+        mbThreadingEnabled = false;
 }
 
 void ScTokenArray::CheckToken( const FormulaToken& r )
 {
     if (IsFormulaVectorDisabled())
+    {
+        if (mbThreadingEnabled)
+            CheckForThreading(r.GetOpCode());
         // It's already disabled.  No more checking needed.
         return;
+    }
 
     OpCode eOp = r.GetOpCode();
 
@@ -1381,6 +1390,7 @@ void ScTokenArray::CheckToken( const FormulaToken& r )
         {
             SAL_INFO("sc.opencl", "opcode " << formula::FormulaCompiler().GetOpCodeMap(sheet::FormulaLanguage::ENGLISH)->getSymbol(eOp) << " disables vectorisation for formula group");
             meVectorState = FormulaVectorDisabledNotInSubSet;
+            mbOpenCLEnabled = false;
             CheckForThreading(eOp);
             return;
         }
@@ -1393,6 +1403,7 @@ void ScTokenArray::CheckToken( const FormulaToken& r )
         {
             SAL_INFO("sc.core.formulagroup", "opcode " << formula::FormulaCompiler().GetOpCodeMap(sheet::FormulaLanguage::ENGLISH)->getSymbol(eOp) << " disables S/W interpreter for formula group");
             meVectorState = FormulaVectorDisabledNotInSoftwareSubset;
+            mbOpenCLEnabled = false;
             CheckForThreading(eOp);
             return;
         }
@@ -1583,6 +1594,7 @@ void ScTokenArray::CheckToken( const FormulaToken& r )
             default:
                 SAL_INFO("sc.opencl", "opcode " << formula::FormulaCompiler().GetOpCodeMap(sheet::FormulaLanguage::ENGLISH)->getSymbol(eOp) << " disables vectorisation for formula group");
                 meVectorState = FormulaVectorDisabledByOpCode;
+                mbOpenCLEnabled = false;
         }
     }
     else if (eOp == ocPush)
@@ -1621,6 +1633,7 @@ void ScTokenArray::CheckToken( const FormulaToken& r )
                 // We don't support vectorization on these.
                 SAL_INFO("sc.opencl", "opcode ocPush: variable type " << StackVarEnumToString(r.GetType()) << " disables vectorisation for formula group");
                 meVectorState = FormulaVectorDisabledByStackVariable;
+                mbOpenCLEnabled = false;
                 CheckForThreading(eOp);
             break;
             default:
@@ -1633,6 +1646,7 @@ void ScTokenArray::CheckToken( const FormulaToken& r )
     {
         SAL_INFO("sc.opencl", "opcode " << formula::FormulaCompiler().GetOpCodeMap(sheet::FormulaLanguage::ENGLISH)->getSymbol(eOp) << " disables vectorisation for formula group");
         meVectorState = FormulaVectorDisabledNotInSubSet;
+        mbOpenCLEnabled = false;
         CheckForThreading(eOp);
     }
     // only when openCL interpreter is not enabled - the assumption is that
@@ -1644,6 +1658,7 @@ void ScTokenArray::CheckToken( const FormulaToken& r )
     {
         SAL_INFO("sc.core.formulagroup", "opcode " << formula::FormulaCompiler().GetOpCodeMap(sheet::FormulaLanguage::ENGLISH)->getSymbol(eOp) << " disables S/W interpreter for formula group");
         meVectorState = FormulaVectorDisabledNotInSoftwareSubset;
+        mbOpenCLEnabled = false;
         CheckForThreading(eOp);
     }
 }
@@ -1763,6 +1778,13 @@ void ScTokenArray::GenHash()
     mnHashValue = nHash;
 }
 
+void ScTokenArray::ResetVectorState()
+{
+    meVectorState = FormulaVectorEnabled;
+    mbOpenCLEnabled = true;
+    mbThreadingEnabled = !ScCalcConfig::isOpenCLEnabled() && ScCalcConfig::isThreadingEnabled();
+}
+
 bool ScTokenArray::IsFormulaVectorDisabled() const
 {
     switch (meVectorState)
@@ -1772,7 +1794,6 @@ bool ScTokenArray::IsFormulaVectorDisabled() const
         case FormulaVectorDisabledNotInSoftwareSubset:
         case FormulaVectorDisabledByStackVariable:
         case FormulaVectorDisabledNotInSubSet:
-        case FormulaVectorEnabledForThreading:
             return true;
         default:
             ;
@@ -1828,14 +1849,18 @@ bool ScTokenArray::IsValidReference( ScRange& rRange, const ScAddress& rPos ) co
 ScTokenArray::ScTokenArray() :
     FormulaTokenArray(),
     mnHashValue(0),
-    meVectorState(FormulaVectorEnabled)
+    meVectorState(FormulaVectorEnabled),
+    mbOpenCLEnabled(true),
+    mbThreadingEnabled(!ScCalcConfig::isOpenCLEnabled() && ScCalcConfig::isThreadingEnabled())
 {
 }
 
 ScTokenArray::ScTokenArray( const ScTokenArray& rArr ) :
     FormulaTokenArray(rArr),
     mnHashValue(rArr.mnHashValue),
-    meVectorState(rArr.meVectorState)
+    meVectorState(rArr.meVectorState),
+    mbOpenCLEnabled(rArr.mbOpenCLEnabled),
+    mbThreadingEnabled(rArr.mbThreadingEnabled)
 {
 }
 
@@ -1849,6 +1874,8 @@ ScTokenArray& ScTokenArray::operator=( const ScTokenArray& rArr )
     Assign( rArr );
     mnHashValue = rArr.mnHashValue;
     meVectorState = rArr.meVectorState;
+    mbOpenCLEnabled = rArr.mbOpenCLEnabled;
+    mbThreadingEnabled = rArr.mbThreadingEnabled;
     return *this;
 }
 
@@ -1873,6 +1900,8 @@ void ScTokenArray::Clear()
 {
     mnHashValue = 0;
     meVectorState = FormulaVectorEnabled;
+    mbOpenCLEnabled = true;
+    mbThreadingEnabled = !ScCalcConfig::isOpenCLEnabled() && ScCalcConfig::isThreadingEnabled();
     FormulaTokenArray::Clear();
 }
 
@@ -1886,6 +1915,8 @@ ScTokenArray* ScTokenArray::Clone() const
     p->bHyperLink = bHyperLink;
     p->mnHashValue = mnHashValue;
     p->meVectorState = meVectorState;
+    p->mbOpenCLEnabled = mbOpenCLEnabled;
+    p->mbThreadingEnabled = mbThreadingEnabled;
     p->mbFromRangeName = mbFromRangeName;
     p->mbShareable = mbShareable;
 


More information about the Libreoffice-commits mailing list